[PATCH v2 4/4] Skip .cold functions in search_minsyms_for_name

[PATCH v2 4/4] Skip .cold functions in search_minsyms_for_name

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

When a function with the same name is also available,
that is preferred.

The .cold function is not the external interface.

Fixes 77f2120b200 ("Don't drop static function bp locations w/o debug info")

The motivation for this patch is that I became aware that the "real"
get_alias_set function (in gcc's cc1) had a two-PC breakpoint
location with gdb-9 but a three-PC breakpoint location with gdb-10.
The third breakpoint location was not an entry point but somewhere in the
error handling, which is named "get_alias_set.cold".

Bisection pointed to the above commit.  Prior to this commit the
static "get_alias_set.cold" was ignored, since there is also a global
get_alias_set function.  Basically the previous behavior was to ignore
any static function, when there is a global function with the same name.
The current behavior is to consider a static function and a global
function with the same name as different entities.

But a function named with ".cold" is not a real function at all.
It is just a wrapper for all the cold code paths in that function.
Therefore this patch checks if a function with the same name is
found and skip the ".cold" overload in that case.

gdb:
2020-11-30  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* linespec.c (search_minsyms_for_name): Filter .cold functions when
	a real function with the same name is found.

gdb/testsuite:
2020-11-30  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.cp/step-and-next-inline-1.exp: New test.

Thanks,
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-Skip-.cold-functions-in-search_minsyms_for_name.patch --]
[-- Type: text/x-patch; name="0004-Skip-.cold-functions-in-search_minsyms_for_name.patch", Size: 4416 bytes --]

From c0cbeb4165c92ae903d1ba2bc2e7756812dca764 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Mon, 30 Nov 2020 19:37:08 +0100
Subject: [PATCH v2 4/4] Skip .cold functions in search_minsyms_for_name

When a function with the same name is also available,
that is preferred.

The .cold function is not the external interface.

Fixes 77f2120b200 ("Don't drop static function bp locations w/o debug info")

The motivation for this patch is that I became aware that the "real"
get_alias_set function (in gcc's cc1) had a two-PC breakpoint
location with gdb-9 but a three-PC breakpoint location with gdb-10.
The third breakpoint location was not an entry point but somewhere in the
error handling, which is named "get_alias_set.cold".

Bisection pointed to the above commit.  Prior to this commit the
static "get_alias_set.cold" was ignored, since there is also a global
get_alias_set function.  Basically the previous behavior was to ignore
any static function, when there is a global function with the same name.
The current behavior is to consider a static function and a global
function with the same name as different entities.

But a function named with ".cold" is not a real function at all.
It is just a wrapper for all the cold code paths in that function.
Therefore this patch checks if a function with the same name is
found and skip the ".cold" overload in that case.

gdb:
2020-11-30  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* linespec.c (search_minsyms_for_name): Filter .cold functions when
	a real function with the same name is found.

gdb/testsuite:
2020-11-30  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.cp/step-and-next-inline-1.exp: New test.
---
 gdb/linespec.c                                  | 24 ++++++++++++++++++++
 gdb/testsuite/gdb.cp/step-and-next-inline-1.exp | 30 +++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 gdb/testsuite/gdb.cp/step-and-next-inline-1.exp

diff --git a/gdb/linespec.c b/gdb/linespec.c
index d088801..2b102a4 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -4393,6 +4393,30 @@ class symtab_collector
 	      break;
 	    }
 	}
+      else if (MSYMBOL_TYPE (item.minsym) == mst_file_text
+	       && strlen (item.minsym->linkage_name ()) > 5)
+	{
+	  size_t namelen = strlen (item.minsym->linkage_name ()) - 5;
+	  if (memcmp (item.minsym->linkage_name () + namelen, ".cold", 5) == 0)
+	    for (const bound_minimal_symbol &item2 : minsyms)
+	      {
+		if (&item2 == &item)
+		  continue;
+
+		if (MSYMBOL_TYPE (item2.minsym) != mst_file_text
+		    && MSYMBOL_TYPE (item2.minsym) != mst_text)
+		  continue;
+
+		if (strlen (item2.minsym->linkage_name ()) == namelen
+		    && memcmp (item.minsym->linkage_name (),
+			       item2.minsym->linkage_name (), namelen) != 0)
+		  continue;
+
+		/* found a real function with the same name as cold part.  */
+		skip = true;
+		break;
+	      }
+	}
 
       if (!skip)
 	info->result.minimal_symbols->push_back (item);
diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline-1.exp b/gdb/testsuite/gdb.cp/step-and-next-inline-1.exp
new file mode 100644
index 0000000..02200a4
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/step-and-next-inline-1.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 step-and-next-inline.cc
+
+if [get_compiler_info "c++"] {
+    unsupported "couldn't find a valid c++ compiler"
+    return -1
+}
+
+set options {c++ debug nowarnings optimize=-O2}
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile $options] } {
+    return -1
+}
+
+# Test that b get_alias_set sets only one breakpoint,
+# thus get_alias_set.cold is filtered away.
+gdb_test "b get_alias_set" ".*Breakpoint .*$srcfile, line .*" "test1"
-- 
1.9.1

Re: [PATCH v2 4/4] Skip .cold functions in search_minsyms_for_name

[Bernd Edlinger]

This patch is withdrawn since a similar patch was committed now:

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

    [gdb/symtab] Ignore cold clones


Bernd.

On 5/30/21 3:59 PM, Bernd Edlinger wrote:
> When a function with the same name is also available,
> that is preferred.
> 
> The .cold function is not the external interface.
> 
> Fixes 77f2120b200 ("Don't drop static function bp locations w/o debug info")
> 
> The motivation for this patch is that I became aware that the "real"
> get_alias_set function (in gcc's cc1) had a two-PC breakpoint
> location with gdb-9 but a three-PC breakpoint location with gdb-10.
> The third breakpoint location was not an entry point but somewhere in the
> error handling, which is named "get_alias_set.cold".
> 
> Bisection pointed to the above commit.  Prior to this commit the
> static "get_alias_set.cold" was ignored, since there is also a global
> get_alias_set function.  Basically the previous behavior was to ignore
> any static function, when there is a global function with the same name.
> The current behavior is to consider a static function and a global
> function with the same name as different entities.
> 
> But a function named with ".cold" is not a real function at all.
> It is just a wrapper for all the cold code paths in that function.
> Therefore this patch checks if a function with the same name is
> found and skip the ".cold" overload in that case.
> 
> gdb:
> 2020-11-30  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* linespec.c (search_minsyms_for_name): Filter .cold functions when
> 	a real function with the same name is found.
> 
> gdb/testsuite:
> 2020-11-30  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.cp/step-and-next-inline-1.exp: New test.
> 
> Thanks,
> Bernd.
>