[RFC] Allow overloaded general functions

[RFC] Allow overloaded general functions

[Pierre Muller (gmail)]

  I tried to write a patch that allows to 
set a breakpoint at each overloaded version of a procedure or function,
which is a fairly common feature in pascal language.

  Below is a patch that allows, for pascal language
(but it could be used for all languages allowing several
version of the same function), to do :
(gdb) break ADD
 [0] cancel
[1] all
[2] ADD at addstring.pp:16
[3] ADD at addlongint.pp:17
[4] ADD at addint.pp:17
> 1
Breakpoint 1 at 0x40102d: file addstring.pp, line 16.
Breakpoint 2 at 0x4010aa: file addlongint.pp, line 17.
Breakpoint 3 at 0x4010ca: file addint.pp, line 17.
warning: Multiple breakpoints were set.
Use the "delete" command to delete unwanted breakpoints.

Thus this enables one to obtain the listing of the overloaded versions of
this function.
 This seemed to be working fine, but
the problem is that this comes in conflict 
with breakpoint_re_set function that is called
each time a new library is loaded.

The address of 'ADD' is searched again using decode_line_1 function
but there is later an assertion 
  gdb_assert (sals.nelts == 1) in breakpoint.c at line 7366

 I could of course remove that assertion and check that  in the list
that I get, I do find the same source and line number I already had in 
my breakpoint structure, but what is the whole point of this?
Why do we re_set breakpoints that are not pending?
Is this for the unloading case ?
If this is the case than the modification above is
probably necessary.
However, I am puzzled why other similar case don't have these
troubles (like the decode_objc case.)

 What should I do if I get a new additional 'ADD' function in the
loaded library?

  There also seems to be a conflict with the multiple breakpoint
that has been implemented recently for inlined functions.
Indeed if my three function are generated using the same 
sources (with include files), then I get a weird result:
(gdb) b ADD
[0] cancel
[1] all
[2] ADD at add.inc:4
[3] ADD at add.inc:5
[4] ADD at add.inc:5
> 1
Breakpoint 1 at 0x40102d: file add.inc, line 4. (3 locations)
Note: breakpoint 1 also set at pc 0x4010aa.
Note: breakpoint 1 also set at pc 0x4010ca.
Breakpoint 2 at 0x401057: file add.inc, line 5. (3 locations)
Note: breakpoint 2 also set at pc 0x401057.
Note: breakpoints 1 and 2 also set at pc 0x4010aa.
Note: breakpoints 1 and 2 also set at pc 0x4010ca.
Breakpoint 3 at 0x401057: file add.inc, line 5. (3 locations)
warning: Multiple breakpoints were set.
Use the "delete" command to delete unwanted breakpoints.
(gdb) inf b
Num     Type           Disp Enb Address    What
1       breakpoint     keep y   <MULTIPLE> 0x0040102d
1.1                         y     0x0040102d in ADD at add.inc:4
1.2                         y     0x004010aa in ADD at add.inc:4
1.3                         y     0x004010ca in ADD at add.inc:4
2       breakpoint     keep y   <MULTIPLE> 0x00401057
2.1                         y     0x00401057 in ADD at add.inc:5
2.2                         y     0x004010aa in ADD at add.inc:5
2.3                         y     0x004010ca in ADD at add.inc:5
3       breakpoint     keep y   <MULTIPLE> 0x00401057
3.1                         y     0x00401057 in ADD at add.inc:5
3.2                         y     0x004010aa in ADD at add.inc:5
3.3                         y     0x004010ca in ADD at add.inc:5

Which is not what I would have liked.
This kind of multiple breakpoints
seems to be restricted to the same source-name/line number pair
being used several times, and thus is not
really extendable to the case of interest for me.

All comments most welcome.

Pierre Muller
Pascal language support maintainer for GDB

ChangeLog entry:

2008-03-28  Pierre Muller  <muller@ics.u-strasbg.fr>

	. linespec.c (decode_multiple_func): New function.
	(decode_line_1): Call decode_multiple_func if 
	current_language is pascal.


