[Bug breakpoints/26096] New: gdb sets breakpoint at cold clone

[Bug breakpoints/26096] New: gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

            Bug ID: 26096
           Summary: gdb sets breakpoint at cold clone
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: breakpoints
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider test-case test.c:
...
$ cat test.c
#include <stdlib.h>

int a;
int b;
int c;

static int __attribute__((used, noinline, noclone))
foo (void)
{
  a = 2;
  if (b)
    abort ();

  return c;
}

static int __attribute__((used, noinline, noclone))
bar (void)
{
  a = 1;
  if (c)
    abort ();
  return b;
}

int
main (int argc, char **)
{
  b = argc * 2;
  c = argc / 2;

  if (b + c == 5)
    abort ();

  return foo () + bar ();
}
...

Compiled with gcc-10:
...
$ g++-10 test.c -O2 -g  -Wall -Wextra
...

The functions foo and bar get split into a hot and cold part by gcc's
freorder-blocks-and-partition:
...
$ nm a.out | c++filt | egrep "foo|bar"
00000000004005b0 t bar()
0000000000400455 t bar() [clone .cold]
0000000000400580 t foo()
0000000000400450 t foo() [clone .cold]
...

When we set breakpoints on foo and bar however, we get two breakpoint locations
per function:
...
$ gdb -batch a.out -ex "b foo" -ex "b bar" -ex "info break"
Breakpoint 1 at 0x400450: foo. (2 locations)
Breakpoint 2 at 0x400455: bar. (2 locations)
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   <MULTIPLE>         
1.1                         y   0x0000000000400450 in foo() at test.c:12
1.2                         y   0x0000000000400580 in foo() at test.c:10
2       breakpoint     keep y   <MULTIPLE>         
2.1                         y   0x0000000000400455 in bar() at test.c:22
2.2                         y   0x00000000004005b0 in bar() at test.c:20
...

So, the cold part of the function gets its own breakpoint.

In other words, we have:
...
$ gdb -q a.out
Reading symbols from a.out...
(gdb) b foo
Breakpoint 1 at 0x400450: foo. (2 locations)
(gdb) r
Starting program: /home/vries/a.out 

Breakpoint 1, foo () at test.c:10
10        a = 2;
(gdb) c
Continuing.

Breakpoint 1, foo () at test.c:12
12          abort ();
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Tentative patch:
...
diff --git a/gdb/utils.c b/gdb/utils.c
index 102db28787..073d5b2e10 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2652,7 +2652,18 @@ strncmp_iw_with_mode (const char *string1, const char
*string2,
          return 0;
        }
       else
-       return (*string1 != '\0' && *string1 != '(');
+       {
+         if (*string1 == '\0')
+           return 0;
+
+         if (*string1 == '(')
+           {
+             const char *end_str1 = string1 + strlen (string1);
+             return end_str1[-1] != ')';
+           }
+
+         return 1;
+       }
     }
   else
     return 1;
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #1)
> Tentative patch:

OK, that's too restrictive, we miss out on the '() const':
...
(gdb) complete b const_overload_fn()^M
b struct_with_const_overload::const_overload_fn()^M
b struct_with_const_overload::const_overload_fn() const^M
(gdb) PASS: gdb.linespec/cpcompletion.exp: const-overload: cmd complete "b
const_overload_fn()"
b const_overload_fn^M
Breakpoint 36 at 0x400646: file
/data/gdb_versions/devel/src/gdb/testsuite/gdb.linespec/cpls.cc, line 197.^M
(gdb) info breakpoint $bpnum^M
Num     Type           Disp Enb Address            What^M
36      breakpoint     keep y   0x0000000000400646 in
struct_with_const_overload::const_overload_fn() at /data/gd\
b_versions/devel/src/gdb/testsuite/gdb.linespec/cpls.cc:197^M
(gdb) FAIL: gdb.linespec/cpcompletion.exp: const-overload: compare "b
const_overload_fn" completion list with bp \
location list: matches
...

