[RFC] lookup problem in blockframe.c:inside_main_func()

[RFC] lookup problem in blockframe.c:inside_main_func()

[Joel Brobecker]

Hello,

I just noticed this strange behavior in GDB when the name of the
application main procedure is called Main:

        (gdb) bt
        #0  process (prm_event=50) at process.adb:6
        [0] cancel
        [1] main at main.adb:6
        [2] main at b~main.adb:135

A bit of information regarding Ada, which could be useful to understand
what is happening in our case. Here is a typical Hello world in Ada95:

        with Ada.Text_IO; use Ada.Text_IO;
        
        procedure Hello is
        begin
           Put_Line ("Hello world");
        end Hello;

As you see, as opposed to C, the main procedure does not need to be
called Main in order for it to be what I will call the "user-level
main procedure". Because Ada defines a notion of elaboration, the
user-level main procedure can not be run directly when you execute
your program. GNAT does the following when building an application:
It creates a procedure which name is main(). This is the entry point
of the application which will do the following:
  - Do the program elaboration
  - Call the user-level procedure.

GNAT, just as g++ I imagine, encodes its entity names, and the way
the user-level main procedure name is encoded is simply by prepending
"_ada_" to its lowercased name. For instance, the encoded name for 
procedure Hello is "_ada_hello". And if our procedure was called Main,
it would be "_ada_main".

Suppose now that the name of the user-level main procedure was "Main",
and the user asks GDB to put a breakpoint on "main". What should GDB
understand: A breakpoint on "main", or "_ada_main"? There is no way
to tell, so GDB, via the ada language lookup function, asks the user
which one he meant with the menu shown in the transcript above.

Back to the problem at hand:

What happens is that the unwinder has be enhanced since 5.3 to make sure
not to unwind past the main procedure. Part of the machinery used to
avoid this uses the inside_main_func() function. This function in turn
relies on lookup_symbol(), which itself eventually calls the
language-specific lookup procedure (for static and global variables).

And this is when things take an unwanted turn. The conjunction of
the fact that the current language is ada, and the fact that "main"
is ambiguous as far as the ada language is concerned, causes the
unwanted multiple choice menu.

Really, the lookup we intended to do was a simple, plain, symbol lookup
of "main". In an attempt to reflect this, I have tried the following
change in inside_main_func(), and it fixes the problem at hand:

| +      enum language saved_language = current_language->la_language;
|  
| +      set_language (language_c);
|        mainsym = lookup_symbol (main_name (), NULL, VAR_DOMAIN, NULL, NULL);
| +      set_language (saved_language);

I would be more than happy to properly contribute this change, with
comments in the code, and a changelog entry, but I see two problems
that bother me a bit:
  
  1. We rely on a global variable to pass some information to a procedure,
     and this not very good.

  2. The current change will fail to restore the saved language in case
     an exception is raised during the call to lookup_symbol. Not really
     that big a deal, but not very user-friendly either.

My current thoughts about each problem:

  1. It's a bit bothersome, but on the other hand lookup_symbol is
     used everywhere. Any change in its interface would be a jumbo
     patch. Nevertheless, I am ready to discuss this option if you
     feel that we should bite the bullet and avoid global variables
     as much as we can (remember about the language problems we still
     have to tackle?). Let me know. For now, since current_language
     is already so much widely used, I'm inclined to simply wrap
     the call to lookup_symbol() between calls to set_language().

  2. This can easily be fixed. I think the cleanest way to do this is
     by renaming inside_main_func() into inside_main_func_1(), and
     write a new inside_main_func() that calls inside_main_func_1()
     via catch_exceptions(). We can then safely wrap it between
     calls to set_language(). But this is assuming that we decide
     that we don't mind about concern #1 above. But I find the
     catch exceptions interface a bit awkward. Mixing the error code
     returned by catch_exceptions() with the return value of the
     function called makes me flinch a bit.

What do you think?
-- 
Joel

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Daniel Jacobowitz]

[First of all, what path takes you through lookup_symbol to that
prompt?  That shouldn't ever happen, unless it's being generated in
Ada-specific code...]

