[binutils-gdb] [gdb/symtab] Ignore cold clones

Tom de Vries vries@sourceware.org
Tue Jun 1 13:25:54 GMT 2021


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.

Diff:
---
 gdb/ChangeLog                       |  6 +++++
 gdb/minsyms.c                       | 52 +++++++++++++++++++++++++++++++++++
 gdb/testsuite/ChangeLog             |  6 +++++
 gdb/testsuite/gdb.cp/cold-clone.cc  | 54 +++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/cold-clone.exp | 30 +++++++++++++++++++++
 5 files changed, 148 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a57a41c97ef..7780f76e84e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+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.
+
 2021-06-01  Fredrik Hederstierna  <fredrik@hederstierna.com>
 	    Andrew Burgess  <andrew.burgess@embecosm.com>
 
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 8ffd9076124..80c3d431209 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -60,6 +60,53 @@
 #include <mutex>
 #endif
 
+/* Return true if MINSYM is a cold clone symbol.
+   Recognize f.i. these symbols (mangled/demangled):
+   - _ZL3foov.cold
+     foo() [clone .cold]
+   - _ZL9do_rpo_vnP8functionP8edge_defP11bitmap_headbb.cold.138
+     do_rpo_vn(function*, edge_def*, bitmap_head*, bool, bool)	\
+       [clone .cold.138].  */
+
+static bool
+msymbol_is_cold_clone (minimal_symbol *minsym)
+{
+  const char *name = minsym->natural_name ();
+  size_t name_len = strlen (name);
+  if (name_len < 1)
+    return false;
+
+  const char *last = &name[name_len - 1];
+  if (*last != ']')
+    return false;
+
+  const char *suffix = " [clone .cold";
+  size_t suffix_len = strlen (suffix);
+  const char *found = strstr (name, suffix);
+  if (found == nullptr)
+    return false;
+
+  const char *start = &found[suffix_len];
+  if (*start == ']')
+    return true;
+
+  if (*start != '.')
+    return false;
+
+  const char *p;
+  for (p = start + 1; p <= last; ++p)
+    {
+      if (*p >= '0' && *p <= '9')
+	continue;
+      break;
+    }
+
+  if (p == last)
+    return true;
+
+  return false;
+}
+
 /* See minsyms.h.  */
 
 bool
@@ -89,6 +136,11 @@ msymbol_is_function (struct objfile *objfile, minimal_symbol *minsym,
 	  }
 	return false;
       }
+    case mst_file_text:
+      /* Ignore function symbol that is not a function entry.  */
+      if (msymbol_is_cold_clone (minsym))
+	return false;
+      /* fallthru */
     default:
       if (func_address_p != NULL)
 	*func_address_p = msym_addr;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ff26ba57d32..9b53b2c9659 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+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.
+
 2021-06-01  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.gdb/unittest.c: New file.
diff --git a/gdb/testsuite/gdb.cp/cold-clone.cc b/gdb/testsuite/gdb.cp/cold-clone.cc
new file mode 100644
index 00000000000..233a84a7ee7
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/cold-clone.cc
@@ -0,0 +1,54 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+#include <stdlib.h>
+#include "../lib/attributes.h"
+
+int a;
+int b;
+int c;
+
+static int __attribute__((used, noinline)) ATTRIBUTE_NOCLONE
+foo (void)
+{
+  a = 2;
+  if (b)
+    abort ();
+
+  return c;
+}
+
+static int __attribute__((used, noinline)) ATTRIBUTE_NOCLONE
+bar (void)
+{
+  a = 1;
+  if (c)
+    abort ();
+  return b;
+}
+
+int
+main (int argc, char **argv __attribute__((unused)))
+{
+  b = argc * 2;
+  c = argc / 2;
+
+  if (b + c == 5)
+    abort ();
+
+  return foo () + bar ();
+}
diff --git a/gdb/testsuite/gdb.cp/cold-clone.exp b/gdb/testsuite/gdb.cp/cold-clone.exp
new file mode 100644
index 00000000000..07b9bdc0a93
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/cold-clone.exp
@@ -0,0 +1,30 @@
+# 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/>.
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 {debug c++ optimize=-O2}]} {
+    return -1
+}
+
+gdb_test_multiple "break foo" "" {
+    -re -wrap "\\($decimal locations\\)" {
+	fail $gdb_test_name
+    }
+    -re -wrap "" {
+	pass $gdb_test_name
+    }
+}


More information about the Gdb-cvs mailing list