[PATCH] gdb: remove assertion in psymbol_functions::expand_symtabs_matching

[PATCH] gdb: remove assertion in psymbol_functions::expand_symtabs_matching

[Simon Marchi]

psymtab_to_symtab is documented as possibly returning nullptr, if the
primary symtab of the partial symtab has no symbols.  However,
psymbol_functions::expand_symtabs_matching asserts that the result of
psymtab_to_symtab as non-nullptr.

I caught this assert by trying the CTF symbol reader on a library I
built with -gctf:

    $ ./gdb --data-directory=data-directory /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
    ...
    Reading symbols from /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0...
    (gdb) maintenance expand-symtabs
    /home/simark/src/binutils-gdb/gdb/psymtab.c:1142: internal-error: expand_symtabs_matching: Assertion `symtab != nullptr' failed.

The "symtab" in question is:

    $  readelf --ctf=.ctf /tmp/babeltrace-ctf/src/lib/.libs/libbabeltrace2.so.0.0.0
    ...
    CTF archive member: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c:

      Header:
        Magic number: 0xdff2
        Version: 4 (CTF_VERSION_3)
        Flags: 0xe (CTF_F_NEWFUNCINFO, CTF_F_IDXSORTED, CTF_F_DYNSTR)
        Parent name: .ctf
        Compilation unit name: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c
        Type section:       0x0 -- 0x13 (0x14 bytes)
        String section:     0x14 -- 0x5f (0x4c bytes)

      Labels:

      Data objects:

      Function objects:

      Variables:

      Types:
        0x80000001: (kind 5) bt_bool (*) (const bt_value *) (aligned at 0x8)

      Strings:
        0x0:
        0x1: .ctf
        0x6: /home/simark/src/babeltrace/src/lib/graph/component-descriptor-set.c

It contains a single type, and it is skipped by ctf_add_type_cb, because
an identical type was already seen earlier in this objfile.  As a
result, no compunit_symtab is created.

Change psymbol_functions::expand_symtabs_matching to expect taht
psymtab_to_symtab can return nullptr.

Another possibility would be to make the CTF symbol reader always create
a compunit_symtab, even if there are no symbols in it (like the DWARF
parser does), but so far I don't see any advantage in doing so.

Change-Id: Ic43c38202c838a5eb87630ed1fd61d33528164f4
---
 gdb/psymtab.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index b31ce877b62d..a26ecd0b4e5e 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1136,13 +1136,10 @@ psymbol_functions::expand_symtabs_matching
 					  *psym_lookup_name,
 					  symbol_matcher))
 	{
-	  struct compunit_symtab *symtab =
-	    psymtab_to_symtab (objfile, ps);
+	  compunit_symtab *cust = psymtab_to_symtab (objfile, ps);
 
-	  gdb_assert (symtab != nullptr);
-
-	  if (expansion_notify != NULL)
-	    if (!expansion_notify (symtab))
+	  if (cust != nullptr && expansion_notify != nullptr)
+	    if (!expansion_notify (cust))
 	      return false;
 	}
     }

base-commit: 49a82d50c0ee41bf51db6899291a8beaea4e7e2a
prerequisite-patch-id: a3abedcfbf04f0618c5f935bb07cbcef058152e8
-- 
2.35.1

Re: [PATCH] gdb: remove assertion in psymbol_functions::expand_symtabs_matching

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> psymtab_to_symtab is documented as possibly returning nullptr, if the
Simon> primary symtab of the partial symtab has no symbols.  However,
Simon> psymbol_functions::expand_symtabs_matching asserts that the result of
Simon> psymtab_to_symtab as non-nullptr.

Thanks.

Simon> Change psymbol_functions::expand_symtabs_matching to expect taht
Simon> psymtab_to_symtab can return nullptr.

s/taht/that/

Simon> Another possibility would be to make the CTF symbol reader always create
Simon> a compunit_symtab, even if there are no symbols in it (like the DWARF
Simon> parser does), but so far I don't see any advantage in doing so.

Probably the stabs or other readers also can return NULL here anyway.
I think this patch is ok.

Tom

Re: [PATCH] gdb: remove assertion in psymbol_functions::expand_symtabs_matching

[Simon Marchi]

On 2022-04-04 12:57, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> psymtab_to_symtab is documented as possibly returning nullptr, if the
> Simon> primary symtab of the partial symtab has no symbols.  However,
> Simon> psymbol_functions::expand_symtabs_matching asserts that the result of
> Simon> psymtab_to_symtab as non-nullptr.
> 
> Thanks.
> 
> Simon> Change psymbol_functions::expand_symtabs_matching to expect taht
> Simon> psymtab_to_symtab can return nullptr.
> 
> s/taht/that/

Fixed.

> Simon> Another possibility would be to make the CTF symbol reader always create
> Simon> a compunit_symtab, even if there are no symbols in it (like the DWARF
> Simon> parser does), but so far I don't see any advantage in doing so.
> 
> Probably the stabs or other readers also can return NULL here anyway.
> I think this patch is ok.

Thanks, will push.

Simon