From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id vtZmCIBUm2gTmgEAWB0awg (envelope-from ) for ; Tue, 12 Aug 2025 10:49:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1755010176; bh=4pKp1dFR2RqlgU63EqLV0ndZBSmTaXj0E9iTqsmW1lM=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=LkzRgPwhrYc9VVZibDg4Zc95zJiUAOdHPbo+IUWloyGaFYFAfWyhBqCHmznqO3CnW 4CAPSA7fv51pXUnukVCD4Oi42xeKyIwzMEQV/TPFwiULOLJnRPCl9RH+Lr82FjZEL1 WBfkCSxdTLjuUJqOX5MpG+FeakjRnLa25QWw6C9w= Received: by simark.ca (Postfix, from userid 112) id 11BD41E04C; Tue, 12 Aug 2025 10:49:36 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=ZunC9FyO; dkim-atps=neutral Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 586411E04C for ; Tue, 12 Aug 2025 10:49:34 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BC91E3858D37 for ; Tue, 12 Aug 2025 14:49:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BC91E3858D37 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=ZunC9FyO Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id B0FD03858D37 for ; Tue, 12 Aug 2025 14:48:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B0FD03858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B0FD03858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1755010134; cv=none; b=EkoDwGUvDcHQRVJ1vTVqamEL4RNUz6MpbGZgftu75TCKX54fdn1htXkFKcAGWAKHHrZ2FbFu/QTDtwGbrGQk4lrTwF8DxwcrzzaX7FlfzVpa7mHk+7h8pjR77MyYy/4S9qhOcOXJWR3luhiiGdJZisOM8EhQ9tH7Of9/5ZUOX28= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1755010134; c=relaxed/simple; bh=4pKp1dFR2RqlgU63EqLV0ndZBSmTaXj0E9iTqsmW1lM=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=waVqbsw9GJpl+59lXNTq/5iLh6I5ko5FgHVuTkHcRaxcK8FmOWPp6mc043jWhsvH6BttTQey2WTwHX7FdHlNcumoxxYoZwxv3hRufftA9pYEfqQ82yMtUY/HOpOWhsbvEoFuFoXZ30i1dEGCQLKxnSnHI8s4882Ovcm7WfJqT4Q= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B0FD03858D37 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1755010134; bh=4pKp1dFR2RqlgU63EqLV0ndZBSmTaXj0E9iTqsmW1lM=; h=Date:Subject:To:References:From:In-Reply-To:From; b=ZunC9FyOaqt1x6Th5d/vq/IDlILGmkV+atOYOTV3uPr5WVJvlCiLQPlq1aXzKVvJ+ nHwV6SpObtwF7eURGf27RrjfFWMEyVsSjbOM9hw6LFVCMdBugJ7LrCDEXbu8JjPgO/ BAZpxd+tcwHDWP9rVIXHujemyFKDOF2nF6S5G5U4= Received: by simark.ca (Postfix) id 520D61E04C; Tue, 12 Aug 2025 10:48:54 -0400 (EDT) Message-ID: <7bcde745-f700-4f64-88c2-83a359ce37a5@simark.ca> Date: Tue, 12 Aug 2025 10:48:23 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] gdb/dwarf2: Add symbols for function declarations To: Kevin Buettner , gdb-patches@sourceware.org References: <20250703194719.2254338-1-kevinb@redhat.com> Content-Language: fr From: Simon Marchi In-Reply-To: <20250703194719.2254338-1-kevinb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org 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 = () > (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

> FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0: > final_debug=0: gdb-command

> 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 > > 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 ".) > > 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 > 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