Re: [PATCH][gdb/symtab] Ignore cold clones

[Tom de Vries <tdevries@suse.de>]

On 5/31/21 4:26 PM, Simon Marchi wrote:
> On 2021-05-31 10:15 a.m., Tom de Vries wrote:
>> Hi,
>>
>> Consider the test-case contained in this patch, compiled for c using gcc-10:
>> ...
>> $ gcc-10 -x c src/gdb/testsuite/gdb.cp/cold-clone.cc -O2 -g -Wall -Wextra
>> ...
>>
>> When setting a breakpoint on foo, we get one breakpoint location:
>> ...
>> $ gdb -q -batch a.out -ex "b foo"
>> Breakpoint 1 at 0x400560: file cold-clone.cc, line 28.
>> ...
>>
>> However, when we compile for c++ instead, we get two breakpoint locations:
>> ...
>> $ gdb -q -batch a.out -ex "b foo" -ex "info break"
>> Breakpoint 1 at 0x400430: foo. (2 locations)
>> Num  Type        Disp Enb Address            What
>> 1    breakpoint  keep y   <MULTIPLE>
>> 1.1                   y   0x0000000000400430 in foo() at cold-clone.cc:30
>> 1.2                   y   0x0000000000400560 in foo() at cold-clone.cc:28
>> ...
>>
>> The additional breakpoint location at 0x400430 corresponds to the cold clone:
>> ...
>> $ nm a.out | grep foo
>> 0000000000400560 t _ZL3foov
>> 0000000000400430 t _ZL3foov.cold
>> ...
>> which demangled looks like this:
>> ...
>> $ nm -C a.out | grep foo
>> 0000000000400560 t foo()
>> 0000000000400430 t foo() [clone .cold]
>> ...
>>
>> [ Or, in the case of the cc1 mentioned in PR23710:
>> ...
>> $ nm cc1 | grep do_rpo_vn.*cold
>> 000000000058659d t \
>>   _ZL9do_rpo_vnP8functionP8edge_defP11bitmap_headbb.cold.138
>> $ nm -C cc1 | grep do_rpo_vn.*cold
>> 000000000058659d t \
>>   do_rpo_vn(function*, edge_def*, bitmap_head*, bool, bool) [clone .cold.138]
>> ... ]
>>
>> The cold clone is a part of the function that is split off from the rest of
>> the function because it's considered cold (not frequently executed).  So while
>> the symbol points to code that is part of a function, it doesn't point to a
>> function entry, so the desirable behaviour for "break foo" is to ignore this
>> symbol.
>>
>> When compiling for c, the symbol "foo.cold" is entered as minimal symbol
>> with the search name "foo.cold", and the lookup using "foo" fails to find that
>> symbol.
>>
>> But when compiling for c++, the symbol "foo.cold" is entered as minimal symbol
>> with both the mangled and demangled name, and for the demangled name
>> "foo() [clone .cold]" we get the search name "foo" (because
>> cp_search_name_hash stops hashing at '('), and the lookup using "foo" succeeds.
>>
>> Fix this by recognizing the cold clone suffix and returning false for such a
>> minimal symbol in msymbol_is_function.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
> 
> Is this related to
> 
> https://sourceware.org/pipermail/gdb-patches/2021-May/179371.html
> 

Yes, it looks like both attempt to fix the same PR, thanks for the pointer.

At first glance, that one doesn't handle symbol
"_ZL9do_rpo_vnP8functionP8edge_defP11bitmap_headbb.cold.138".

Another difference seems to be how the cold clone status is determined:
- this patch uses the demangled name, and treats finding the .cold
  string (used in a specific way) as proof
- Bernd's patch uses the mangled name, and treats finding the .cold
  string as a hint, and proceeds to find the corresponding function
  entry symbol to proof it

[ FWIW, that patch mentions it fixes 77f2120b200 ("Don't drop static
function bp locations w/o debug info"), but the PR did exist before that
commit, that commit just made it more frequent. ]

Thanks,
- Tom