[PATCH] Fix infinite recursion bug at get_msymbol_address.

[Simon Marchi]

On 2019-11-06 11:05 p.m., Ali Tamur via gdb-patches wrote:
> The patch 4b610737f0 seems to have introduced the possibility of infinite
> recursion. I have encountered the problem while debugging a failing in-house
> test. I am sorry, it is fairly difficult to reduce the test case (and I don't
> understand most of what is going on) but the stack trace shows a call to
> objfpy_add_separate_debug_file, which eventually causes
> lookup_minimal_symbol_by_pc_name to be invoked, which calls get_msymbol_address.
> Somehow lookup_minimal_symbol_linkage finds the same symbol and the function
> calls itself with the same parameters. I don't know whether this should be
> classified as 'it should never happen', but this simple patch makes the test
> pass and should be harmless, I think.
> 
> gdb/ChangeLog
> 
> 	* symtab.c (get_msymbol_address): Guard against infinite recursion.
> ---
>  gdb/symtab.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 2c934b9c22..b231cc6e84 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -6328,7 +6328,7 @@ get_msymbol_address (struct objfile *objf, const struct minimal_symbol *minsym)
>  	{
>  	  bound_minimal_symbol found
>  	    = lookup_minimal_symbol_linkage (linkage_name, objfile);
> -	  if (found.minsym != nullptr)
> +	  if (found.minsym != nullptr && found.minsym != minsym)
>  	    return BMSYMBOL_VALUE_ADDRESS (found);
>  	}
>      }

Hi Ali,

At the top of the function, we assert that the objfile associated to the minsym
is not the main one:

  gdb_assert ((objf->flags & OBJF_MAINLINE) == 0);

We then iterate on all objfiles, looking only for main objfiles (presumably
there's only one).  We look up a minsym with the same name as our original minsym in
the main objfile.

How could our original minsym (which is not supposed to come from the main objfile,
I believe) could be the same one as the minsym found when looking up in the main
objfile?

If this is indeed not supposed to happen, I think a fix like this would
just paper over the real problem.  Or maybe you have a legitimate use case that
wasn't expected.  You've talked about objfpy_add_separate_debug_file, so clearly your
use case involves separate debug files.  It's possible that we are passing the objfile
representing the separate debug file of the main objfile or something like that.

I'd like if we could get a reproducer before we commit a fix we are not sure to
understand.  Can you give the backtrace, maybe we'll get some idea?

Also, could you provide as much info about your use case as possible to try to guide
us in finding the root issue?

Simon