[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    Re: [PATCH 1/2] [gdb/symtab] Fix segfault in search_one_symtab
From:       Tom de Vries via Gdb-patches <gdb-patches () sourceware ! org>
Date:       2021-11-29 15:22:17
Message-ID: 2c9b07d2-b133-00f8-5d1e-1ffdf3c3b8fa () suse ! de
[Download RAW message or body]

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
[prev in list] [next in list] [prev in thread] [next in thread]