Index: gdb/linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.74
diff -u -p -r1.74 linespec.c
--- gdb/linespec.c      1 Jan 2008 22:53:11 -0000       1.74
+++ gdb/linespec.c      28 Mar 2008 14:12:16 -0000
@@ -58,6 +58,13 @@ static struct symtabs_and_lines decode_o
                                             char ***canonical,
                                             char *saved_arg);

+static struct symtabs_and_lines decode_multiple_func (char **argptr,
+                                            int funfirstline,
+                                            struct symtab *file_symtab,
+                                            char ***canonical,
+                                            char *saved_arg);
+
+
 static struct symtabs_and_lines decode_compound (char **argptr,
                                                 int funfirstline,
                                                 char ***canonical,
@@ -856,6 +863,16 @@ decode_line_1 (char **argptr, int funfir
   /* Look up that token as a variable.
      If file specified, use that file's per-file block to start with.  */

+  if (current_language && current_language->la_language == language_pascal)
+  {
+    struct symtabs_and_lines values;
+    values = decode_multiple_func (&copy, funfirstline, file_symtab,
+                         canonical, saved_arg);
+    if (values.sals != NULL)
+      return values;
+  }
+
+
   return decode_variable (copy, funfirstline, canonical,
                          file_symtab, not_found_ptr);
 }
@@ -1161,6 +1178,86 @@ decode_objc (char **argptr, int funfirst
   return values;
 }

+/* Search for multiple functions with same na
+   as allowed in pascal language for instance */
+
+
+struct symtabs_and_lines
+decode_multiple_func (char **argptr, int funfirstline,
+            struct symtab *file_symtab,
+            char ***canonical, char *saved_arg)
+{
+  struct symtabs_and_lines values;
+  struct symbol **sym_arr = NULL;
+  struct symbol *sym = NULL;
+  char *copy = NULL;
+  struct block *block = NULL;
+  unsigned i1 = 0;
+  unsigned i2 = 0;
+
+  struct symbol_search *symbols;
+  struct symbol_search *p;
+  struct cleanup *old_chain;
+  char *last_filename = NULL;
+  int first = 1;
+  values.sals = NULL;
+  values.nelts = 0;
+
+  /* must make sure that if we're interrupted, symbols gets freed */
+  if (file_symtab)
+    search_symbols (*argptr, FUNCTIONS_DOMAIN, 1,
+                   (char **) &(file_symtab->filename), &symbols);
+  else
+    search_symbols (saved_arg, FUNCTIONS_DOMAIN, 0, (char **) NULL,
&symbols);
+
+  for (p = symbols; p != NULL; p = p->next)
+    {
+      i1++;
+    }
+
+  sym_arr = (struct symbol **) alloca ((i1 + 1) * sizeof (struct symbol
*));
+  sym_arr[i1] = NULL;
+  for (p = symbols; p != NULL; p = p->next)
+    {
+      if (p->symbol)
+       {
+         sym_arr[i2] = p->symbol;
+         i2++;
+       }
+    }
+
+  if (i2 > 1)
+    {
+      /* More than one match. The user must choose one or more.  */
+      return decode_line_2 (sym_arr, i2, funfirstline, canonical);
+    }
+  else if (i2 == 1)
+    {
+      struct symbol *sym = symbols->symbol;
+
+      values.sals = (struct symtab_and_line *) xmalloc (
+                    sizeof (struct symtab_and_line));
+      values.nelts = 1;
+      if (sym && SYMBOL_CLASS (sym) == LOC_BLOCK)
+       {
+         /* Canonicalize this, so it remains resolved for dylib loads.  */
+         values.sals[0] = find_function_start_sal (sym, funfirstline);
+         build_canonical_line_spec (values.sals, SYMBOL_NATURAL_NAME (sym),
+                                    canonical);
+       }
+      else
+       {
+         /* The only match was a non-debuggable symbol.  */
+         values.sals[0].symtab = NULL;
+         values.sals[0].line = 0;
+         values.sals[0].end = 0;
+         values.sals[0].pc = SYMBOL_VALUE_ADDRESS (sym_arr[0]);
+       }
+    }
+  return values;
+}
+
+
 /* This handles C++ and Java compound data structures.  P should point
    at the first component separator, i.e. double-colon or period.  As
    an example, on entrance to this function we could have ARGPTR

Re: [RFC] Allow overloaded general functions

[Matt Rice]

On Fri, Mar 28, 2008 at 9:22 AM, Pierre Muller (gmail)
<pierre.c.muller@gmail.com> wrote:

<snip>

>  The address of 'ADD' is searched again using decode_line_1 function
>  but there is later an assertion
>   gdb_assert (sals.nelts == 1) in breakpoint.c at line 7366
>
>   I could of course remove that assertion and check that  in the list
>  that I get, I do find the same source and line number I already had in
>  my breakpoint structure, but what is the whole point of this?
>  Why do we re_set breakpoints that are not pending?
>
>  Is this for the unloading case ?
>  If this is the case than the modification above is
>  probably necessary.
>  However, I am puzzled why other similar case don't have these
>  troubles (like the decode_objc case.)
>

the reason this doesn't appear to be an issue with decode_objc is because

(gdb) break foo
[0] cancel
[1] all
[2] +[Bar foo] at main.m:30
[3] +[Foo foo] at main.m:19

therefore this line above the assertion
7310:      s = b->addr_string;
will be unique and the assertion wont fail (i think),
though decode_objc currently also has its own problems that
i still need to revisit as per Daniels comments

http://sourceware.org/ml/gdb-patches/2008-03/msg00200.html

>   What should I do if I get a new additional 'ADD' function in the
>  loaded library?
>

i can imagine a
[2] pending

might handle this but i'm not sure that that won't just open up a can
of worms :)

the RFA Keep breakpoints always inserted thread sounds like it could
help with this re_set ing (sorry its spread out across months in the
archives), i've been meaning to try those patches out.

Re: [RFC] Allow overloaded general functions

[Joel Brobecker]

On Fri, Mar 28, 2008 at 05:22:03PM +0100, Pierre Muller (gmail) wrote:
> I tried to write a patch that allows to 
> set a breakpoint at each overloaded version of a procedure or function,
> which is a fairly common feature in pascal language.

Sorry for being so late in replying to this, but I was really REALLY
hoping to look at the same sort of issue for Ada as well before
answering. Bad luck, I have been so tied up that I never found
the time.

Nevertheless, I don't believe that Pascal and Ada are the only languages
affected. Not sure how the C++ mode handles overloading in GDB, buti
presumably, C++ should have the same issue - perhaps the user is
expected to fully qualify his entity?

I suggest you have a look at a patch I sent last January:

    http://www.sourceware.org/ml/gdb-patches/2008-01/msg00008.html

The way we handle overloading is by creating several breakpoints that
have only one location, as opposed to multiple-location breakpoints.
Just like you.  If creating several breakpoints is easier in the short
term, I really think we should try to transition towards multiple-location
breakpoint when we can.

The problem, as you have found out, is when we re-evaluate each
breakpoint we just created. Using your example:

> [2] ADD at addstring.pp:16
> [3] ADD at addlongint.pp:17
> [4] ADD at addint.pp:17

Since you chose "all", you created 3 breakpoints. I'll number them
1, 2 and 3 in the order above.  When you "run" your program, or when
a new shared library is mapped, you need to re-evaluate each of them,
meaning we need to verify that the actual address hasn't changed.
If you do not add some kind of information in each breakpoint to
say which instance it is refering to, then you'll never be able
to determine that breakpoint 2 is at addlongint.pp:17.

For Ada, we handled that issue by rewriting the breakpoint string
into a canonical form that allows us to uniquely identify the location
of each of them. From memory, it should look like this:

    addstring.pp:ADD:16

(or, said different in a general way: FILE:FUNCTION_NAME:LINE_NO).

Feedback about this approach was not entirely enthousiastic,
unfortunately, largely because I was using that form for Ada only,
which was a divergence that Daniel wanted to avoid if possible.
But also, when I proposed to see if that form could be generalized
to all languages , Daniel pointed out:

> I think the entire concept of a location specification which is both
> canonical and user-inputable is on its last legs.  If you want to
> identify just one line of one homonym, what about one inlined instance
> of that line?  One instantiation in a particular shared library?
> I've looked at the problem before but never got anywhere.

This was part of a very small private discussion because I wasn't
sure how bad Daniel was feeling about my patch.

I was hoping to be able to look at this problem again in the near
future, and since then nothing :-(.

I can propose the following: We can look again at working from
the Ada approach (and patch) and make it work for all languages
instead of just Ada. We can use this canonical form, or any way
we find convenient, to specify the breakpoint location. As Daniel
points out, it won't cover all possible cases and a more powerful
approach will eventually be needed if we want to support some of
them. But in the meantime, we will be able to insert breakpoints
on homonyms in both Pascal and Ada (and possibly C++) for the vast
majority of cases.

Daniel, what do you think?

-- 
Joel

RE: [RFC] Allow overloaded general functions

[Pierre Muller]

  Hi Joel,

Thanks for working on this issue.

> -----Message d'origine-----
> De : Joel Brobecker [mailto:brobecker@adacore.com]
> Envoyé : Thursday, September 18, 2008 2:34 AM
> À : Pierre Muller (gmail)
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFC] Allow overloaded general functions
> 
> On Fri, Mar 28, 2008 at 05:22:03PM +0100, Pierre Muller (gmail) wrote:
> > I tried to write a patch that allows to
> > set a breakpoint at each overloaded version of a procedure or
> function,
> > which is a fairly common feature in pascal language.
> 
> Sorry for being so late in replying to this, but I was really REALLY
> hoping to look at the same sort of issue for Ada as well before
> answering. Bad luck, I have been so tied up that I never found
> the time.
> 
> Nevertheless, I don't believe that Pascal and Ada are the only
> languages
> affected. Not sure how the C++ mode handles overloading in GDB, buti
> presumably, C++ should have the same issue - perhaps the user is
> expected to fully qualify his entity?
> 
> I suggest you have a look at a patch I sent last January:
> 
>     http://www.sourceware.org/ml/gdb-patches/2008-01/msg00008.html
> 
> The way we handle overloading is by creating several breakpoints that
> have only one location, as opposed to multiple-location breakpoints.
> Just like you.  If creating several breakpoints is easier in the short
> term, I really think we should try to transition towards multiple-
> location
> breakpoint when we can.
> 
> The problem, as you have found out, is when we re-evaluate each
> breakpoint we just created. Using your example:
> 
> > [2] ADD at addstring.pp:16
> > [3] ADD at addlongint.pp:17
> > [4] ADD at addint.pp:17
> 
> Since you chose "all", you created 3 breakpoints. I'll number them
> 1, 2 and 3 in the order above.  When you "run" your program, or when
> a new shared library is mapped, you need to re-evaluate each of them,
> meaning we need to verify that the actual address hasn't changed.
> If you do not add some kind of information in each breakpoint to
> say which instance it is refering to, then you'll never be able
> to determine that breakpoint 2 is at addlongint.pp:17.
> 
> For Ada, we handled that issue by rewriting the breakpoint string
> into a canonical form that allows us to uniquely identify the location
> of each of them. From memory, it should look like this:
> 
>     addstring.pp:ADD:16

  In pascal, you could  also have some (fairly rare) case where this would
not be enough:
you could simulate 'templates' (does this concept also mean something in C++
or Ada?)
by simply writing an include file (called add.inc) like this:

line 1)  function add(x,y : add_type) : add_type;
line 2)   
line 3)  begin
line 4)    add := x + y;
line 5)  end;

and include that file in several units, add_real, add_integer, add_string:

line  1)  unit add_real;
line  2)  
line  3)  interface
line  4)  type add_type = real;
line  5)  
line  6)  {$i addh.inc}
line  7)  
line  8)  implementation
line  9)  
line 10)  {$i add.inc}
line 11)  
line 12)  end.

In such a case all three versions would have the same
  add.inc:ADD:3
identifier...

   In fact even adding the name of the argument types or return type
wouldn't help here as all are 'add_type'.

  But as you said, this case would only be marginal, and your
suggestion should allow to treat most cases successfully.

  The only option that would (maybe) give a more deterministic
behavior for pascal would be to associate each sub-breakpoint to
the real mangled assembler label that is always unique (at least for
interface defined functions).

  To summarize, I agree that your patch would probably
resolve most issues and as such I think that it is a step in the good
direction.



Pierre Muller
Pascal language support maintainer for GDB

Re: [RFC] Allow overloaded general functions

[Joel Brobecker]

> > For Ada, we handled that issue by rewriting the breakpoint string
> > into a canonical form that allows us to uniquely identify the location
> > of each of them. From memory, it should look like this:
> > 
> >     addstring.pp:ADD:16
> 
> In pascal, you could  also have some (fairly rare) case where this
> would not be enough: you could simulate 'templates' (does this
> concept also mean something in C++ or Ada?)

Ah, yes. In Ada, the C++ templates are called "generics" (generic
package, or generic subprogram), and indeed, this can be a problem.

The reason this hasn't been a problem in Ada is because I gave you
a simplified version of the story. Without entering too much into
details, Ada entities have a fully-qualified name. For instance,
if variable Foo is defined in Package Pck, the the fully qualified
name is Pck.Foo.  Now, generics are not compiled, only the actual
instances.  And the fully qualified name depends on the scope where
the generic is instantiated. For instance, let's say that Package
Pck is a generic, and that it is instantiated in package Bar using
the following statement:

        package My_Instance is new Pck (...);

The fully qualified name for our procedure Foo is then: Bar.My_Instance.Foo.

Now, going back to the actual issue, we store the fully qualified name
in our cannonical version (or, in GDB terms, the SYMBOL_PRINT_NAME).
That, in itself, handles the case of generics.  The filename and line
number additions are only necessary to handle overloading, ie cases
where two functions have the same fully qualfied name:

        package Pck is
           procedure Print (F : Float);
           procedure Print (I : Integer);
        end Pck;

Now, back to Pascal:

> by simply writing an include file (called add.inc) like this:
[...]
> In such a case all three versions would have the same
>   add.inc:ADD:3
> identifier...
[...]
>   But as you said, this case would only be marginal, and your
> suggestion should allow to treat most cases successfully.

I don't think it's marginal enough that we should ignore it.
Generics are a very common practice in Ada, and I don't see why
it wouldn't be as common in Pascal.

>   The only option that would (maybe) give a more deterministic
> behavior for pascal would be to associate each sub-breakpoint to
> the real mangled assembler label that is always unique (at least for
> interface defined functions).

Yes, that's my thought as well - we can use the SYMBOL_LINKAGE_NAME
in the cannonical form. However, we need to be a little careful, here,
because we also need to be able to print something friendly when the
user ask for the list of breakpoints:

   (gdb) info break
   Num     Type           Disp Enb Address    What
   1       breakpoint     keep y   0x080495df in foo.one.call_me at pck.adb:5
   2       breakpoint     keep y   0x080495d3 in foo.two.call_me at pck.adb:5

In the "What" column, we don't want:

   in foo__one__call_me.1 at pck.adb:5
   in foo__two__call_me.2 at pck.adb:5

Also, we need to make sure that we are able to re-evaluate correctly
the breakpoint later on.  For instance, the ".DIGIT" suffix that the
Ada compiler adds at the end of nested procedures can confuse the
symbol lookup routine (not sure if this is the case or not in this
example, but that's only a technical details).

-- 
Joel