On Mon, Oct 06, 2003 at 04:37:29PM -0700, Joel Brobecker wrote:
> Really, the lookup we intended to do was a simple, plain, symbol lookup
> of "main". In an attempt to reflect this, I have tried the following
> change in inside_main_func(), and it fixes the problem at hand:
> 
> | +      enum language saved_language = current_language->la_language;
> |  
> | +      set_language (language_c);
> |        mainsym = lookup_symbol (main_name (), NULL, VAR_DOMAIN, NULL, NULL);
> | +      set_language (saved_language);
> 
> I would be more than happy to properly contribute this change, with
> comments in the code, and a changelog entry, but I see two problems
> that bother me a bit:
>   
>   1. We rely on a global variable to pass some information to a procedure,
>      and this not very good.
> 
>   2. The current change will fail to restore the saved language in case
>      an exception is raised during the call to lookup_symbol. Not really
>      that big a deal, but not very user-friendly either.
> 
> My current thoughts about each problem:
> 
>   1. It's a bit bothersome, but on the other hand lookup_symbol is
>      used everywhere. Any change in its interface would be a jumbo
>      patch. Nevertheless, I am ready to discuss this option if you
>      feel that we should bite the bullet and avoid global variables
>      as much as we can (remember about the language problems we still
>      have to tackle?). Let me know. For now, since current_language
>      is already so much widely used, I'm inclined to simply wrap
>      the call to lookup_symbol() between calls to set_language().
> 
>   2. This can easily be fixed. I think the cleanest way to do this is
>      by renaming inside_main_func() into inside_main_func_1(), and
>      write a new inside_main_func() that calls inside_main_func_1()
>      via catch_exceptions(). We can then safely wrap it between
>      calls to set_language(). But this is assuming that we decide
>      that we don't mind about concern #1 above. But I find the
>      catch exceptions interface a bit awkward. Mixing the error code
>      returned by catch_exceptions() with the return value of the
>      function called makes me flinch a bit.
> 
> What do you think?

Don't use lookup_symbol?  On David's branch there's a minsym function
for finding the minimal symbol associated with a mangled name.  I
thought it had been merged to mainline but it hasn't.  Something to do
the same for symbols would be reasonable, although rare.  Or you could
just use the minsym version, and then call find_pc_function.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Joel Brobecker]

> [First of all, what path takes you through lookup_symbol to that
> prompt?  That shouldn't ever happen, unless it's being generated in
> Ada-specific code...]

It is generated in ada-specific code. Just FYI, here is our ada-specific
lookup function, which is hooked into the language vector:

    struct symbol *
    ada_lookup_symbol ([snip])
    { 
      [snip]
      n_candidates = ada_lookup_symbol_list (ada_mangle (ada_fold_name (name)),
                                             block0, namespace, &candidates);
      
      if (n_candidates == 0)
        return NULL;
      else if (n_candidates != 1)
        user_select_syms (candidates, n_candidates, 1);  <<<-----
      [snip]
    }

user_select_syms causes the menu to appear...


> Don't use lookup_symbol?  On David's branch there's a minsym function
> for finding the minimal symbol associated with a mangled name.  I
> thought it had been merged to mainline but it hasn't.  Something to do
> the same for symbols would be reasonable, although rare.

>   Or you could
> just use the minsym version, and then call find_pc_function.

That's a good idea, I think. Lookup the minsym.... Hmmm, let me explore
this path. Thanks!

-- 
Joel

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Daniel Jacobowitz]

On Mon, Oct 06, 2003 at 05:24:22PM -0700, Joel Brobecker wrote:
> > [First of all, what path takes you through lookup_symbol to that
> > prompt?  That shouldn't ever happen, unless it's being generated in
> > Ada-specific code...]
> 
> It is generated in ada-specific code. Just FYI, here is our ada-specific
> lookup function, which is hooked into the language vector:
> 
>     struct symbol *
>     ada_lookup_symbol ([snip])
>     { 
>       [snip]
>       n_candidates = ada_lookup_symbol_list (ada_mangle (ada_fold_name (name)),
>                                              block0, namespace, &candidates);
>       
>       if (n_candidates == 0)
>         return NULL;
>       else if (n_candidates != 1)
>         user_select_syms (candidates, n_candidates, 1);  <<<-----
>       [snip]
>     }
> 
> user_select_syms causes the menu to appear...

