Re: The 'cold' function attribute and GDB

[Kevin Buettner <kevinb@redhat.com>]

On Sat, 04 May 2019 11:30:14 +0300
Eli Zaretskii <eliz@gnu.org> wrote:

> Do you want me to test some patch?

See below.
 
> And why doesn't the GNU/Linux executable have this minimal symbol in
> the first place, btw?

It turns out that the GNU/Linux executable does have this symbol.  The
difference is in its placement. The GNU/Linux executable placed the
.cold address before that of the entry pc for the function.  The windows
executable places it after the entry pc.  The code mentioned in my
earlier analysis prefers to use the minsym only when it's greater than
the entry pc.

My patch is below.  If it works for you and the approach seems sound,
I'll work on a test case which doesn't depend on gcc placing the
.cold section after the entry pc.

- - -

Prefer symtab symbol over minsym for function names in discontiguous blocks

The discussion on gdb-patches which led to this patch may be found
here:

https://www.sourceware.org/ml/gdb-patches/2019-05/msg00018.html

Here's a brief synopsis/analysis:

Eli Zaretskii, while debugging a Windows emacs executable, found
that functions comprised of more than one (non-contiguous)
address range were not being displayed correctly in a backtrace.  This
is the example that Eli provided:

  (gdb) bt
  #0  0x76a63227 in KERNELBASE!DebugBreak ()
     from C:\Windows\syswow64\KernelBase.dll
  #1  0x012e7b89 in emacs_abort () at w32fns.c:10768
  #2  0x012e1f3b in print_vectorlike.cold () at print.c:1824
  #3  0x011d2dec in print_object (obj=<optimized out>, printcharfun=XIL(0),
      escapeflag=true) at print.c:2150

The function print_vectorlike consists of two address ranges, one of
which contains "cold" code which is expected to not execute very often.
There is a minimal symbol, print_vectorlike.cold.65, which is the address
of the "cold" range.

GDB is prefering this minsym over the the name provided by the
DWARF info due to some really old code in GDB which handles
"certain pathological cases".  See the first big block comment
in find_frame_funname for more info.

I considered removing the code for this corner case entirely, but it
seems as though it might still be useful, so I left it intact.

That code is already disabled for inline functions.  I added a
condition which disables it for non-contiguous functions as well.

gdb/ChangeLog:

	* stack.c (find_frame_funname): Disable use of minsym for function
	name in functions comprised of non-contiguous blocks.
---
 gdb/stack.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index e5de10949d..66ba5ab43c 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1067,9 +1067,18 @@ find_frame_funname (struct frame_info *frame, enum language *funlang,
 
       struct bound_minimal_symbol msymbol;
 
-      /* Don't attempt to do this for inlined functions, which do not
-	 have a corresponding minimal symbol.  */
-      if (!block_inlined_p (SYMBOL_BLOCK_VALUE (func)))
+      /* Don't attempt to do this for two cases:
+
+	 1) Inlined functions, which do not have a corresponding minimal
+	    symbol.
+
+	 2) Functions which are comprised of non-contiguous blocks.
+	    Such functions often contain a minimal symbol for a
+	    "cold" range, i.e. code which is not expected to execute
+	    very often.  It is incorrect to use the minimal symbol
+	    associated with this range.  */
+      if (!block_inlined_p (SYMBOL_BLOCK_VALUE (func))
+          && BLOCK_CONTIGUOUS_P (SYMBOL_BLOCK_VALUE (func)))
 	msymbol
 	  = lookup_minimal_symbol_by_pc (get_frame_address_in_block (frame));
       else