public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
@ 2021-11-12 17:16 Tom de Vries
  2021-11-12 17:16 ` [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose Tom de Vries
  2021-11-13 14:51 ` [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Simon Marchi
  0 siblings, 2 replies; 10+ messages in thread
From: Tom de Vries @ 2021-11-12 17:16 UTC (permalink / raw)
  To: gdb-patches

PR28539 describes a segfault in lambda function search_one_symtab due to
psymbol_functions::expand_symtabs_matching calling expansion_notify with a
nullptr symtab:
...
          struct compunit_symtab *symtab =
            psymtab_to_symtab (objfile, ps);

          if (expansion_notify != NULL)
            if (!expansion_notify (symtab))
              return false;
...

This happens as follows.  The partial symtab ps is a dwarf2_include_psymtab
for some header file:
...
(gdb) p ps.filename
$5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h"
...

The includer of ps is a shared symtab for a partial unit, with as user:
...
(gdb) p ps.includer().user.filename
$11 = 0x64fc9f0 \
  "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp"
...

The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently
the shared symtab), but returns nullptr because:
...
struct dwarf2_include_psymtab : public partial_symtab
{
  ...
  compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
  {
    return nullptr;
  }
...

Fix this by returning the Decl.cpp symtab instead, which fixes the segfault
in the PR.

While trying to write a reproducer for this, I realized that this is difficult
because not all callers of psymbol_functions::expand_symtabs_matching have an
expansion_notify.  Consequently, I decided to add this assert:
...
          struct compunit_symtab *symtab =
            psymtab_to_symtab (objfile, ps);

+         gdb_assert (symtab != nullptr);
+
          if (expansion_notify != NULL)
            if (!expansion_notify (symtab))
              return false;
...
which without the fix triggers in a few test-cases, f.i.:
...
(gdb) maint expand-symtab dw2-symtab-includes.h^M
psymtab.c:1155: internal-error: virtual bool \
  psymbol_functions::expand_symtabs_matching(...): \
  Assertion `symtab != nullptr' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
FAIL: gdb.dwarf2/dw2-symtab-includes.exp: \
  maint expand-symtab dw2-symtab-includes.h (GDB internal error)
...

I also realized that with the assert fixed, it becomes possible to implement
a "maint expand-symtabs -verbose".

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539
---
 gdb/dwarf2/read.c | 5 ++++-
 gdb/psymtab.c     | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ed101237587..b59c638b2eb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5787,7 +5787,10 @@ struct dwarf2_include_psymtab : public partial_symtab
 
   compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
   {
-    return nullptr;
+    compunit_symtab *cust = includer ()->get_compunit_symtab (objfile);
+    while (cust != nullptr && cust->user != nullptr)
+      cust = cust->user;
+    return cust;
   }
 
 private:
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 7ffb7437785..e09537d8f5e 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1152,6 +1152,8 @@ psymbol_functions::expand_symtabs_matching
 	  struct compunit_symtab *symtab =
 	    psymtab_to_symtab (objfile, ps);
 
+	  gdb_assert (symtab != nullptr);
+
 	  if (expansion_notify != NULL)
 	    if (!expansion_notify (symtab))
 	      return false;

base-commit: 1f28b70def1bea937fb9227c8346657016168456
-- 
2.26.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose
  2021-11-12 17:16 [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Tom de Vries
@ 2021-11-12 17:16 ` Tom de Vries
  2022-01-03 14:53   ` [PING][PATCH " Tom de Vries
  2022-01-10  2:34   ` [PATCH " Simon Marchi
  2021-11-13 14:51 ` [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Simon Marchi
  1 sibling, 2 replies; 10+ messages in thread
From: Tom de Vries @ 2021-11-12 17:16 UTC (permalink / raw)
  To: gdb-patches

Add a -verbose option to command "maint expand-symtabs", such that we
have (abbreviating /home/abuild/rpmbuild/BUILD to $BUILD):
...
$ gdb -q -batch ./outputs/gdb.dwarf2/dw2-symtab-includes/dw2-symtab-includes \
  -ex "maint expand-symtabs -verbose"
Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/crtn.S at 0x26c9f00
Expanded symtab for file <unknown> at 0x26ca1e0
Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/crti.S at 0x26ca540
Expanded symtab for file $BUILD/glibc-2.34/csu/static-reloc.c at 0x26ca860
Expanded symtab for file $BUILD/glibc-2.34/csu/init.c at 0x2716890
Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86/abi-note.c at 0x2708a90
Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/start.S at 0x2717810
...

Note that the <unknown> corresponds to a CU without name:
...
 <0><963>: Abbrev Number: 2 (DW_TAG_compile_unit)
    <964>   DW_AT_language    : 2       (non-ANSI C)
    <965>   DW_AT_stmt_list   : 0x1e0
 <1><969>: Abbrev Number: 3 (DW_TAG_imported_unit)
...

Tested on x86_64-linux.
---
 gdb/doc/gdb.texinfo                              |  5 +++--
 gdb/symmisc.c                                    | 14 +++++++++++++-
 gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp |  3 ++-
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fc8e5bdf3db..fbf6e8549e1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39159,10 +39159,11 @@ Use this to check, for example, whether a symbol is in one but not the other.
 Check the consistency of currently expanded symtabs.
 
 @kindex maint expand-symtabs
-@item maint expand-symtabs [@var{regexp}]
+@item maint expand-symtabs @r{[}-verbose@r{]} [@var{regexp}]
 Expand symbol tables.
 If @var{regexp} is specified, only expand symbol tables for file
-names matching @var{regexp}.
+names matching @var{regexp}.  If @code{-verbose} is passed, mentions
+expanded symbol tables.
 
 @kindex maint set catch-demangler-crashes
 @kindex maint show catch-demangler-crashes
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index e38ceb6bda1..7597487201e 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -904,6 +904,7 @@ maintenance_check_symtabs (const char *ignore, int from_tty)
 static void
 maintenance_expand_symtabs (const char *args, int from_tty)
 {
+  bool verbose = args != nullptr && check_for_argument (&args, "-verbose");
   char *regexp = NULL;
 
   /* We use buildargv here so that we handle spaces in the regexp
@@ -923,6 +924,17 @@ maintenance_expand_symtabs (const char *args, int from_tty)
   if (regexp)
     re_comp (regexp);
 
+  auto on_expansion = [&] (compunit_symtab *symtab)
+  {
+    if (verbose)
+      printf_filtered ("Expanded symtab for file %s at %s\n",
+		       symtab_to_fullname (COMPUNIT_FILETABS (symtab)),
+		       host_address_to_string (symtab));
+
+    /* Keep going.  */
+    return true;
+  };
+
   for (struct program_space *pspace : program_spaces)
     for (objfile *objfile : pspace->objfiles ())
       objfile->expand_symtabs_matching
@@ -934,7 +946,7 @@ maintenance_expand_symtabs (const char *args, int from_tty)
 	 },
 	 NULL,
 	 NULL,
-	 NULL,
+	 on_expansion,
 	 SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
 	 UNDEF_DOMAIN,
 	 ALL_DOMAIN);
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
index 5eecc8f58ef..a97c5328f7c 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
@@ -70,7 +70,8 @@ if { [readnow] } {
 gdb_test_no_output "maint info symtabs" $test
 
 # Expand dw2-symtab-includes.h symtab
-gdb_test "maint expand-symtab dw2-symtab-includes.h"
+gdb_test "maint expand-symtab -verbose dw2-symtab-includes.h" \
+    "Expanded symtab for file <unknown> at $hex"
 
 # Check that there are includes.
 gdb_test "maint info symtabs" \
-- 
2.26.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
  2021-11-12 17:16 [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Tom de Vries
  2021-11-12 17:16 ` [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose Tom de Vries
@ 2021-11-13 14:51 ` Simon Marchi
  2021-11-15 12:58   ` Tom de Vries
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-11-13 14:51 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches



On 2021-11-12 12:16, Tom de Vries via Gdb-patches wrote:
> PR28539 describes a segfault in lambda function search_one_symtab due to
> psymbol_functions::expand_symtabs_matching calling expansion_notify with a
> nullptr symtab:
> ...
>           struct compunit_symtab *symtab =
>             psymtab_to_symtab (objfile, ps);
> 
>           if (expansion_notify != NULL)
>             if (!expansion_notify (symtab))
>               return false;
> ...
> 
> This happens as follows.  The partial symtab ps is a dwarf2_include_psymtab
> for some header file:
> ...
> (gdb) p ps.filename
> $5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h"
> ...
> 
> The includer of ps is a shared symtab for a partial unit, with as user:
> ...
> (gdb) p ps.includer().user.filename
> $11 = 0x64fc9f0 \
>   "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp"
> ...
> 
> The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently
> the shared symtab), but returns nullptr because:
> ...
> struct dwarf2_include_psymtab : public partial_symtab
> {
>   ...
>   compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
>   {
>     return nullptr;
>   }
> ...
> 
> Fix this by returning the Decl.cpp symtab instead, which fixes the segfault
> in the PR.
> 
> While trying to write a reproducer for this, I realized that this is difficult
> because not all callers of psymbol_functions::expand_symtabs_matching have an
> expansion_notify.  Consequently, I decided to add this assert:
> ...
>           struct compunit_symtab *symtab =
>             psymtab_to_symtab (objfile, ps);
> 
> +         gdb_assert (symtab != nullptr);
> +
>           if (expansion_notify != NULL)
>             if (!expansion_notify (symtab))
>               return false;
> ...
> which without the fix triggers in a few test-cases, f.i.:
> ...
> (gdb) maint expand-symtab dw2-symtab-includes.h^M
> psymtab.c:1155: internal-error: virtual bool \
>   psymbol_functions::expand_symtabs_matching(...): \
>   Assertion `symtab != nullptr' failed.^M
> A problem internal to GDB has been detected,^M
> further debugging may prove unreliable.^M
> FAIL: gdb.dwarf2/dw2-symtab-includes.exp: \
>   maint expand-symtab dw2-symtab-includes.h (GDB internal error)
> ...
> 
> I also realized that with the assert fixed, it becomes possible to implement
> a "maint expand-symtabs -verbose".
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539

Have you tried writing a test for this?

Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
  2021-11-13 14:51 ` [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Simon Marchi
@ 2021-11-15 12:58   ` Tom de Vries
  2021-11-22 19:29     ` [PING][PATCH " Tom de Vries
  2021-11-29 13:51     ` [PATCH " Simon Marchi
  0 siblings, 2 replies; 10+ messages in thread
From: Tom de Vries @ 2021-11-15 12:58 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

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

On 11/13/21 3:51 PM, Simon Marchi wrote:
> 
> 
> On 2021-11-12 12:16, Tom de Vries via Gdb-patches wrote:
>> PR28539 describes a segfault in lambda function search_one_symtab due to
>> psymbol_functions::expand_symtabs_matching calling expansion_notify with a
>> nullptr symtab:
>> ...
>>           struct compunit_symtab *symtab =
>>             psymtab_to_symtab (objfile, ps);
>>
>>           if (expansion_notify != NULL)
>>             if (!expansion_notify (symtab))
>>               return false;
>> ...
>>
>> This happens as follows.  The partial symtab ps is a dwarf2_include_psymtab
>> for some header file:
>> ...
>> (gdb) p ps.filename
>> $5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h"
>> ...
>>
>> The includer of ps is a shared symtab for a partial unit, with as user:
>> ...
>> (gdb) p ps.includer().user.filename
>> $11 = 0x64fc9f0 \
>>   "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp"
>> ...
>>
>> The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently
>> the shared symtab), but returns nullptr because:
>> ...
>> struct dwarf2_include_psymtab : public partial_symtab
>> {
>>   ...
>>   compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
>>   {
>>     return nullptr;
>>   }
>> ...
>>
>> Fix this by returning the Decl.cpp symtab instead, which fixes the segfault
>> in the PR.
>>
>> While trying to write a reproducer for this, I realized that this is difficult
>> because not all callers of psymbol_functions::expand_symtabs_matching have an
>> expansion_notify.  Consequently, I decided to add this assert:
>> ...
>>           struct compunit_symtab *symtab =
>>             psymtab_to_symtab (objfile, ps);
>>
>> +         gdb_assert (symtab != nullptr);
>> +
>>           if (expansion_notify != NULL)
>>             if (!expansion_notify (symtab))
>>               return false;
>> ...
>> which without the fix triggers in a few test-cases, f.i.:
>> ...
>> (gdb) maint expand-symtab dw2-symtab-includes.h^M
>> psymtab.c:1155: internal-error: virtual bool \
>>   psymbol_functions::expand_symtabs_matching(...): \
>>   Assertion `symtab != nullptr' failed.^M
>> A problem internal to GDB has been detected,^M
>> further debugging may prove unreliable.^M
>> FAIL: gdb.dwarf2/dw2-symtab-includes.exp: \
>>   maint expand-symtab dw2-symtab-includes.h (GDB internal error)
>> ...
>>
>> I also realized that with the assert fixed, it becomes possible to implement
>> a "maint expand-symtabs -verbose".
>>
>> Tested on x86_64-linux.
>>
>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539
> 
> Have you tried writing a test for this?

I gave up after spending considerable time trying to minimize the
reproducer.  The original reproducer is something like:
...
$ gdb -q -batch exec core -ex bt
...
and I managed to get that to:
...
$ gdb -q -batch exec core -ex "ptype clang::TranslationUnitDecl"
...
but didn't manage to reproduce using:
...
$ gdb -q -batch
/usr/lib/debug/usr/lib64/libclang-cpp.so.13-13.0.0-lp152.5.1.x86_64.debug -ex
"ptype clang::TranslationUnitDecl"
...

I wrote a patch that adds "maint expand-symtabs -id <CU offset>",
recorded the expanded symtabs, and generated a command file from that
replays the expansion order, and tried to use that to ensure that
"expansion state" is the same when doing the ptype.  Again, no success.

Usually, when problems are this hard to minimize, it requires a lot
trial and error to build a small reproducer, so I went with the abort,
which makes the problem trivial to reproduce, in existing test-cases.

But, prompted by your question, I copied
gdb.dwarf2/dw2-symtab-includes.exp, modified it to resemble the
situation I observed at the segfault, and ... the problem reproduced at
the first try :) .

So, here's an updated version with the assert dropped, and test-case added.

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-symtab-Fix-segfault-in-search_one_symtab.patch --]
[-- Type: text/x-patch, Size: 5261 bytes --]

[gdb/symtab] Fix segfault in search_one_symtab

PR28539 describes a segfault in lambda function search_one_symtab due to
psymbol_functions::expand_symtabs_matching calling expansion_notify with a
nullptr symtab:
...
          struct compunit_symtab *symtab =
            psymtab_to_symtab (objfile, ps);

          if (expansion_notify != NULL)
            if (!expansion_notify (symtab))
              return false;
...

This happens as follows.  The partial symtab ps is a dwarf2_include_psymtab
for some header file:
...
(gdb) p ps.filename
$5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h"
...

The includer of ps is a shared symtab for a partial unit, with as user:
...
(gdb) p ps.includer().user.filename
$11 = 0x64fc9f0 \
  "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp"
...

The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently
the shared symtab), but returns nullptr because:
...
struct dwarf2_include_psymtab : public partial_symtab
{
  ...
  compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
  {
    return nullptr;
  }
...

Fix this by returning the Decl.cpp symtab instead, which fixes the segfault
in the PR.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539

---
 gdb/dwarf2/read.c                                  |  5 +-
 .../gdb.dwarf2/dw2-symtab-includes-lookup.exp      | 99 ++++++++++++++++++++++
 2 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ed101237587..b59c638b2eb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5787,7 +5787,10 @@ struct dwarf2_include_psymtab : public partial_symtab
 
   compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
   {
-    return nullptr;
+    compunit_symtab *cust = includer ()->get_compunit_symtab (objfile);
+    while (cust != nullptr && cust->user != nullptr)
+      cust = cust->user;
+    return cust;
   }
 
 private:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes-lookup.exp b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes-lookup.exp
new file mode 100644
index 00000000000..74fcf56cd5c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes-lookup.exp
@@ -0,0 +1,99 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Lookup a type in a partial unit with DW_AT_stmt_list.
+#
+# The test-case is setup such that the partial symtab expansion route is
+# .h partial symtab -> shared partial symtab -> toplevel symtab.
+#
+# That is, the partial symtabs (as displayed by maint print objfiles) are:
+#
+#   ../sysdeps/x86_64/crtn.S at 0x3d944e0^M
+#   elf-init.c at 0x3d94440^M
+#   dw2-symtab-includes.h at 0x3d7c7a0^M
+#   <unknown> at 0x31ef870^M
+#   bla.c at 0x33985f0^M
+#   ../sysdeps/x86_64/crti.S at 0x33e9a00^M
+#   init.c at 0x33fa600^M
+#   ../sysdeps/x86_64/start.S at 0x33f3fd0^M
+#
+# and the expansion of dw2-symtab-includes.h triggers the expansion of its
+# includer <unknown>, which triggers expansion of user bla.c.
+#
+# The problem in PR28539 was that after expansion of dw2-symtab-includes.h
+# the expansion_notify function in psymbol_functions::expand_symtabs_matching
+# should be called with the bla.c symtab, but instead it got called with
+# nullptr, which caused a segfault.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support 1
+
+standard_testfile main.c .S
+
+# Create the DWARF.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    declare_labels partial_label lines_label
+    global srcdir subdir srcfile
+
+    cu {} {
+	partial_label: partial_unit {
+	    {stmt_list ${lines_label} DW_FORM_sec_offset}
+	} {
+	    DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      myint}
+	    }
+	}
+    }
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_C}
+	    {DW_AT_name bla.c}
+	} {
+	    imported_unit {
+		{import $partial_label ref_addr}
+	    }
+	}
+    }
+
+    lines {version 2} lines_label {
+	include_dir "${srcdir}/${subdir}"
+	file_name "dw2-symtab-includes.h" 1
+	program {
+	    {DW_LNS_advance_line 1}
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile \
+	  "${asm_file} ${srcfile}" {}] } {
+    return -1
+}
+
+# Check that no symtabs are expanded.
+set test "no symtabs expanded"
+if { [readnow] } {
+    unsupported $test
+    return -1
+}
+gdb_test_no_output "maint info symtabs" $test
+
+# Lookup myint.  Regression test for PR28539.
+gdb_test "ptype myint" "type = myint"

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PING][PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
  2021-11-15 12:58   ` Tom de Vries
@ 2021-11-22 19:29     ` Tom de Vries
  2021-11-29 13:51     ` [PATCH " Simon Marchi
  1 sibling, 0 replies; 10+ messages in thread
From: Tom de Vries @ 2021-11-22 19:29 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches, Tom Tromey

On 11/15/21 1:58 PM, Tom de Vries wrote:
> On 11/13/21 3:51 PM, Simon Marchi wrote:
>>
>> On 2021-11-12 12:16, Tom de Vries via Gdb-patches wrote:
>>> PR28539 describes a segfault in lambda function search_one_symtab due to
>>> psymbol_functions::expand_symtabs_matching calling expansion_notify with a
>>> nullptr symtab:
>>> ...
>>>           struct compunit_symtab *symtab =
>>>             psymtab_to_symtab (objfile, ps);
>>>
>>>           if (expansion_notify != NULL)
>>>             if (!expansion_notify (symtab))
>>>               return false;
>>> ...
>>>
>>> This happens as follows.  The partial symtab ps is a dwarf2_include_psymtab
>>> for some header file:
>>> ...
>>> (gdb) p ps.filename
>>> $5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h"
>>> ...
>>>
>>> The includer of ps is a shared symtab for a partial unit, with as user:
>>> ...
>>> (gdb) p ps.includer().user.filename
>>> $11 = 0x64fc9f0 \
>>>   "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp"
>>> ...
>>>
>>> The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently
>>> the shared symtab), but returns nullptr because:
>>> ...
>>> struct dwarf2_include_psymtab : public partial_symtab
>>> {
>>>   ...
>>>   compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
>>>   {
>>>     return nullptr;
>>>   }
>>> ...
>>>
>>> Fix this by returning the Decl.cpp symtab instead, which fixes the segfault
>>> in the PR.
>>>
>>> While trying to write a reproducer for this, I realized that this is difficult
>>> because not all callers of psymbol_functions::expand_symtabs_matching have an
>>> expansion_notify.  Consequently, I decided to add this assert:
>>> ...
>>>           struct compunit_symtab *symtab =
>>>             psymtab_to_symtab (objfile, ps);
>>>
>>> +         gdb_assert (symtab != nullptr);
>>> +
>>>           if (expansion_notify != NULL)
>>>             if (!expansion_notify (symtab))
>>>               return false;
>>> ...
>>> which without the fix triggers in a few test-cases, f.i.:
>>> ...
>>> (gdb) maint expand-symtab dw2-symtab-includes.h^M
>>> psymtab.c:1155: internal-error: virtual bool \
>>>   psymbol_functions::expand_symtabs_matching(...): \
>>>   Assertion `symtab != nullptr' failed.^M
>>> A problem internal to GDB has been detected,^M
>>> further debugging may prove unreliable.^M
>>> FAIL: gdb.dwarf2/dw2-symtab-includes.exp: \
>>>   maint expand-symtab dw2-symtab-includes.h (GDB internal error)
>>> ...
>>>
>>> I also realized that with the assert fixed, it becomes possible to implement
>>> a "maint expand-symtabs -verbose".
>>>
>>> Tested on x86_64-linux.
>>>
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539
>> Have you tried writing a test for this?
> I gave up after spending considerable time trying to minimize the
> reproducer.  The original reproducer is something like:
> ...
> $ gdb -q -batch exec core -ex bt
> ...
> and I managed to get that to:
> ...
> $ gdb -q -batch exec core -ex "ptype clang::TranslationUnitDecl"
> ...
> but didn't manage to reproduce using:
> ...
> $ gdb -q -batch
> /usr/lib/debug/usr/lib64/libclang-cpp.so.13-13.0.0-lp152.5.1.x86_64.debug -ex
> "ptype clang::TranslationUnitDecl"
> ...
> 
> I wrote a patch that adds "maint expand-symtabs -id <CU offset>",
> recorded the expanded symtabs, and generated a command file from that
> replays the expansion order, and tried to use that to ensure that
> "expansion state" is the same when doing the ptype.  Again, no success.
> 
> Usually, when problems are this hard to minimize, it requires a lot
> trial and error to build a small reproducer, so I went with the abort,
> which makes the problem trivial to reproduce, in existing test-cases.
> 
> But, prompted by your question, I copied
> gdb.dwarf2/dw2-symtab-includes.exp, modified it to resemble the
> situation I observed at the segfault, and ... the problem reproduced at
> the first try :) .
> 
> So, here's an updated version with the assert dropped, and test-case added.
> 