Oh... I see that Ada already has an interface for selecting a list of
symbols.  Another bit that doesn't belong anywhere near the
language-specific code, if you'll permit some historical ranting.  That
should be pushed out to the symtab layer.

I'll be back to that after I finish something I'm working on in
the breakpoint code, so that the list of multiple symbols is actually
useful.

> > Don't use lookup_symbol?  On David's branch there's a minsym function
> > for finding the minimal symbol associated with a mangled name.  I
> > thought it had been merged to mainline but it hasn't.  Something to do
> > the same for symbols would be reasonable, although rare.
> 
> >   Or you could
> > just use the minsym version, and then call find_pc_function.
> 
> That's a good idea, I think. Lookup the minsym.... Hmmm, let me explore
> this path. Thanks!

Follow along with the code immediately above in the same function. 
Beware, right now that minsym code may find the one which demangles to
main; but that should change.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1748 bytes --]

> Oh... I see that Ada already has an interface for selecting a list of
> symbols.  Another bit that doesn't belong anywhere near the
> language-specific code, if you'll permit some historical ranting.

I agree. My holy grail would be to push all the code doing symbol
lookups for Ada, or rather the 90% part of it that's almost a copy/paste
of the C code. But this is not something that I have been looking at,
because Paul Hilfinger was more familiar with this part and its
historical legacy.

> > >   Or you could
> > > just use the minsym version, and then call find_pc_function.
> > 
> > That's a good idea, I think. Lookup the minsym.... Hmmm, let me explore
> > this path. Thanks!
> 
> Follow along with the code immediately above in the same function. 

I don't understand you suggestion, there isn't much code above the
part I'd like to change. Maybe below?

In any case, attached is the patch that I came up with. I tested it
on x86-linux RH9.0 using the stock gcc/g++ there. No regression found.
Would it be acceptable for inclusion? I can also add a dated comments
explaining why we are doing it that way instead of using lookup_symbol.

> Beware, right now that minsym code may find the one which demangles to
> main; but that should change.

Hmmm, did I do it the right way, or did I open a bad door? In the latter
case, is there a way for me to make sure I find the right one? It seems
pretty easy to verify that I did find the right own by comparing
main_name() against the SYMBOL_LINKAGE_NAME, but that still would not
help us find the right symbol :-).

2003-10-06  J. Brobecker  <brobecker@gnat.com>

	* blockframe.c (inside_main_func): No longer use symbol_lookup()
        to lookup the main function symbol.

Thanks,
-- 
Joel