Let's try this:
...
diff --git a/gdb/utils.c b/gdb/utils.c
index 102db28787..8b20d6669e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2652,7 +2652,34 @@ strncmp_iw_with_mode (const char *string1, const char
*string2,
          return 0;
        }
       else
-       return (*string1 != '\0' && *string1 != '(');
+       {
+         if (*string1 == '\0')
+           return 0;
+
+         /* Skip over '(.*)'.  */
+         if (*string1 != '(')
+           return 1;
+         while (true)
+           {
+             string1++;
+             if (*string1 == '\0')
+               return 0;
+             if (*string1 == ')')
+               break;
+           }
+
+         /* Return no-match if there's a "[clone .cold]" suffix.  */
+         while (true)
+           {
+             string1++;
+             if (*string1 == '\0')
+               return 0;
+             if (*string1 == '[')
+               return strcmp (string1, "[clone .cold]") == 0;
+           }
+
+         return 1;
+       }
     }
   else
     return 1;
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Another thing I noticed with cold clones:
...
$ gdb -q -batch a.out -ex "maint print psymbols" | egrep "foo|bar"
    `bar', function, 0x400455
    `foo', function, 0x400450
...

The psymbol address of the function bar is the address of "bar() [clone
.cold]".

Presumably we'd want the entry address here.

Looking at the corresponding DWARF, we have:
...
 <1><990>: Abbrev Number: 35 (DW_TAG_subprogram)
    <991>   DW_AT_name        : foo
    <995>   DW_AT_decl_file   : 1
    <996>   DW_AT_decl_line   : 8
    <997>   DW_AT_decl_column : 1
    <998>   DW_AT_type        : <0x39b>
    <99c>   DW_AT_ranges      : 0x40
    <9a0>   DW_AT_frame_base  : 1 byte block: 9c        (DW_OP_call_frame_cfa)
    <9a2>   DW_AT_GNU_all_call_sites: 1
    <9a2>   DW_AT_sibling     : <0x9b4>
...

In DWARF 4 standard 2.18 Entry Address we read:
...
If no DW_AT_entry_pc attribute is present, then the entry address is assumed to
be the same as the value of the DW_AT_low_pc attribute, if present; otherwise,
the entry address is unknown.
...

In this case, we have no DW_AT_entry_pc and no DW_AT_low_pc attribute. 
Arguably, this is a gcc bug.

Either way, in absence of a fix in gcc for this, we could adapt a
interpretation that the entry pc is the start address of the first range. At
least, this works for this exec:
...
    00000070 00000000004005b0 00000000004005d7
    00000070 0000000000400455 000000000040045a
    00000070 <End of list>
...