Ping.

Thanks,
- Tom


> 0001-gdb-symtab-Fix-segfault-in-search_one_symtab.patch
> 
> [gdb/symtab] Fix segfault in search_one_symtab
> 
> PR28539 describes a segfault in lambda function search_one_symtab due to
> psymbol_functions::expand_symtabs_matching calling expansion_notify with a
> nullptr symtab:
> ...
>           struct compunit_symtab *symtab =
>             psymtab_to_symtab (objfile, ps);
> 
>           if (expansion_notify != NULL)
>             if (!expansion_notify (symtab))
>               return false;
> ...
> 
> This happens as follows.  The partial symtab ps is a dwarf2_include_psymtab
> for some header file:
> ...
> (gdb) p ps.filename
> $5 = 0x64fcf80 "/usr/include/c++/11/bits/stl_construct.h"
> ...
> 
> The includer of ps is a shared symtab for a partial unit, with as user:
> ...
> (gdb) p ps.includer().user.filename
> $11 = 0x64fc9f0 \
>   "/usr/src/debug/llvm13-13.0.0-1.2.x86_64/tools/clang/lib/AST/Decl.cpp"
> ...
> 
> The call to psymtab_to_symtab expands the Decl.cpp symtab (and consequently
> the shared symtab), but returns nullptr because:
> ...
> struct dwarf2_include_psymtab : public partial_symtab
> {
>   ...
>   compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
>   {
>     return nullptr;
>   }
> ...
> 
> Fix this by returning the Decl.cpp symtab instead, which fixes the segfault
> in the PR.
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28539
> 
> ---
>  gdb/dwarf2/read.c                                  |  5 +-
>  .../gdb.dwarf2/dw2-symtab-includes-lookup.exp      | 99 ++++++++++++++++++++++
>  2 files changed, 103 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index ed101237587..b59c638b2eb 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -5787,7 +5787,10 @@ struct dwarf2_include_psymtab : public partial_symtab
>  
>    compunit_symtab *get_compunit_symtab (struct objfile *objfile) const override
>    {
> -    return nullptr;
> +    compunit_symtab *cust = includer ()->get_compunit_symtab (objfile);
> +    while (cust != nullptr && cust->user != nullptr)
> +      cust = cust->user;
> +    return cust;
>    }
>  
>  private:
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes-lookup.exp b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes-lookup.exp
> new file mode 100644
> index 00000000000..74fcf56cd5c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes-lookup.exp
> @@ -0,0 +1,99 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Lookup a type in a partial unit with DW_AT_stmt_list.
> +#
> +# The test-case is setup such that the partial symtab expansion route is
> +# .h partial symtab -> shared partial symtab -> toplevel symtab.
> +#
> +# That is, the partial symtabs (as displayed by maint print objfiles) are:
> +#
> +#   ../sysdeps/x86_64/crtn.S at 0x3d944e0^M
> +#   elf-init.c at 0x3d94440^M
> +#   dw2-symtab-includes.h at 0x3d7c7a0^M
> +#   <unknown> at 0x31ef870^M
> +#   bla.c at 0x33985f0^M
> +#   ../sysdeps/x86_64/crti.S at 0x33e9a00^M
> +#   init.c at 0x33fa600^M
> +#   ../sysdeps/x86_64/start.S at 0x33f3fd0^M
> +#
> +# and the expansion of dw2-symtab-includes.h triggers the expansion of its
> +# includer <unknown>, which triggers expansion of user bla.c.
> +#
> +# The problem in PR28539 was that after expansion of dw2-symtab-includes.h
> +# the expansion_notify function in psymbol_functions::expand_symtabs_matching
> +# should be called with the bla.c symtab, but instead it got called with
> +# nullptr, which caused a segfault.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +require dwarf2_support 1
> +
> +standard_testfile main.c .S
> +
> +# Create the DWARF.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    declare_labels partial_label lines_label
> +    global srcdir subdir srcfile
> +
> +    cu {} {
> +	partial_label: partial_unit {
> +	    {stmt_list ${lines_label} DW_FORM_sec_offset}
> +	} {
> +	    DW_TAG_base_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_encoding  @DW_ATE_signed}
> +		{DW_AT_name      myint}
> +	    }
> +	}
> +    }
> +
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_C}
> +	    {DW_AT_name bla.c}
> +	} {
> +	    imported_unit {
> +		{import $partial_label ref_addr}
> +	    }
> +	}
> +    }
> +
> +    lines {version 2} lines_label {
> +	include_dir "${srcdir}/${subdir}"
> +	file_name "dw2-symtab-includes.h" 1
> +	program {
> +	    {DW_LNS_advance_line 1}
> +	}
> +    }
> +}
> +
> +if { [prepare_for_testing "failed to prepare" $testfile \
> +	  "${asm_file} ${srcfile}" {}] } {
> +    return -1
> +}
> +
> +# Check that no symtabs are expanded.
> +set test "no symtabs expanded"
> +if { [readnow] } {
> +    unsupported $test
> +    return -1
> +}
> +gdb_test_no_output "maint info symtabs" $test
> +
> +# Lookup myint.  Regression test for PR28539.
> +gdb_test "ptype myint" "type = myint"
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
  2021-11-15 12:58   ` Tom de Vries
  2021-11-22 19:29     ` [PING][PATCH " Tom de Vries
@ 2021-11-29 13:51     ` Simon Marchi
  2021-11-29 15:22       ` Tom de Vries
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2021-11-29 13:51 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

On 2021-11-15 07:58, Tom de Vries wrote:
>> Have you tried writing a test for this?
> 
> I gave up after spending considerable time trying to minimize the
> reproducer.  The original reproducer is something like:
> ...
> $ gdb -q -batch exec core -ex bt
> ...
> and I managed to get that to:
> ...
> $ gdb -q -batch exec core -ex "ptype clang::TranslationUnitDecl"
> ...
> but didn't manage to reproduce using:
> ...
> $ gdb -q -batch
> /usr/lib/debug/usr/lib64/libclang-cpp.so.13-13.0.0-lp152.5.1.x86_64.debug -ex
> "ptype clang::TranslationUnitDecl"
> ...
> 
> I wrote a patch that adds "maint expand-symtabs -id <CU offset>",
> recorded the expanded symtabs, and generated a command file from that
> replays the expansion order, and tried to use that to ensure that
> "expansion state" is the same when doing the ptype.  Again, no success.
> 
> Usually, when problems are this hard to minimize, it requires a lot
> trial and error to build a small reproducer, so I went with the abort,
> which makes the problem trivial to reproduce, in existing test-cases.
> 
> But, prompted by your question, I copied
> gdb.dwarf2/dw2-symtab-includes.exp, modified it to resemble the
> situation I observed at the segfault, and ... the problem reproduced at
> the first try :) .
> 
> So, here's an updated version with the assert dropped, and test-case added.

