Re: [PATCH v2] gdb/dwarf2: Add symbols for function declarations

[Simon Marchi <simark@simark.ca>]

On 7/3/25 3:45 PM, Kevin Buettner wrote:
> This commit was motivated by comments 3 and 4 for bug 31563:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=31563#c3
> 
> When a program is built with -g3, macro information is available to
> GDB; for errno, the macro defined in /usr/include/errno.h (provided by
> GLIBC) looks like this:
> 
>     # define errno (*__errno_location ())
> 
> However, up to now, GDB doesn't know the type of __errno_location,
> despite (sometimes) having a DIE representing a declaration providing
> its type.  In any case, apparently not knowing the return type of
> __errno_location, GDB was unable to perform the inferior function call
> specified by the errno macro:
> 
>     (gdb) p errno
>     '__errno_location' has unknown return type; cast the call to its
>     declared return type
> 
> But, for some compilers, GDB *should* be able to know the type.  These
> are the DIEs related to the __errno_location declaration from the
> "macros" case for the gdb.base/errno.exp test:
> 
>  <1><37>: Abbrev Number: 2 (DW_TAG_subprogram)
>     <38>   DW_AT_external    : 1
>     <38>   DW_AT_name        : (indirect string, offset: 0x20e4):
>                                __errno_location
>     <3c>   DW_AT_decl_file   : 2
>     <3d>   DW_AT_decl_line   : 37
>     <3e>   DW_AT_decl_column : 13
>     <3f>   DW_AT_prototyped  : 1
>     <3f>   DW_AT_type        : <0x43>
>     <43>   DW_AT_declaration : 1
>  <1><43>: Abbrev Number: 3 (DW_TAG_pointer_type)
>     <44>   DW_AT_byte_size   : 8
>     <45>   DW_AT_type        : <0x49>
>  <1><49>: Abbrev Number: 4 (DW_TAG_base_type)
>     <4a>   DW_AT_byte_size   : 4
>     <4b>   DW_AT_encoding    : 5	(signed)
>     <4c>   DW_AT_name        : int
> 
> If you wish to see this for yourself, from your gdb build directory,
> do:
> 
>     make check TESTS=gdb.base/errno.exp
>     readelf -w testsuite/outputs/gdb.base/errno/errno-macros | less
> 
> With this commit in place, using gcc as the C compiler, 8 XFAILs in
> gdb.base/errno.exp turn into PASSes.  They are:
> 
>     XFAIL: gdb.base/errno.exp: macros: print (int) errno
>     XFAIL: gdb.base/errno.exp: macros: print errno
>     XFAIL: gdb.base/errno.exp: pthreads-macros: print (int) errno
>     XFAIL: gdb.base/errno.exp: pthreads-macros: print errno
>     XFAIL: gdb.base/errno.exp: pthreads-static-macros: print (int) errno
>     XFAIL: gdb.base/errno.exp: pthreads-static-macros: print errno
>     XFAIL: gdb.base/errno.exp: static-macros: print (int) errno
>     XFAIL: gdb.base/errno.exp: static-macros: print errno
> 
> For the example shown earlier, GDB is now able to print the correct
> value for errno.
> 
> As mentioned earlier, it doesn't work for all compliers.  In
> particular, when clang is used instead, there's (currently) no change
> in results in the errno.exp test since clang doesn't provide the
> necessary declaration(s) in its DWARF output.
> 
> Perhaps even more compelling is being able to call functions like
> malloc() without having debug info for the C library.  To demonstrate
> this, I'll use the test program from gdb.base/break.exp.  After
> starting the program (and not letting debuginfod fetch GLIBC's
> symbols), an unpatched GDB will show:
> 
>     (gdb) ptype malloc
>     type = <unknown return type> ()
>     (gdb) p malloc(4)
>     'malloc' has unknown return type; cast the call to its declared
>     return type
> 
> However, with this commit, we now see:
> 
>     (gdb) ptype malloc
>     type = void *(unsigned long)
>     (gdb) p malloc(4)
>     $1 = (void *) 0x4042a0
> 
> This commit changes the name of read_func_scope in gdb/dwarf2/read.c
> to read_func_scope_or_decl, changing all callers.  I also added a
> comment for this function.
> 
> It introduces a new function, die_is_func_decl_p and uses it in
> read_func_scope_or_decl().  If the call to die_is_func_decl_p()
> returns true, the code in read_func_scope_or_decl which attempts to
> get the function bounds is skipped and, after existing code which
> attempts to do some template related stuff happens, a new symbol with
> address class LOC_UNRESOLVED will be added.
> 
> If just this change alone is made and regression testing is performed,
> there are quite a few regressions (well over 50, as I recall), mostly
> due to the fact that the PLT symbol / declaration is now found in
> various cases, perhaps ahead of the symbol for the function
> definition.  I'll go into depth regarding the various cases, below.
> 
> Many of the regressions were fixed by making the LOC_UNRESOLVED case
> in language_defn::read_var_value in gdb/findvar.c prefer "normal"
> symbols over PLT symbols, though the PLT symbol will be used if no
> normal symbol is found.
> 
> This change contains a (perhaps) surprising addition to deal with GNU
> ifunc symbols:
> 
> 			if (bmsym.minsym->type () == mst_text_gnu_ifunc)
> 			  {
> 			    /* GNU ifunc code elsewhere in GDB depends
> 			       on the symbol's type being set as shown
> 			       below.  But, coming into this function,
> 			       VAR might have an arguably better type
> 			       obtained from a declaration, i.e.
> 			       DW_AT_declaration.  In this case, the
> 			       PLT (solib trampoline) symbol is
> 			       usually found first; see above.
> 			       Nevertheless, we change the type to
> 			       what the rest of GDB expects in order
> 			       for the rest of the GNU ifunc related
> 			       code in GDB to work.  */
> 			    type = builtin_type (objfile)
> 				     ->nodebug_text_gnu_ifunc_symbol;
> 			  }
> 
> Hopefully, the comment adequately describes what this is about, but
> I'll note that without this particular bit of code, we see the
> following GNU ifunc related failures:
> 
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: gdb-command<p (int) gnu_ifunc (3)>
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: gdb-command<p gnu_ifunc (3)>
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: p gnu_ifunc executing
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: p gnu_ifunc()
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: resolver received HWCAP
> 
> There are 17 more, but they're essentially repeats of the above, with
> varying resolver_attr, resolver_debug, and final_debug cases.
> 
> The change to info_address_command in gdb/printcmd.c forces execution
> into the minimal symbol lookup case when presented with a
> LOC_UNRESOLVED function symbol.  Without this change, there were 12
> falures in gdb.base/gnu-ifunc.exp, two of which look like this:
> 
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: info addr gnu_ifunc
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: info sym <gnu_ifunc-address>
> 
> The remaining failures are similar, only differing in the values
> for resolver_attr, resolver_debug, and final_debug.
> 
> With regard to the failure itself, for the first one, the log output
> looks like this:
> 
>     info addr gnu_ifunc
>     Symbol "gnu_ifunc" is static storage at address 0x7ffff7fbb389.
>     (gdb) FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0:
>       final_debug=0: info addr gnu_ifunc
> 
> The expected message from "info addr gnu_ifunc" was:
> 
>     Symbol "gnu_ifunc" is at 0x7ffff7fbb389 in a file compiled without
>     debugging.
> 
> I don't think that the FAILing message is wrong, but I think that the
> PASSing message (regarding being in a file without debugging) is more
> helpful to the user.
> 
> It bothered me that the only tests which caught this problem were
> in gdb.base/gnu-ifunc.exp.  There is now an "info addr foo" test
> in the new test case gdb.dwarf2/func-decl.exp which also performs
> this test.
> 
> With the above change in place, we then see these failures:
> 
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1:
>       final_debug=0: info addr gnu_ifunc
>     FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1:
>       final_debug=1: info addr gnu_ifunc
> 
> (There are two others for "info sym <gnu_ifunc-address>".)
> 
> In each case, we now see a message like this...
> 
>     Symbol "gnu_ifunc" is at 0x7ffff7fbb389 in a file compiled without
>     debugging.
> 
> ...when we should in fact see:
> 
>     Symbol "gnu_ifunc" is a function at address 0x7ffff7fbb389.
> 
> Note that this is for the resolver_debug=1 case; for this case, the
> resolver library has symbols, so the latter message makes sense and
> the "failing" message is just plain wrong.
> 
> These new failures are fixed by the change to
> lookup_global_or_static_symbol in gdb/symtab.c.  In this change,
> normal function symbols are preferred to those whose address class is
> LOC_UNRESOLVED.  I used a similar approach to that for
> language_defn::read_var_value, discussed earlier.
> 
> Again, it seemed to me that there should be a non-gnu-ifunc test
> for this, so I added one; it'll be tested by:
> 
>     gdb.dwarf2/func-decl.exp: lib_debug: info addr foo
> 
> There were also regressions in gdb.base/info-fun.exp and
> gdb.mi/mi-sym-info.exp:
> 
>     FAIL: gdb.base/info-fun.exp: n_flag=0: IN: info fun  foo
>     FAIL: gdb.base/info-fun.exp: n_flag=0: NO: info fun  foo
>     FAIL: gdb.base/info-fun.exp: n_flag=0: SEP: info fun  foo
>     FAIL: gdb.base/info-fun.exp: n_flag=1: IN: info fun -n foo
>     FAIL: gdb.base/info-fun.exp: n_flag=1: NO: info fun -n foo
>     FAIL: gdb.base/info-fun.exp: n_flag=1: SEP: info fun -n foo
>     FAIL: gdb.mi/mi-sym-info.exp: List all functions matching pattern f3
>       (unexpected output)
> 
> For each of these failures, there was more output than expected.  For
> example, for one of the failing cases...
> 
>     (gdb) info fun  foo
>     All functions matching regular expression "foo":
> 
>     File .../gdb/testsuite/gdb.base/info-fun.c:
>     16:	int foo(void);
> 
>     Non-debugging symbols:
>     0x0000000000400370  foo@plt
>     0x00007ffff7fbb389  foo
>     (gdb) FAIL: gdb.base/info-fun.exp: n_flag=0: NO: info fun  foo
> 
> The "passing" output looks like this:
> 
>     (gdb) info fun  foo
>     All functions matching regular expression "foo":
> 
>     Non-debugging symbols:
>     0x0000000000400370  foo@plt
>     0x00007ffff7fbb389  foo
>     (gdb) PASS: gdb.base/info-fun.exp: n_flag=0: NO: info fun  foo
> 
> At first glance, the "failing" output looks useful; perhaps it could
> be, but I'll note that the extra lines being output are for a
> declaration for a function which is not in the CU where the function
> is defined.  I have a hunch that we might be overwhelmed by extra
> output in a program with many libraries - it's conceivable that for
> some symbols, each library would have its own declaration.
> 
> In any case, I was able to obtain the original / passing behavior
> by discarding LOC_UNRESOLVED symbols when searching in the
> function domain in global_symbol_searcher::add_matching_symbols.
> 
> Finally, there were two regressions in gdb.base/completion.exp:
> 
>     FAIL: gdb.base/completion.exp: complete break break.c:ma
>     FAIL: gdb.base/completion.exp: tab complete break break.c:ma (timeout)
> 
> The log file for these failing tests is not especially helpful, but I
> debugged it by throwing a "gdb_interact" into the test to see what
> was going on.  As I recall, when trying to complete "break.c:ma",
> "marker1", "marker2", "marker3", "marker4", and "malloc" were all
> being found in addition to "main", which is what the what the testcase
> was expecting to be the sole completion.
> 
> This problem was fixed by adjusting completion_skip_symbol in
> symtab.h.
> 
> The new test case, gdb.dwarf2/func-decl.exp, contains, in addition to
> the tests already discussed, two tests which will fail in a GDB built
> without this commit and pass in a GDB built with it...
> 
>     PASS: gdb.dwarf2/func-decl.exp: no_lib_debug:
>       gdb-command<print foo ("abc", 5)>
>     PASS: gdb.dwarf2/func-decl.exp: no_lib_debug: ptype foo
> 
> The remaining tests in gdb.dwarf2/func-decl.exp should all pass in a
> GDB built with or without this commit.  They will only fail if one of
> the relevant changes discussed above is missing or becomes broken for
> some reason (perhaps due to some future change to this area of the
> code).
> 
> Regarding the use of the DWARF assembler in the test...  Using some
> version(s) of GNU C, it's possible to write a test which causes a
> suitable declaration DIE to be placed in the DWARF output.  In fact, I
> originally wrote most of the new test without the DWARF assembler.
> But not all compilers do this, e.g. clang does not, and I wanted a
> test which would test this functionality regardless of whether the
> compiler generates the DWARF required for this test.
> 
> I've tested on Fedora 42 w/ architectures x86_64, aarch64, riscv,
> s390x, and ppc64le.  On x86_64 Fedora 42, I've also tested with
> --target_board=unix/-m32, --target_board=native-gdbserver, and
> --target_board=native-extended-gdbserver.  No regressions found.
> 
> After skimming version 1 of this commit, Tom Tromey suggested that
> there should also be changes to the indexer.  This version 2 commit
> adds that by making DW_TAG_subprogram declarations "interesting" to
> the indexer.  The changes which do this are in gdb/dwarf2/abbrev.c
> and gdb/dwarf2/cooked-indexer.c.  I also added a test to the new
> test case which attempts to do "ptype foo" prior to starting the
> program.  This failed when using version 1 of this commit, but
> passes now.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31563