This also seems to be the current interpretation for full symtabs, AFAIU from
the comment for BLOCK_ENTRY_PC in block.h:
...
/* Define the "entry pc" for a block BL to be the lowest (start) address        
   for the block when all addresses within the block are contiguous.  If        
   non-contiguous, then use the start address for the first range in the        
   block.                                                                       
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/pipermail/gdb-patches/2021-May/179385.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
Related patch:
https://sourceware.org/pipermail/gdb-patches/2021-May/179371.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[bernd.edlinger at hotmail dot de]

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

Bernd Edlinger <bernd.edlinger at hotmail dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bernd.edlinger at hotmail dot de

--- Comment #6 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Tom de Vries from comment #3)
> Another thing I noticed with cold clones:
> ...
> $ gdb -q -batch a.out -ex "maint print psymbols" | egrep "foo|bar"
>     `bar', function, 0x400455
>     `foo', function, 0x400450
> ...
> 
> The psymbol address of the function bar is the address of "bar() [clone
> .cold]".
> 
> Presumably we'd want the entry address here.
> 
> Looking at the corresponding DWARF, we have:
> ...
>  <1><990>: Abbrev Number: 35 (DW_TAG_subprogram)
>     <991>   DW_AT_name        : foo
>     <995>   DW_AT_decl_file   : 1
>     <996>   DW_AT_decl_line   : 8
>     <997>   DW_AT_decl_column : 1
>     <998>   DW_AT_type        : <0x39b>
>     <99c>   DW_AT_ranges      : 0x40
>     <9a0>   DW_AT_frame_base  : 1 byte block: 9c       
> (DW_OP_call_frame_cfa)
>     <9a2>   DW_AT_GNU_all_call_sites: 1
>     <9a2>   DW_AT_sibling     : <0x9b4>
> ...
> 
> In DWARF 4 standard 2.18 Entry Address we read:
> ...
> If no DW_AT_entry_pc attribute is present, then the entry address is assumed
> to be the same as the value of the DW_AT_low_pc attribute, if present;
> otherwise, the entry address is unknown.
> ...
> 
> In this case, we have no DW_AT_entry_pc and no DW_AT_low_pc attribute. 
> Arguably, this is a gcc bug.
> 
> Either way, in absence of a fix in gcc for this, we could adapt a
> interpretation that the entry pc is the start address of the first range. At
> least, this works for this exec:
> ...
>     00000070 00000000004005b0 00000000004005d7
>     00000070 0000000000400455 000000000040045a
>     00000070 <End of list>
> ...
> 
> This also seems to be the current interpretation for full symtabs, AFAIU
> from the comment for BLOCK_ENTRY_PC in block.h:
> ...
> /* Define the "entry pc" for a block BL to be the lowest (start) address    
> 
>    for the block when all addresses within the block are contiguous.  If    
> 
>    non-contiguous, then use the start address for the first range in the    
> 
>    block.                                                                   
> 
> ...

This is related to this patch:
https://sourceware.org/pipermail/gdb-patches/2021-May/179369.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=17d305ef8f4b5bf20beaaad427490b3c6773909b

commit 17d305ef8f4b5bf20beaaad427490b3c6773909b
Author: Tom de Vries <tdevries@suse.de>
Date:   Tue Jun 1 15:25:51 2021 +0200

    [gdb/symtab] Ignore cold clones

    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.

    gdb/ChangeLog:

    2021-06-01  Tom de Vries  <tdevries@suse.de>

            PR symtab/26096
            * minsyms.c (msymbol_is_cold_clone): New function.
            (msymbol_is_function): Use msymbol_is_cold_clone.

    gdb/testsuite/ChangeLog:

    2021-06-01  Tom de Vries  <tdevries@suse.de>

            PR symtab/26096
            * gdb.cp/cold-clone.cc: New test.
            * gdb.cp/cold-clone.exp: New file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #6)
> This is related to this patch:
> https://sourceware.org/pipermail/gdb-patches/2021-May/179369.html

I looked at the patch, and it doesn't look related.  AFAUI, your patch does
something related to full symbols, my observation was related to partial
symbols.

To confirm, I've applied the patch, and ran the example, no changes.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[vries at gcc dot gnu.org]

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

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #9 from Tom de Vries <vries at gcc dot gnu.org> ---
Patch with test-case committed, marking resolved-fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug breakpoints/26096] gdb sets breakpoint at cold clone

[bernd.edlinger at hotmail dot de]

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

--- Comment #10 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Apparently I misunderstood your comment above:

> Either way, in absence of a fix in gcc for this, we could adapt a interpretation
> that the entry pc is the start address of the first range. At least, this works
> for this exec:

What I meant, is that currently the entry_pc is ignored at least when
you want to place a break point on an inline function.
Instead the start of the first subrange is used.

But the entry_pc is sometimes not the start of the first subrange,
but maybe the second or third.

My patch re-orders the sub-range so that the subrange that starts at
the entry_pc becomes the first one.

but I always looked at the breakpoint addresses:

(gdb) b <symbol>
(gdb) info b

-- 
You are receiving this mail because:
You are on the CC list for the bug.