Cool, thanks for doing that!

Any reason not to leave the assert in?  It looks right to me.

In the test:

    # Check that no symtabs are expanded.
    set test "no symtabs expanded"
    if { [readnow] } {
        unsupported $test
        return -1
    }
    gdb_test_no_output "maint info symtabs" $test

    # Lookup myint.  Regression test for PR28539.
    gdb_test "ptype myint" "type = myint"

When testing with readnow, I would skip the "maint info symtabs" test,
but I would maybe still do the "ptype myint" test.  It should still
work, and who knows what random bug this could catch.

Otherwise, LGTM.

Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
  2021-11-29 13:51     ` [PATCH " Simon Marchi
@ 2021-11-29 15:22       ` Tom de Vries
  0 siblings, 0 replies; 10+ messages in thread
From: Tom de Vries @ 2021-11-29 15:22 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 11/29/21 2:51 PM, Simon Marchi wrote:
> On 2021-11-15 07:58, Tom de Vries wrote:
>>> Have you tried writing a test for this?
>>
>> I gave up after spending considerable time trying to minimize the
>> reproducer.  The original reproducer is something like:
>> ...
>> $ gdb -q -batch exec core -ex bt
>> ...
>> and I managed to get that to:
>> ...
>> $ gdb -q -batch exec core -ex "ptype clang::TranslationUnitDecl"
>> ...
>> but didn't manage to reproduce using:
>> ...
>> $ gdb -q -batch
>> /usr/lib/debug/usr/lib64/libclang-cpp.so.13-13.0.0-lp152.5.1.x86_64.debug -ex
>> "ptype clang::TranslationUnitDecl"
>> ...
>>
>> I wrote a patch that adds "maint expand-symtabs -id <CU offset>",
>> recorded the expanded symtabs, and generated a command file from that
>> replays the expansion order, and tried to use that to ensure that
>> "expansion state" is the same when doing the ptype.  Again, no success.
>>
>> Usually, when problems are this hard to minimize, it requires a lot
>> trial and error to build a small reproducer, so I went with the abort,
>> which makes the problem trivial to reproduce, in existing test-cases.
>>
>> But, prompted by your question, I copied
>> gdb.dwarf2/dw2-symtab-includes.exp, modified it to resemble the
>> situation I observed at the segfault, and ... the problem reproduced at
>> the first try :) .
>>
>> So, here's an updated version with the assert dropped, and test-case added.
> 
> Cool, thanks for doing that!
> 
> Any reason not to leave the assert in?  It looks right to me.
> 