Wow, this is a lot of info to unpack, I read through it and haven't
recovered yet :P.  I will just look at the test case for now, and I
might do a later pass with a fresh head.

I just have one minor suggestion in the test:

> +# Test against a shared library built with no debugging symbols.  Due
> +# to the DWARF info provided by the DWARF assembler above, there will
> +# be a declaration for the shared lib symbol "foo" in the main
> +# program.  Thus, due to the lack of DWARF info in the shared library,
> +# GDB can't know the type from the shared library.  Instead, it must
> +# rely on the declaration of foo from the main program.
> +#
> +# Due to that declaration, it should be possible to examine its type
> +# as well as make an inferior function call.  We expect "info addr foo"
> +# to provide the address of the actual function instead of foo's PLT
> +# in the main program.
> +
> +with_test_prefix no_lib_debug {
> +    if { [gdb_compile_shlib $libsrc $libobj [list nodebug]] != "" } {
> +	untested "failed to compile shared object"
> +	return -1
> +    }
> +
> +    if { [prepare_for_testing "failed to prepare" ${binfile} \
> +	      [list $srcfile $asm_file] [list nodebug shlib=${libobj}]] } {
> +	return -1
> +    }
> +
> +    with_test_prefix "before program start" {
> +	# Verify that the type of foo is available prior to starting
> +	# the program.
> +	gdb_test "ptype foo" "^type = int \\(char \\*, int\\)"
> +    }
> +
> +    clean_restart $binfile
> +
> +    if ![runto_main] {
> +	return
> +    }
> +
> +    gdb_test "ptype foo" "^type = int \\(char \\*, int\\)"
> +    gdb_test "print foo \(\"abc\", 5\)" "= 8"
> +    gdb_test "info addr foo" "Symbol \"foo\" is at $::hex in a file compiled without debugging\\."
> +}
> +
> +# Test again with a library built with debugging symbols.  The
> +# "info addr foo" test can fail if PLT symbols are preferred over
> +# normal symbols when looking up a global or static symbol.
> +
> +with_test_prefix lib_debug {
> +    set binfile $binfile-debug
> +    set libobj [standard_output_file "${testfile}-lib-debug.so"]
> +
> +    if { [gdb_compile_shlib $libsrc $libobj [list debug]] != "" } {
> +	untested "failed to compile shared object"
> +	return -1
> +    }
> +
> +    if { [prepare_for_testing "failed to prepare" ${binfile} \
> +	      [list $srcfile $asm_file] [list nodebug shlib=${libobj}]] } {
> +	return -1
> +    }
> +
> +    if ![runto_main] {
> +	return
> +    }
> +
> +    gdb_test "info addr foo" "Symbol \"foo\" is a function at address $::hex\\."
> +}

I would suggest doing a:

foreach_with_prefix lib_debug {nodebug debug} {
   ...
}

to test both scenarios.  There will be less duplication, and all
commands will get tested in both modes, for free.  I think you will just
need one if/else for the "info addr foo" output.

Simon