[-- Attachment #2: blockframe.c.diff --]
[-- Type: text/plain, Size: 1805 bytes --]

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.80
diff -u -p -r1.80 blockframe.c
--- blockframe.c	14 Sep 2003 16:32:12 -0000	1.80
+++ blockframe.c	7 Oct 2003 01:56:40 -0000
@@ -83,22 +83,27 @@ deprecated_inside_entry_file (CORE_ADDR 
 int
 inside_main_func (CORE_ADDR pc)
 {
+  struct minimal_symbol *msymbol;
+
   if (pc == 0)
     return 1;
   if (symfile_objfile == 0)
     return 0;
 
+  msymbol = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
+
   /* If the addr range is not set up at symbol reading time, set it up
      now.  This is for DEPRECATED_FRAME_CHAIN_VALID_ALTERNATE. I do
      this for coff, because it is unable to set it up and symbol
      reading time. */
 
-  if (symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC &&
-      symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
+  if (msymbol != NULL
+      && symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC
+      && symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
     {
-      struct symbol *mainsym;
+      struct symbol *mainsym
+        = find_pc_function (SYMBOL_VALUE_ADDRESS (msymbol));
 
-      mainsym = lookup_symbol (main_name (), NULL, VAR_DOMAIN, NULL, NULL);
       if (mainsym && SYMBOL_CLASS (mainsym) == LOC_BLOCK)
 	{
 	  symfile_objfile->ei.main_func_lowpc =
@@ -111,8 +116,6 @@ inside_main_func (CORE_ADDR pc)
   /* Not in the normal symbol tables, see if "main" is in the partial
      symbol table.  If it's not, then give up.  */
   {
-    struct minimal_symbol *msymbol
-      = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
     if (msymbol != NULL && MSYMBOL_TYPE (msymbol) == mst_text)
       {
 	struct obj_section *osect

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Daniel Jacobowitz]

On Mon, Oct 06, 2003 at 07:14:31PM -0700, Joel Brobecker wrote:
> I don't understand you suggestion, there isn't much code above the
> part I'd like to change. Maybe below?

That's what I meant, yeah.

> In any case, attached is the patch that I came up with. I tested it
> on x86-linux RH9.0 using the stock gcc/g++ there. No regression found.
> Would it be acceptable for inclusion? I can also add a dated comments
> explaining why we are doing it that way instead of using lookup_symbol.

Please do.

> > Beware, right now that minsym code may find the one which demangles to
> > main; but that should change.
> 
> Hmmm, did I do it the right way, or did I open a bad door? In the latter
> case, is there a way for me to make sure I find the right one? It seems
> pretty easy to verify that I did find the right own by comparing
> main_name() against the SYMBOL_LINKAGE_NAME, but that still would not
> help us find the right symbol :-).
> 
> 2003-10-06  J. Brobecker  <brobecker@gnat.com>
> 
> 	* blockframe.c (inside_main_func): No longer use symbol_lookup()
>         to lookup the main function symbol.

This patch looks good to me, and I can approve it, but let's give it a
little while in case anyone else has comments.  Symtab maintainers
maybe?  Is there any risk of this finding the wrong function - I can't
see any?

> Index: blockframe.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/blockframe.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 blockframe.c
> --- blockframe.c	14 Sep 2003 16:32:12 -0000	1.80
> +++ blockframe.c	7 Oct 2003 01:56:40 -0000
> @@ -83,22 +83,27 @@ deprecated_inside_entry_file (CORE_ADDR 
>  int
>  inside_main_func (CORE_ADDR pc)
>  {
> +  struct minimal_symbol *msymbol;
> +
>    if (pc == 0)
>      return 1;
>    if (symfile_objfile == 0)
>      return 0;
>  
> +  msymbol = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
> +
>    /* If the addr range is not set up at symbol reading time, set it up
>       now.  This is for DEPRECATED_FRAME_CHAIN_VALID_ALTERNATE. I do
>       this for coff, because it is unable to set it up and symbol
>       reading time. */
>  
> -  if (symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC &&
> -      symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
> +  if (msymbol != NULL
> +      && symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC
> +      && symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
>      {
> -      struct symbol *mainsym;
> +      struct symbol *mainsym
> +        = find_pc_function (SYMBOL_VALUE_ADDRESS (msymbol));
>  
> -      mainsym = lookup_symbol (main_name (), NULL, VAR_DOMAIN, NULL, NULL);
>        if (mainsym && SYMBOL_CLASS (mainsym) == LOC_BLOCK)
>  	{
>  	  symfile_objfile->ei.main_func_lowpc =
> @@ -111,8 +116,6 @@ inside_main_func (CORE_ADDR pc)
>    /* Not in the normal symbol tables, see if "main" is in the partial
>       symbol table.  If it's not, then give up.  */
>    {
> -    struct minimal_symbol *msymbol
> -      = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
>      if (msymbol != NULL && MSYMBOL_TYPE (msymbol) == mst_text)
>        {
>  	struct obj_section *osect


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Daniel Jacobowitz]

On Mon, Oct 06, 2003 at 07:14:31PM -0700, Joel Brobecker wrote:
> 2003-10-06  J. Brobecker  <brobecker@gnat.com>
> 
> 	* blockframe.c (inside_main_func): No longer use symbol_lookup()
>         to lookup the main function symbol.

This is OK.

> Index: blockframe.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/blockframe.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 blockframe.c
> --- blockframe.c	14 Sep 2003 16:32:12 -0000	1.80
> +++ blockframe.c	7 Oct 2003 01:56:40 -0000
> @@ -83,22 +83,27 @@ deprecated_inside_entry_file (CORE_ADDR 
>  int
>  inside_main_func (CORE_ADDR pc)
>  {
> +  struct minimal_symbol *msymbol;
> +
>    if (pc == 0)
>      return 1;
>    if (symfile_objfile == 0)
>      return 0;
>  
> +  msymbol = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
> +
>    /* If the addr range is not set up at symbol reading time, set it up
>       now.  This is for DEPRECATED_FRAME_CHAIN_VALID_ALTERNATE. I do
>       this for coff, because it is unable to set it up and symbol
>       reading time. */
>  
> -  if (symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC &&
> -      symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
> +  if (msymbol != NULL
> +      && symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC
> +      && symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
>      {
> -      struct symbol *mainsym;
> +      struct symbol *mainsym
> +        = find_pc_function (SYMBOL_VALUE_ADDRESS (msymbol));
>  
> -      mainsym = lookup_symbol (main_name (), NULL, VAR_DOMAIN, NULL, NULL);
>        if (mainsym && SYMBOL_CLASS (mainsym) == LOC_BLOCK)
>  	{
>  	  symfile_objfile->ei.main_func_lowpc =
> @@ -111,8 +116,6 @@ inside_main_func (CORE_ADDR pc)
>    /* Not in the normal symbol tables, see if "main" is in the partial
>       symbol table.  If it's not, then give up.  */
>    {
> -    struct minimal_symbol *msymbol
> -      = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
>      if (msymbol != NULL && MSYMBOL_TYPE (msymbol) == mst_text)
>        {
>  	struct obj_section *osect


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] lookup problem in blockframe.c:inside_main_func()

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 332 bytes --]

> > 2003-10-06  J. Brobecker  <brobecker@gnat.com>
> > 
> > 	* blockframe.c (inside_main_func): No longer use symbol_lookup()
> >         to lookup the main function symbol.
> 
> This is OK.

Thank you Daniel. Attached is what I ended up commiting (added the dated
comment - let me know if you'd like to see it reworked).

-- 
Joel

[-- Attachment #2: blockframe.c.diff --]
[-- Type: text/plain, Size: 2365 bytes --]

Index: blockframe.c
===================================================================
RCS file: /cvs/src/src/gdb/blockframe.c,v
retrieving revision 1.80
diff -u -p -r1.80 blockframe.c
--- blockframe.c	14 Sep 2003 16:32:12 -0000	1.80
+++ blockframe.c	10 Oct 2003 20:21:02 -0000
@@ -83,22 +83,35 @@ deprecated_inside_entry_file (CORE_ADDR 
 int
 inside_main_func (CORE_ADDR pc)
 {
+  struct minimal_symbol *msymbol;
+
   if (pc == 0)
     return 1;
   if (symfile_objfile == 0)
     return 0;
 
+  msymbol = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
+
   /* If the addr range is not set up at symbol reading time, set it up
      now.  This is for DEPRECATED_FRAME_CHAIN_VALID_ALTERNATE. I do
      this for coff, because it is unable to set it up and symbol
      reading time. */
 
-  if (symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC &&
-      symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
+  if (msymbol != NULL
+      && symfile_objfile->ei.main_func_lowpc == INVALID_ENTRY_LOWPC
+      && symfile_objfile->ei.main_func_highpc == INVALID_ENTRY_HIGHPC)
     {
-      struct symbol *mainsym;
+      /* brobecker/2003-10-10: We used to rely on lookup_symbol() to search
+         the symbol associated to the main function.  Unfortunately,
+         lookup_symbol() uses the current-language la_lookup_symbol_nonlocal
+         function to do the global symbol search.  Depending on the language,
+         this can introduce certain side-effects, because certain languages
+         such as Ada for instance may find more than one match.  So we prefer
+         to search the main function symbol using its address rather than
+         its name.  */
+      struct symbol *mainsym
+        = find_pc_function (SYMBOL_VALUE_ADDRESS (msymbol));
 
-      mainsym = lookup_symbol (main_name (), NULL, VAR_DOMAIN, NULL, NULL);
       if (mainsym && SYMBOL_CLASS (mainsym) == LOC_BLOCK)
 	{
 	  symfile_objfile->ei.main_func_lowpc =
@@ -111,8 +124,6 @@ inside_main_func (CORE_ADDR pc)
   /* Not in the normal symbol tables, see if "main" is in the partial
      symbol table.  If it's not, then give up.  */
   {
-    struct minimal_symbol *msymbol
-      = lookup_minimal_symbol (main_name (), NULL, symfile_objfile);
     if (msymbol != NULL && MSYMBOL_TYPE (msymbol) == mst_text)
       {
 	struct obj_section *osect