I dropped it because it was not necessary anymore to trigger the
problem, but agreed, it can be left in.

I've added it back.

> In the test:
> 
>     # Check that no symtabs are expanded.
>     set test "no symtabs expanded"
>     if { [readnow] } {
>         unsupported $test
>         return -1
>     }
>     gdb_test_no_output "maint info symtabs" $test
> 
>     # Lookup myint.  Regression test for PR28539.
>     gdb_test "ptype myint" "type = myint"
> 
> When testing with readnow, I would skip the "maint info symtabs" test,
> but I would maybe still do the "ptype myint" test.  It should still
> work, and who knows what random bug this could catch.
> 

Done.

> Otherwise, LGTM.
> 

Thanks for the review, committed.
- Tom

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PING][PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose
  2021-11-12 17:16 ` [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose Tom de Vries
@ 2022-01-03 14:53   ` Tom de Vries
  2022-01-08 10:55     ` Joel Brobecker
  2022-01-10  2:34   ` [PATCH " Simon Marchi
  1 sibling, 1 reply; 10+ messages in thread
From: Tom de Vries @ 2022-01-03 14:53 UTC (permalink / raw)
  To: gdb-patches

On 11/12/21 18:16, Tom de Vries via Gdb-patches wrote:
> Add a -verbose option to command "maint expand-symtabs", such that we
> have (abbreviating /home/abuild/rpmbuild/BUILD to $BUILD):
> ...
> $ gdb -q -batch ./outputs/gdb.dwarf2/dw2-symtab-includes/dw2-symtab-includes \
>    -ex "maint expand-symtabs -verbose"
> Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/crtn.S at 0x26c9f00
> Expanded symtab for file <unknown> at 0x26ca1e0
> Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/crti.S at 0x26ca540
> Expanded symtab for file $BUILD/glibc-2.34/csu/static-reloc.c at 0x26ca860
> Expanded symtab for file $BUILD/glibc-2.34/csu/init.c at 0x2716890
> Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86/abi-note.c at 0x2708a90
> Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/start.S at 0x2717810
> ...
> 
> Note that the <unknown> corresponds to a CU without name:
> ...
>   <0><963>: Abbrev Number: 2 (DW_TAG_compile_unit)
>      <964>   DW_AT_language    : 2       (non-ANSI C)
>      <965>   DW_AT_stmt_list   : 0x1e0
>   <1><969>: Abbrev Number: 3 (DW_TAG_imported_unit)
> ...
> 
> Tested on x86_64-linux.

Ping.

Thanks,
- Tom

> ---
>   gdb/doc/gdb.texinfo                              |  5 +++--
>   gdb/symmisc.c                                    | 14 +++++++++++++-
>   gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp |  3 ++-
>   3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index fc8e5bdf3db..fbf6e8549e1 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -39159,10 +39159,11 @@ Use this to check, for example, whether a symbol is in one but not the other.
>   Check the consistency of currently expanded symtabs.
>   
>   @kindex maint expand-symtabs
> -@item maint expand-symtabs [@var{regexp}]
> +@item maint expand-symtabs @r{[}-verbose@r{]} [@var{regexp}]
>   Expand symbol tables.
>   If @var{regexp} is specified, only expand symbol tables for file
> -names matching @var{regexp}.
> +names matching @var{regexp}.  If @code{-verbose} is passed, mentions
> +expanded symbol tables.
>   
>   @kindex maint set catch-demangler-crashes
>   @kindex maint show catch-demangler-crashes
> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index e38ceb6bda1..7597487201e 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -904,6 +904,7 @@ maintenance_check_symtabs (const char *ignore, int from_tty)
>   static void
>   maintenance_expand_symtabs (const char *args, int from_tty)
>   {
> +  bool verbose = args != nullptr && check_for_argument (&args, "-verbose");
>     char *regexp = NULL;
>   
>     /* We use buildargv here so that we handle spaces in the regexp
> @@ -923,6 +924,17 @@ maintenance_expand_symtabs (const char *args, int from_tty)
>     if (regexp)
>       re_comp (regexp);
>   
> +  auto on_expansion = [&] (compunit_symtab *symtab)
> +  {
> +    if (verbose)
> +      printf_filtered ("Expanded symtab for file %s at %s\n",
> +		       symtab_to_fullname (COMPUNIT_FILETABS (symtab)),
> +		       host_address_to_string (symtab));
> +
> +    /* Keep going.  */
> +    return true;
> +  };
> +
>     for (struct program_space *pspace : program_spaces)
>       for (objfile *objfile : pspace->objfiles ())
>         objfile->expand_symtabs_matching
> @@ -934,7 +946,7 @@ maintenance_expand_symtabs (const char *args, int from_tty)
>   	 },
>   	 NULL,
>   	 NULL,
> -	 NULL,
> +	 on_expansion,
>   	 SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
>   	 UNDEF_DOMAIN,
>   	 ALL_DOMAIN);
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
> index 5eecc8f58ef..a97c5328f7c 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
> @@ -70,7 +70,8 @@ if { [readnow] } {
>   gdb_test_no_output "maint info symtabs" $test
>   
>   # Expand dw2-symtab-includes.h symtab
> -gdb_test "maint expand-symtab dw2-symtab-includes.h"
> +gdb_test "maint expand-symtab -verbose dw2-symtab-includes.h" \
> +    "Expanded symtab for file <unknown> at $hex"
>   
>   # Check that there are includes.
>   gdb_test "maint info symtabs" \

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PING][PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose
  2022-01-03 14:53   ` [PING][PATCH " Tom de Vries
@ 2022-01-08 10:55     ` Joel Brobecker
  0 siblings, 0 replies; 10+ messages in thread
From: Joel Brobecker @ 2022-01-08 10:55 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches; +Cc: Joel Brobecker

Hi Tom,

On Mon, Jan 03, 2022 at 03:53:02PM +0100, Tom de Vries via Gdb-patches wrote:
> On 11/12/21 18:16, Tom de Vries via Gdb-patches wrote:
> > Add a -verbose option to command "maint expand-symtabs", such that we
> > have (abbreviating /home/abuild/rpmbuild/BUILD to $BUILD):
> > ...
> > $ gdb -q -batch ./outputs/gdb.dwarf2/dw2-symtab-includes/dw2-symtab-includes \
> >    -ex "maint expand-symtabs -verbose"
> > Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/crtn.S at 0x26c9f00
> > Expanded symtab for file <unknown> at 0x26ca1e0
> > Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/crti.S at 0x26ca540
> > Expanded symtab for file $BUILD/glibc-2.34/csu/static-reloc.c at 0x26ca860
> > Expanded symtab for file $BUILD/glibc-2.34/csu/init.c at 0x2716890
> > Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86/abi-note.c at 0x2708a90
> > Expanded symtab for file $BUILD/glibc-2.34/sysdeps/x86_64/start.S at 0x2717810
> > ...
> > 
> > Note that the <unknown> corresponds to a CU without name:
> > ...
> >   <0><963>: Abbrev Number: 2 (DW_TAG_compile_unit)
> >      <964>   DW_AT_language    : 2       (non-ANSI C)
> >      <965>   DW_AT_stmt_list   : 0x1e0
> >   <1><969>: Abbrev Number: 3 (DW_TAG_imported_unit)
> > ...
> > 
> > Tested on x86_64-linux.
> 
> Ping.

Thanks for the patch.

This seems reasonable to me, so OK for me (minus the changes to
the documentation).

> > ---
> >   gdb/doc/gdb.texinfo                              |  5 +++--
> >   gdb/symmisc.c                                    | 14 +++++++++++++-
> >   gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp |  3 ++-
> >   3 files changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index fc8e5bdf3db..fbf6e8549e1 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -39159,10 +39159,11 @@ Use this to check, for example, whether a symbol is in one but not the other.
> >   Check the consistency of currently expanded symtabs.
> >   @kindex maint expand-symtabs
> > -@item maint expand-symtabs [@var{regexp}]
> > +@item maint expand-symtabs @r{[}-verbose@r{]} [@var{regexp}]
> >   Expand symbol tables.
> >   If @var{regexp} is specified, only expand symbol tables for file
> > -names matching @var{regexp}.
> > +names matching @var{regexp}.  If @code{-verbose} is passed, mentions
> > +expanded symbol tables.
> >   @kindex maint set catch-demangler-crashes
> >   @kindex maint show catch-demangler-crashes
> > diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> > index e38ceb6bda1..7597487201e 100644
> > --- a/gdb/symmisc.c
> > +++ b/gdb/symmisc.c
> > @@ -904,6 +904,7 @@ maintenance_check_symtabs (const char *ignore, int from_tty)
> >   static void
> >   maintenance_expand_symtabs (const char *args, int from_tty)
> >   {
> > +  bool verbose = args != nullptr && check_for_argument (&args, "-verbose");
> >     char *regexp = NULL;
> >     /* We use buildargv here so that we handle spaces in the regexp
> > @@ -923,6 +924,17 @@ maintenance_expand_symtabs (const char *args, int from_tty)
> >     if (regexp)
> >       re_comp (regexp);
> > +  auto on_expansion = [&] (compunit_symtab *symtab)
> > +  {
> > +    if (verbose)
> > +      printf_filtered ("Expanded symtab for file %s at %s\n",
> > +		       symtab_to_fullname (COMPUNIT_FILETABS (symtab)),
> > +		       host_address_to_string (symtab));
> > +
> > +    /* Keep going.  */
> > +    return true;
> > +  };
> > +
> >     for (struct program_space *pspace : program_spaces)
> >       for (objfile *objfile : pspace->objfiles ())
> >         objfile->expand_symtabs_matching
> > @@ -934,7 +946,7 @@ maintenance_expand_symtabs (const char *args, int from_tty)
> >   	 },
> >   	 NULL,
> >   	 NULL,
> > -	 NULL,
> > +	 on_expansion,
> >   	 SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
> >   	 UNDEF_DOMAIN,
> >   	 ALL_DOMAIN);
> > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
> > index 5eecc8f58ef..a97c5328f7c 100644
> > --- a/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
> > +++ b/gdb/testsuite/gdb.dwarf2/dw2-symtab-includes.exp
> > @@ -70,7 +70,8 @@ if { [readnow] } {
> >   gdb_test_no_output "maint info symtabs" $test
> >   # Expand dw2-symtab-includes.h symtab
> > -gdb_test "maint expand-symtab dw2-symtab-includes.h"
> > +gdb_test "maint expand-symtab -verbose dw2-symtab-includes.h" \
> > +    "Expanded symtab for file <unknown> at $hex"
> >   # Check that there are includes.
> >   gdb_test "maint info symtabs" \

-- 
Joel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose
  2021-11-12 17:16 ` [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose Tom de Vries
  2022-01-03 14:53   ` [PING][PATCH " Tom de Vries
@ 2022-01-10  2:34   ` Simon Marchi
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2022-01-10  2:34 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

> diff --git a/gdb/symmisc.c b/gdb/symmisc.c
> index e38ceb6bda1..7597487201e 100644
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -904,6 +904,7 @@ maintenance_check_symtabs (const char *ignore, int from_tty)
>  static void
>  maintenance_expand_symtabs (const char *args, int from_tty)
>  {
> +  bool verbose = args != nullptr && check_for_argument (&args, "-verbose");

IWBN to use gdb::option::process_options for any new option we add.

>    char *regexp = NULL;
>  
>    /* We use buildargv here so that we handle spaces in the regexp
> @@ -923,6 +924,17 @@ maintenance_expand_symtabs (const char *args, int from_tty)
>    if (regexp)
>      re_comp (regexp);
>  
> +  auto on_expansion = [&] (compunit_symtab *symtab)
> +  {
> +    if (verbose)
> +      printf_filtered ("Expanded symtab for file %s at %s\n",
> +		       symtab_to_fullname (COMPUNIT_FILETABS (symtab)),
> +		       host_address_to_string (symtab));
> +
> +    /* Keep going.  */
> +    return true;
> +  };

I think we usually format them with one more indentation level, as if it was
a control flow structure:

  auto on_expansion = [&] (compunit_symtab *symtab)
    {
      ...
    }

Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-01-10  2:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 17:16 [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Tom de Vries
2021-11-12 17:16 ` [PATCH 2/2] [gdb/symtab] Add maint expand-symtabs -verbose Tom de Vries
2022-01-03 14:53   ` [PING][PATCH " Tom de Vries
2022-01-08 10:55     ` Joel Brobecker
2022-01-10  2:34   ` [PATCH " Simon Marchi
2021-11-13 14:51 ` [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab Simon Marchi
2021-11-15 12:58   ` Tom de Vries
2021-11-22 19:29     ` [PING][PATCH " Tom de Vries
2021-11-29 13:51     ` [PATCH " Simon Marchi
2021-11-29 15:22       ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).