[PATCH 0/4] Non-contiguous address range bug fixes / improvements

[PATCH 0/4] Non-contiguous address range bug fixes / improvements

[Kevin Buettner]

This four part series fixes some bugs associated with GDB's non-contiguous
address range support.

Kevin Buettner (4):
  Prefer symtab symbol over minsym for function names in non-contiguous
    blocks
  dwarf2-frame.c: Fix FDE processing bug involving non-contiguous ranges
  Allow display of negative offsets in print_address_symbolic()
  Improve test gdb.dwarf2/dw2-ranges-func.exp

 gdb/dwarf2-frame.c                            |   8 +-
 gdb/printcmd.c                                |   8 +-
 gdb/stack.c                                   |  15 +-
 ...anges-func.c => dw2-ranges-func-hi-cold.c} |  44 +-
 .../gdb.dwarf2/dw2-ranges-func-lo-cold.c      |  82 +++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp  | 694 ++++++++++--------
 6 files changed, 505 insertions(+), 346 deletions(-)
 rename gdb/testsuite/gdb.dwarf2/{dw2-ranges-func.c => dw2-ranges-func-hi-cold.c} (85%)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c

-- 
2.21.0

[PATCH 4/4] Improve test gdb.dwarf2/dw2-ranges-func.exp

[Kevin Buettner]

The original dw2-ranges-func.exp test caused a function named foo to be
created with two non-contiguous address ranges.  In the C source file,
a function named foo_low was incorporated into the function foo which
was also defined in that file.  The DWARF assembler is used to do this
manipulation.  The source file had been laid out so that foo_low would
likely be placed (by the compiler and linker) at a lower address than
foo().

The case where a range at a higher set of addresses (than foo) was not
being tested.  In a recent discussion on gdb-patches, it became clear
that performing such tests are desirable because bugs were discovered
which only became evident when the other range was located at high(er)
addresses than the range containing the entry point for the function.

This other (non entry pc) address range is typically used for "cold"
code which executes less frequently.  Thus, I renamed foo_low to
foo_cold and renamed the C source file from dw-ranges-func.c to
dw-ranges-func-lo.c.  I then made a copy of this file, naming it
dw-ranges-func-hi.c.  (That was my intent anyway.  According to git,
I renamed dw-ranges-func.c to dw-ranges-func-hi.c and then modified it.
dw-ranges-func-lo.c shows up as an entirely new file.)

Within dw-ranges-func-hi.c, I changed the placement of foo_cold()
along with some of the other functions so that foo_cold() would be at
a higher address than foo() while also remaining non-contiguous.  The
two files, dw-ranges-func-lo.c and dw-ranges-func-hi.c, are
essentially the same except for the placement of some of the functions
therein.

The tests in dw2-ranges-func.exp where then wrapped in a new proc named
do_test which was then called in a loop from the outermost level.  The
loop causes each of the source files to have the same tests run upon
them.

I also added a few new tests which test functionality fixed by the other
commits to this patch series.  Due to the reorganization of the file,
it's hard to identify these changes in the patch.  So, here are the
tests which were added:

    # Avoid duplicate test name for x/i below by adding another test
    # prefix.
    with_test_prefix "no-cold-names" {

	# Due to the calling sequence, this backtrace would normally
	# show function foo_cold for frame #1.  However, we don't want
	# this to be the case due to placing it in the same block
	# (albeit at a different range) as foo.  Thus it is correct to
	# see foo for frames #1 and #2.  It is incorrect to see
	# foo_cold at frame #1.
	gdb_test_sequence "bt" "backtrace from baz" {
	    "\[\r\n\]#0 .*? baz \\(\\) "
	    "\[\r\n\]#1 .*? foo \\(\\) "
	    "\[\r\n\]#2 .*? foo \\(\\) "
	    "\[\r\n\]#3 .*? main \\(\\) "
	}

	# Using the x command on foo_cold should show an address at an offset
	# from foo instead of foo_cold.  We also check to make sure that
	# the offset is not too large - we don't GDB to display really large
	# offsets that wrap around the address space.
	set foo_cold_offset 0
	set test "x/i foo_cold"
	gdb_test_multiple $test $test {
	    -re "   (?:$hex) <foo\[+-\](\[0-9\]+)>.*${gdb_prompt}" {
	        set foo_cold_offset $expect_out(1,string)
		pass $test
	    }
	}
	gdb_assert {$foo_cold_offset <= 10000} "offset to foo_cold is not too large"

	# Likewise, verify that addresses shown by "info line" are offsets
	# from foo instead of foo_cold.
	gdb_test "info line *foo_cold" "starts at address $hex <foo\[+-\].*?> and ends at $hex <foo\[+-\].*?>.*"

    }

When run against a GDB without the requisite bug fixes (from this patch
series), these 5 failures should be seen:

FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: no-cold-names: backtrace from baz (pattern 4)
FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: no-cold-names: offset to foo_cold is not too large
FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: no-cold-names: backtrace from baz (pattern 3)
FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: no-cold-names: x/i foo_cold
FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: no-cold-names: info line *foo_cold

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/dw2-ranges-func.c: Rename to...
	* gdb.dwarf2/dw2-ranges-func-lo-cold.c: ...this.
	* gdb.dwarf2/dw2-ranges-func-lo-cold.c (foo_low): Change name to
	foo_cold.  Revise comments to match.
	* gdb.dwarf2/dw2-ranges-func-hi-cold.c: New file.
	* gdb.dwarf2/dw2-ranges-func.exp (do_test): New proc. Existing tests
	were wrapped into this proc; Call do_test in loop from outermost
	level.
	(foo_low): Rename all occurrences to "foo_cold".
	(backtrace from baz): New test.
	(x/i foo_cold): New test.
	(info line *foo_cold): New test.
---
 ...anges-func.c => dw2-ranges-func-hi-cold.c} |  44 +-
 .../gdb.dwarf2/dw2-ranges-func-lo-cold.c      |  82 +++
 gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp  | 694 ++++++++++--------
 3 files changed, 481 insertions(+), 339 deletions(-)
 rename gdb/testsuite/gdb.dwarf2/{dw2-ranges-func.c => dw2-ranges-func-hi-cold.c} (85%)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-hi-cold.c
similarity index 85%
rename from gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c
rename to gdb/testsuite/gdb.dwarf2/dw2-ranges-func-hi-cold.c
index fa1b7bab41..06c3a3485a 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.c
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-hi-cold.c
@@ -16,7 +16,7 @@
 /* The idea here is to, via use of the dwarf assembler, create a function
    which occupies two non-contiguous address ranges.
 
-   foo_low and foo will be combined into a single function foo with a
+   foo_cold and foo will be combined into a single function foo with a
    function bar in between these two ranges.
 
    This test case was motivated by a bug in which a function which
@@ -37,19 +37,19 @@
 
 volatile int e = 0;
 
-void
-baz (void)
-{
-  asm ("baz_label: .globl baz_label");
-}						/* baz end */
+void bar (void);
+void foo_cold (void);
+void baz (void);
 
 void
-foo_low (void)
-{						/* foo_low prologue */
-  asm ("foo_low_label: .globl foo_low_label");
-  baz ();					/* foo_low baz call */
-  asm ("foo_low_label2: .globl foo_low_label2");
-}						/* foo_low end */
+foo (void)
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  bar ();					/* foo bar call */
+  asm ("foo_label2: .globl foo_label2");
+  if (e) foo_cold ();				/* foo foo_cold call */
+  asm ("foo_label3: .globl foo_label3");
+}						/* foo end */
 
 void
 bar (void)
@@ -58,14 +58,18 @@ bar (void)
 }						/* bar end */
 
 void
-foo (void)
-{						/* foo prologue */
-  asm ("foo_label: .globl foo_label");
-  bar ();					/* foo bar call */
-  asm ("foo_label2: .globl foo_label2");
-  if (e) foo_low ();				/* foo foo_low call */
-  asm ("foo_label3: .globl foo_label3");
-}						/* foo end */
+foo_cold (void)
+{						/* foo_cold prologue */
+  asm ("foo_cold_label: .globl foo_cold_label");
+  baz ();					/* foo_cold baz call */
+  asm ("foo_cold_label2: .globl foo_cold_label2");
+}						/* foo_cold end */
+
+void
+baz (void)
+{
+  asm ("baz_label: .globl baz_label");
+}						/* baz end */
 
 int
 main (void)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c
new file mode 100644
index 0000000000..40966ce6dc
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c
@@ -0,0 +1,82 @@
+/* Copyright 2018-2019 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/>.  */
+
+/* The idea here is to, via use of the dwarf assembler, create a function
+   which occupies two non-contiguous address ranges.
+
+   foo_cold and foo will be combined into a single function foo with a
+   function bar in between these two ranges.
+
+   This test case was motivated by a bug in which a function which
+   occupied two non-contiguous address ranges was calling another
+   function which resides in between these ranges.  So we end up with
+   a situation in which the low/start address of our constructed foo
+   (in this case) will be less than any of the addresses in bar, but
+   the high/end address of foo will be greater than any of bar's
+   addresses.
+
+   This situation was causing a problem in the caching code of
+   find_pc_partial_function:  When the low and high addresses of foo
+   are placed in the cache, the simple check that was used to see if
+   the cache was applicable would (incorrectly) succeed when presented
+   with an address in bar.  I.e. an address in bar presented as an
+   input to find_pc_partial_function could produce the answer "this
+   address belongs to foo".  */
+
+volatile int e = 0;
+
+void bar (void);
+void foo_cold (void);
+void baz (void);
+
+void
+baz (void)
+{
+  asm ("baz_label: .globl baz_label");
+}						/* baz end */
+
+void
+foo_cold (void)
+{						/* foo_cold prologue */
+  asm ("foo_cold_label: .globl foo_cold_label");
+  baz ();					/* foo_cold baz call */
+  asm ("foo_cold_label2: .globl foo_cold_label2");
+}						/* foo_cold end */
+
+void
+bar (void)
+{
+  asm ("bar_label: .globl bar_label");
+}						/* bar end */
+
+void
+foo (void)
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  bar ();					/* foo bar call */
+  asm ("foo_label2: .globl foo_label2");
+  if (e) foo_cold ();				/* foo foo_cold call */
+  asm ("foo_label3: .globl foo_label3");
+}						/* foo end */
+
+int
+main (void)
+{						/* main prologue */
+  asm ("main_label: .globl main_label");
+  foo ();					/* main foo call */
+  asm ("main_label2: .globl main_label2");
+  return 0;					/* main return */
+}						/* main end */
+
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
index ccf18b6983..b1d96e6488 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp
@@ -30,376 +30,432 @@ if !$gcc_compiled {
     return 0
 }
 
-standard_testfile dw2-ranges-func.c dw2-ranges-func-dw.S
-
-# We need to know the size of integer and address types in order to
-# write some of the debugging info we'd like to generate.
-#
-# For that, we ask GDB by debugging our test program.  Any program
-# would do, but since we already have it specifically for this
-# testcase, might as well use that.
+proc do_test {suffix} {
+    global gdb_test_file_name
+    global testfile binfile srcfile srcfile2 gdb_prompt hex
+
+    # Don't use standard_testfile; we want different binaries for
+    # each suffix.
+    set testfile $gdb_test_file_name-$suffix
+    set binfile [standard_output_file ${testfile}]
+    set srcfile $testfile.c
+    set srcfile2 $testfile-dw2.S
+
+    # We need to know the size of integer and address types in order to
+    # write some of the debugging info we'd like to generate.
+    #
+    # For that, we ask GDB by debugging our test program.  Any program
+    # would do, but since we already have it specifically for this
+    # testcase, might as well use that.
 
-if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
-    return -1
-}
+    if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+	return -1
+    }
 
-set asm_file [standard_output_file $srcfile2]
-Dwarf::assemble $asm_file {
-    global srcdir subdir srcfile srcfile2
-    declare_labels integer_label volatile_label func_ranges_label cu_ranges_label L
-    set int_size [get_sizeof "int" 4]
-
-    # Find start address and length for our functions.
-    lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
-	main_start main_len
-    set main_end "$main_start + $main_len"
-    lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
-	foo_start foo_len
-    set foo_end "$foo_start + $foo_len"
-    lassign [function_range foo_low [list ${srcdir}/${subdir}/$srcfile]] \
-	foo_low_start foo_low_len
-    set foo_low_end "$foo_low_start + $foo_low_len"
-    lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
-	bar_start bar_len
-    set bar_end "$bar_start + $bar_len"
-    lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
-	baz_start baz_len
-    set baz_end "$baz_start + $baz_len"
-
-    set e_var [gdb_target_symbol e]
-
-    cu {} {
-	compile_unit {
-	    {language @DW_LANG_C}
-	    {name dw-ranges-func.c}
-	    {stmt_list $L DW_FORM_sec_offset}
-	    {low_pc 0 addr}
-	    {ranges ${cu_ranges_label} DW_FORM_sec_offset}
-	} {
-	    integer_label: DW_TAG_base_type {
-		{DW_AT_byte_size $int_size DW_FORM_sdata}
-		{DW_AT_encoding  @DW_ATE_signed}
-		{DW_AT_name      integer}
-	    }
-	    volatile_label: DW_TAG_volatile_type {
-		{type :$integer_label}
-	    }
-	    DW_TAG_variable {
-		{name e}
-		{external 1 flag}
-		{type :$volatile_label}
-		{location {addr $e_var} SPECIAL_expr}
+    set asm_file [standard_output_file $srcfile2]
+    Dwarf::assemble $asm_file {
+	global srcdir subdir srcfile srcfile2
+	declare_labels integer_label volatile_label func_ranges_label cu_ranges_label L
+	set int_size [get_sizeof "int" 4]
+
+	# Find start address and length for our functions.
+	lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	    main_start main_len
+	set main_end "$main_start + $main_len"
+	lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
+	    foo_start foo_len
+	set foo_end "$foo_start + $foo_len"
+	lassign [function_range foo_cold [list ${srcdir}/${subdir}/$srcfile]] \
+	    foo_cold_start foo_cold_len
+	set foo_cold_end "$foo_cold_start + $foo_cold_len"
+	lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
+	    bar_start bar_len
+	set bar_end "$bar_start + $bar_len"
+	lassign [function_range baz [list ${srcdir}/${subdir}/$srcfile]] \
+	    baz_start baz_len
+	set baz_end "$baz_start + $baz_len"
+
+	set e_var [gdb_target_symbol e]
+
+	cu {} {
+	    compile_unit {
+		{language @DW_LANG_C}
+		{name dw-ranges-func2.c}
+		{stmt_list $L DW_FORM_sec_offset}
+		{low_pc 0 addr}
+		{ranges ${cu_ranges_label} DW_FORM_sec_offset}
+	    } {
+		integer_label: DW_TAG_base_type {
+		    {DW_AT_byte_size $int_size DW_FORM_sdata}
+		    {DW_AT_encoding  @DW_ATE_signed}
+		    {DW_AT_name      integer}
+		}
+		volatile_label: DW_TAG_volatile_type {
+		    {type :$integer_label}
+		}
+		DW_TAG_variable {
+		    {name e}
+		    {external 1 flag}
+		    {type :$volatile_label}
+		    {location {addr $e_var} SPECIAL_expr}
+		}
+		subprogram {
+		    {external 1 flag}
+		    {name main}
+		    {DW_AT_type :$integer_label}
+		    {low_pc $main_start addr}
+		    {high_pc $main_len DW_FORM_data4}
+		}
+		subprogram {
+		    {external 1 flag}
+		    {name foo}
+		    {ranges ${func_ranges_label} DW_FORM_sec_offset}
+		}
+		subprogram {
+		    {external 1 flag}
+		    {name bar}
+		    {low_pc $bar_start addr}
+		    {high_pc $bar_len DW_FORM_data4}
+		}
+		subprogram {
+		    {external 1 flag}
+		    {name baz}
+		    {low_pc $baz_start addr}
+		    {high_pc $baz_len DW_FORM_data4}
+		}
 	    }
-	    subprogram {
-		{external 1 flag}
-		{name main}
-		{DW_AT_type :$integer_label}
-		{low_pc $main_start addr}
-		{high_pc $main_len DW_FORM_data4}
-	    }
-	    subprogram {
-		{external 1 flag}
-		{name foo}
-		{ranges ${func_ranges_label} DW_FORM_sec_offset}
+	}
+
+	lines {version 2} L {
+	    include_dir "${srcdir}/${subdir}"
+	    file_name "$srcfile" 1
+
+	    # Generate a line table program.  An attempt was made to make it
+	    # reasonably accurate as it made debugging the test case easier.
+	    program {
+		{DW_LNE_set_address $main_start}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "main prologue"] - 1]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address main_label}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "main foo call"] - [gdb_get_line_number "main prologue"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address main_label2}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "main return"] - [gdb_get_line_number "main foo call"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address $main_end}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "main end"] - [gdb_get_line_number "main return"] + 1]}
+		{DW_LNS_copy}
+		{DW_LNE_end_sequence}
+
+		{DW_LNE_set_address $foo_start}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo prologue"] - 1] }
+		{DW_LNS_copy}
+		{DW_LNE_set_address foo_label}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo bar call"] - [gdb_get_line_number "foo prologue"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address foo_label2}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_cold call"] - [gdb_get_line_number "foo bar call"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address foo_label3}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_cold call"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address $foo_end}
+		{DW_LNS_advance_line 1}
+		{DW_LNS_copy}
+		{DW_LNE_end_sequence}
+
+		{DW_LNE_set_address $bar_start}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "bar end"] - 1]}
+		{DW_LNS_copy}
+		{DW_LNS_advance_pc $bar_len}
+		{DW_LNS_advance_line 1}
+		{DW_LNS_copy}
+		{DW_LNE_end_sequence}
+
+		{DW_LNE_set_address $baz_start}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "baz end"] - 1]}
+		{DW_LNS_copy}
+		{DW_LNS_advance_pc $baz_len}
+		{DW_LNS_advance_line 1}
+		{DW_LNS_copy}
+		{DW_LNE_end_sequence}
+
+		{DW_LNE_set_address $foo_cold_start}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold prologue"] - 1]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address foo_cold_label}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold baz call"] - [gdb_get_line_number "foo_cold prologue"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address foo_cold_label2}
+		{DW_LNS_advance_line [expr [gdb_get_line_number "foo_cold end"] - [gdb_get_line_number "foo_cold baz call"]]}
+		{DW_LNS_copy}
+		{DW_LNE_set_address $foo_cold_end}
+		{DW_LNS_advance_line 1}
+		{DW_LNS_copy}
+		{DW_LNE_end_sequence}
 	    }
-	    subprogram {
-		{external 1 flag}
-		{name bar}
-		{low_pc $bar_start addr}
-		{high_pc $bar_len DW_FORM_data4}
+	}
+
+	# Generate ranges data.
+	ranges {is_64 [is_64_target]} {
+	    func_ranges_label: sequence {
+		{range {$foo_start } $foo_end}
+		{range {$foo_cold_start} $foo_cold_end}
 	    }
-	    subprogram {
-		{external 1 flag}
-		{name baz}
-		{low_pc $baz_start addr}
-		{high_pc $baz_len DW_FORM_data4}
+	    cu_ranges_label: sequence {
+		{range {$foo_start } $foo_end}
+		{range {$foo_cold_start} $foo_cold_end}
+		{range {$main_start} $main_end}
+		{range {$bar_start} $bar_end}
+		{range {$baz_start} $baz_end}
 	    }
 	}
     }
 
-    lines {version 2} L {
-	include_dir "${srcdir}/${subdir}"
-	file_name "$srcfile" 1
-
-	# Generate a line table program.  An attempt was made to make it
-	# reasonably accurate as it made debugging the test case easier.
-	program {
-	    {DW_LNE_set_address $main_start}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "main prologue"] - 1]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address main_label}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "main foo call"] - [gdb_get_line_number "main prologue"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address main_label2}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "main return"] - [gdb_get_line_number "main foo call"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address $main_end}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "main end"] - [gdb_get_line_number "main return"] + 1]}
-	    {DW_LNS_copy}
-	    {DW_LNE_end_sequence}
-
-	    {DW_LNE_set_address $foo_start}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo prologue"] - 1] }
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address foo_label}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo bar call"] - [gdb_get_line_number "foo prologue"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address foo_label2}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo foo_low call"] - [gdb_get_line_number "foo bar call"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address foo_label3}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo end"] - [gdb_get_line_number "foo foo_low call"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address $foo_end}
-	    {DW_LNS_advance_line 1}
-	    {DW_LNS_copy}
-	    {DW_LNE_end_sequence}
-
-	    {DW_LNE_set_address $bar_start}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "bar end"] - 1]}
-	    {DW_LNS_copy}
-	    {DW_LNS_advance_pc $bar_len}
-	    {DW_LNS_advance_line 1}
-	    {DW_LNS_copy}
-	    {DW_LNE_end_sequence}
-
-	    {DW_LNE_set_address $baz_start}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "baz end"] - 1]}
-	    {DW_LNS_copy}
-	    {DW_LNS_advance_pc $baz_len}
-	    {DW_LNS_advance_line 1}
-	    {DW_LNS_copy}
-	    {DW_LNE_end_sequence}
-
-	    {DW_LNE_set_address $foo_low_start}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low prologue"] - 1]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address foo_low_label}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low baz call"] - [gdb_get_line_number "foo_low prologue"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address foo_low_label2}
-	    {DW_LNS_advance_line [expr [gdb_get_line_number "foo_low end"] - [gdb_get_line_number "foo_low baz call"]]}
-	    {DW_LNS_copy}
-	    {DW_LNE_set_address $foo_low_end}
-	    {DW_LNS_advance_line 1}
-	    {DW_LNS_copy}
-	    {DW_LNE_end_sequence}
-	}
+    if { [prepare_for_testing "failed to prepare" ${testfile} \
+	      [list $srcfile $asm_file] {nodebug}] } {
+	return -1
     }
 
-    # Generate ranges data.
-    ranges {is_64 [is_64_target]} {
-	func_ranges_label: sequence {
-	    {range {$foo_start } $foo_end}
-	    {range {$foo_low_start} $foo_low_end}
-	}
-	cu_ranges_label: sequence {
-	    {range {$foo_start } $foo_end}
-	    {range {$foo_low_start} $foo_low_end}
-	    {range {$main_start} $main_end}
-	    {range {$bar_start} $bar_end}
-	    {range {$baz_start} $baz_end}
-	}
+    if ![runto_main] {
+	return -1
     }
-}
 
-if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile $asm_file] {nodebug}] } {
-    return -1
-}
+    set main_prologue_line_num [gdb_get_line_number "main prologue"]
+    # Do a sanity check to make sure that line number info is available.
+    gdb_test "info line main" \
+	"Line ${main_prologue_line_num} of .* starts at address .* and ends at .*"
 
-if ![runto_main] {
-    return -1
-}
+    with_test_prefix "step-test-1" {
+	set bp_foo_bar [gdb_get_line_number "foo bar call"]
 
-set main_prologue_line_num [gdb_get_line_number "main prologue"]
-# Do a sanity check to make sure that line number info is available.
-gdb_test "info line main" \
-    "Line ${main_prologue_line_num} of .* starts at address .* and ends at .*"
+	gdb_test "break $bp_foo_bar" \
+	    "Breakpoint.*at.* file .*$srcfile, line $bp_foo_bar\\." \
+	    "break at call to bar"
 
-with_test_prefix "step-test-1" {
-    set bp_foo_bar [gdb_get_line_number "foo bar call"]
+	gdb_test "continue" \
+	    "Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\).*$bp_foo_bar\\s+bar\\s\\(\\);.*foo bar call.*" \
+	    "continue to call of bar"
 
-    gdb_test "break $bp_foo_bar" \
-	"Breakpoint.*at.* file .*$srcfile, line $bp_foo_bar\\." \
-	"break at call to bar"
+	gdb_test "step" \
+	    "bar \\(\\).*bar end.*" \
+	    "step into bar"
 
-    gdb_test "continue" \
-	"Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\).*$bp_foo_bar\\s+bar\\s\\(\\);.*foo bar call.*" \
-	"continue to call of bar"
+	gdb_test "step" \
+	    "foo \\(\\).*foo foo_cold call.*" \
+	    "step out of bar, back into foo"
+    }
 
-    gdb_test "step" \
-	"bar \\(\\).*bar end.*" \
-	"step into bar"
+    with_test_prefix "step-test-2" {
+	clean_restart ${testfile}
+	if ![runto_main] {
+	    return -1
+	}
 
-    gdb_test "step" \
-	"foo \\(\\).*foo foo_low call.*" \
-	"step out of bar, back into foo"
-}
+	# Note that the RE used for the following test will fail when the
+	# breakpoint has been set on multiple locations. E.g. "(2 locations)". 
+	# This is intentional since that behavior is one of the bugs that
+	# this test case tests for.
+	gdb_test "break foo" \
+	    "Breakpoint.*at.* file .*$srcfile, line \\d+\\." \
+	    "break foo"
+
+	# Continue to foo.  Allow execution to stop either on the prologue
+	# or on the call to bar since either behavior is acceptable though
+	# the latter is preferred.
+	set test "continue to foo"
+	gdb_test_multiple "continue" $test {
+	    -re "Breakpoint \\d+, foo \\(\\).*foo prologue.*${gdb_prompt}" {
+		pass $test
+		gdb_test "step" \
+			 "foo bar call .*" \
+			 "step to call of bar after landing on prologue"
+	    }
+	    -re "Breakpoint \\d+, foo \\(\\).*foo bar call.*${gdb_prompt}" {
+		pass $test
+	    }
+	}
+
+	gdb_test "step" \
+	    "bar \\(\\).*bar end.*" \
+	    "step into bar"
+
+	gdb_test "step" \
+	    "foo \\(\\).*foo foo_cold call.*" \
+	    "step out of bar, back into foo"
+    }
 
-with_test_prefix "step-test-2" {
     clean_restart ${testfile}
     if ![runto_main] {
 	return -1
     }
 
-    # Note that the RE used for the following test will fail when the
-    # breakpoint has been set on multiple locations. E.g. "(2 locations)". 
-    # This is intentional since that behavior is one of the bugs that
-    # this test case tests for.
-    gdb_test "break foo" \
-	"Breakpoint.*at.* file .*$srcfile, line \\d+\\." \
-	"break foo"
-
-    # Continue to foo.  Allow execution to stop either on the prologue
-    # or on the call to bar since either behavior is acceptable though
-    # the latter is preferred.
-    set test "continue to foo"
-    gdb_test_multiple "continue" $test {
-	-re "Breakpoint \\d+, foo \\(\\).*foo prologue.*${gdb_prompt}" {
+    # Disassembly of foo should have multiple address ranges.
+    gdb_test_sequence "disassemble foo" "" [list \
+	"Dump of assembler code for function foo:" \
+	"Address range $hex to $hex:" \
+	"   $hex <\\+0>:" \
+	"Address range $hex to $hex:" \
+	"   $hex <(.+?)>:" \
+	"End of assembler dump\\." \
+    ]
+
+    set foo_cold_addr -1
+    set test "x/i foo_cold"
+    gdb_test_multiple $test $test {
+	-re "   ($hex) <foo.*?>.*${gdb_prompt}" {
+	    set foo_cold_addr $expect_out(1,string)
 	    pass $test
-	    gdb_test "step" \
-		     "foo bar call .*" \
-		     "step to call of bar after landing on prologue"
 	}
-	-re "Breakpoint \\d+, foo \\(\\).*foo bar call.*${gdb_prompt}" {
+    }
+
+    set foo_addr -1
+    set test "x/i foo"
+    gdb_test_multiple $test $test {
+	-re "   ($hex) <foo.*?>.*${gdb_prompt}" {
+	    set foo_addr $expect_out(1,string)
 	    pass $test
 	}
     }
 
-    gdb_test "step" \
-	"bar \\(\\).*bar end.*" \
-	"step into bar"
-
-    gdb_test "step" \
-	"foo \\(\\).*foo foo_low call.*" \
-	"step out of bar, back into foo"
-}
+    gdb_assert {$foo_cold_addr != $foo_addr} "foo and foo_cold are at different addresses"
 
-clean_restart ${testfile}
-if ![runto_main] {
-    return -1
-}
+    # This more permissive RE for "break foo" will allow a breakpoint on
+    # multiple locations to PASS.  */
+    gdb_test "break foo" \
+	"Breakpoint.*at.*" \
+	"break foo"
 
-# Disassembly of foo should have multiple address ranges.
-gdb_test_sequence "disassemble foo" "" [list \
-    "Dump of assembler code for function foo:" \
-    "Address range $hex to $hex:" \
-    "   $hex <\\+0>:" \
-    "Address range $hex to $hex:" \
-    "   $hex <(.+?)>:" \
-    "End of assembler dump\\." \
-]
-
-set foo_low_addr -1
-set test "x/i foo_low"
-gdb_test_multiple $test $test {
-    -re "   ($hex) <foo.*?>.*${gdb_prompt}" {
-	set foo_low_addr $expect_out(1,string)
-	pass $test
-    }
-}
+    gdb_test "break baz" \
+	"Breakpoint.*at.* file .*$srcfile, line \\d+\\."
 
-set foo_addr -1
-set test "x/i foo"
-gdb_test_multiple $test $test {
-    -re "   ($hex) <foo.*?>.*${gdb_prompt}" {
-	set foo_addr $expect_out(1,string)
-	pass $test
-    }
-}
+    gdb_test "continue" \
+	"Breakpoint \\d+, foo \\(\\).*" \
+	"continue to foo"
 
-gdb_assert {$foo_low_addr != $foo_addr} "foo and foo_low are at different addresses"
+    gdb_test_no_output "set variable e=1"
 
-# This more permissive RE for "break foo" will allow a breakpoint on
-# multiple locations to PASS.  */
-gdb_test "break foo" \
-    "Breakpoint.*at.*" \
-    "break foo"
+    # If GDB incorrectly places the foo breakpoint on multiple locations,
+    # then GDB will (incorrectly) stop in foo_cold instead of in baz.
+    gdb_test "continue" \
+	"Breakpoint \\d+, (?:$hex in )?baz \\(\\).*" \
+	"continue to baz"
+
+    # Avoid duplicate test name for x/i below by adding another test
+    # prefix.
+    with_test_prefix "no-cold-names" {
+
+	# Due to the calling sequence, this backtrace would normally
+	# show function foo_cold for frame #1.  However, we don't want
+	# this to be the case due to placing it in the same block
+	# (albeit at a different range) as foo.  Thus it is correct to
+	# see foo for frames #1 and #2.  It is incorrect to see
+	# foo_cold at frame #1.
+	gdb_test_sequence "bt" "backtrace from baz" {
+	    "\[\r\n\]#0 .*? baz \\(\\) "
+	    "\[\r\n\]#1 .*? foo \\(\\) "
+	    "\[\r\n\]#2 .*? foo \\(\\) "
+	    "\[\r\n\]#3 .*? main \\(\\) "
+	}
 
-gdb_test "break baz" \
-    "Breakpoint.*at.* file .*$srcfile, line \\d+\\."
+	# Using the x command on foo_cold should show an address at an offset
+	# from foo instead of foo_cold.  We also check to make sure that
+	# the offset is not too large - we don't GDB to display really large
+	# offsets that wrap around the address space.
+	set foo_cold_offset 0
+	set test "x/i foo_cold"
+	gdb_test_multiple $test $test {
+	    -re "   (?:$hex) <foo\[+-\](\[0-9\]+)>.*${gdb_prompt}" {
+	        set foo_cold_offset $expect_out(1,string)
+		pass $test
+	    }
+	}
+	gdb_assert {$foo_cold_offset <= 10000} "offset to foo_cold is not too large"
 
-gdb_test "continue" \
-    "Breakpoint \\d+, foo \\(\\).*" \
-    "continue to foo"
+	# Likewise, verify that addresses shown by "info line" are offsets
+	# from foo instead of foo_cold.
+	gdb_test "info line *foo_cold" "starts at address $hex <foo\[+-\].*?> and ends at $hex <foo\[+-\].*?>.*"
 
-gdb_test_no_output "set variable e=1"
+    }
 
-# If GDB incorrectly places the foo breakpoint on multiple locations,
-# then GDB will (incorrectly) stop in foo_low instead of in baz.
-gdb_test "continue" \
-    "Breakpoint \\d+, (?:$hex in )?baz \\(\\).*" \
-    "continue to baz"
+    with_test_prefix "step-test-3" {
+	clean_restart ${testfile}
+	if ![runto_main] {
+	    return -1
+	}
 
-with_test_prefix "step-test-3" {
-    clean_restart ${testfile}
-    if ![runto_main] {
-	return -1
-    }
+	gdb_test "step" \
+		 "foo \\(\\).*bar \\(\\);.*foo bar call.*" \
+		 "step into foo from main"
 
-    gdb_test "step" \
-	     "foo \\(\\).*bar \\(\\);.*foo bar call.*" \
-	     "step into foo from main"
-
-    gdb_test "step" \
-	     "bar \\(\\).*\}.* bar end.*" \
-	     "step into bar from foo"
-
-    gdb_test "step" \
-	     "foo(_label2)? \\(\\).*foo_low \\(\\);.*foo foo_low call.*" \
-	     "step out of bar to foo"
-
-    # The tests in the "enable_foo_low_stepping" section, below, work
-    # with some versions of gcc, though it's not clear that they
-    # should.  This test case causes foo_low, originally a separate
-    # function invoked via a subroutine call, to be considered as part
-    # of foo via use of DW_AT_ranges.  Real code that I've looked at
-    # uses a branch instruction to cause code in the "cold" range to
-    # be executed. 
-    #
-    # For the moment though, these tests have been left in place, but
-    # disabled, in case we decide that making such a subroutine call
-    # is a reasonable thing to do that should also be supported by
-    # GDB.
+	gdb_test "step" \
+		 "bar \\(\\).*\}.* bar end.*" \
+		 "step into bar from foo"
 
-    set enable_foo_low_stepping false
+	gdb_test "step" \
+		 "foo(_label2)? \\(\\).*foo_cold \\(\\);.*foo foo_cold call.*" \
+		 "step out of bar to foo"
+
+	# The tests in the "enable_foo_cold_stepping" section, below, work
+	# with some versions of gcc, though it's not clear that they
+	# should.  This test case causes foo_cold, originally a separate
+	# function invoked via a subroutine call, to be considered as part
+	# of foo via use of DW_AT_ranges.  Real code that I've looked at
+	# uses a branch instruction to cause code in the "cold" range to
+	# be executed. 
+	#
+	# For the moment though, these tests have been left in place, but
+	# disabled, in case we decide that making such a subroutine call
+	# is a reasonable thing to do that should also be supported by
+	# GDB.
+
+	set enable_foo_cold_stepping false
+
+	if { $enable_foo_cold_stepping } {
+	    gdb_test_no_output "set variable e=1"
+
+	    set test "step into foo_cold from foo"
+	    gdb_test_multiple "step" $test {
+		-re "foo(_low)? \\(\\).*\{.*foo_cold prologue.*${gdb_prompt}" {
+		    pass $test
+		    gdb_test "step" \
+			     "foo \\(\\).*baz \\(\\);.*foo_cold baz call.*" \
+			     "step to baz call in foo_cold"
+
+		}
+		-re "foo(_low)? \\(\\).*baz \\(\\);.*foo_cold baz call.*${gdb_prompt}" {
+		    pass $test
+		}
+	    }
 
-    if { $enable_foo_low_stepping } {
-	gdb_test_no_output "set variable e=1"
+	    gdb_test "step" \
+		     "baz \\(\\).*\}.*baz end.*" \
+		     "step into baz from foo_cold"
 
-	set test "step into foo_low from foo"
-	gdb_test_multiple "step" $test {
-	    -re "foo(_low)? \\(\\).*\{.*foo_low prologue.*${gdb_prompt}" {
-		pass $test
-		gdb_test "step" \
-			 "foo \\(\\).*baz \\(\\);.*foo_low baz call.*" \
-			 "step to baz call in foo_low"
+	    gdb_test "step" \
+		     "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_cold end.*" \
+		     "step out of baz to foo_cold"
 
-	    }
-	    -re "foo(_low)? \\(\\).*baz \\(\\);.*foo_low baz call.*${gdb_prompt}" {
-		pass $test
-	    }
+	    gdb_test "step" \
+		     "foo(?:_label3)? \\(\\).*\}.*foo end.*" \
+		     "step out of foo_cold to foo"
+	} else {
+	    gdb_test "next" \
+		     ".*foo end.*" \
+		     "next over foo_cold call"
 	}
 
 	gdb_test "step" \
-		 "baz \\(\\).*\}.*baz end.*" \
-		 "step into baz from foo_low"
+		 "main(?:_label2)? \\(\\).*" \
+		 "step out of foo to main"
+    }
+}
 
-	gdb_test "step" \
-		 "foo(?:_low(?:_label2)?)? \\(\\).*\}.*foo_low end.*" \
-		 "step out of baz to foo_low"
+# foreach_with_prefix could be used here, but the log file output is somewhat
+# less verbose when using an explicit "with_test_prefix".
 
-	gdb_test "step" \
-		 "foo(?:_label3)? \\(\\).*\}.*foo end.*" \
-		 "step out of foo_low to foo"
-    } else {
-	gdb_test "next" \
-		 ".*foo end.*" \
-		 "next over foo_low call"
+foreach test_suffix { "lo-cold" "hi-cold" } {
+    with_test_prefix $test_suffix {
+	do_test $test_suffix
     }
-
-    gdb_test "step" \
-	     "main(?:_label2)? \\(\\).*" \
-	     "step out of foo to main"
 }
-- 
2.21.0

[PATCH 1/4] Prefer symtab symbol over minsym for function names in non-contiguous blocks

[Kevin Buettner]

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 information.

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.

The change to find_frame_funname in stack.c fixes this problem for
stack traces.  A similar change to print_address_symbolic in
printcmd.c fixes this problem for the "x" command and other commands
which use print_address_symbolic().

gdb/ChangeLog:

	* stack.c (find_frame_funname): Disable use of minsym for function
	name in functions comprised of non-contiguous blocks.
	* printcmd.c (print_address_symbolic): Likewise.
---
 gdb/printcmd.c |  4 +++-
 gdb/stack.c    | 15 ++++++++++++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 9e84594fe6..e00a9c671a 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -627,7 +627,9 @@ build_address_symbolic (struct gdbarch *gdbarch,
 
   if (msymbol.minsym != NULL)
     {
-      if (BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL)
+      if (symbol == NULL
+          || (BLOCK_CONTIGUOUS_P (SYMBOL_BLOCK_VALUE (symbol))
+	      && BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location))
 	{
 	  /* If this is a function (i.e. a code address), strip out any
 	     non-address bits.  For instance, display a pointer to the
diff --git a/gdb/stack.c b/gdb/stack.c
index 408c795e38..d511690a14 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
-- 
2.21.0

[PATCH 2/4] dwarf2-frame.c: Fix FDE processing bug involving non-contiguous ranges

[Kevin Buettner]

In the course of revising the test case for
gdb.dwarf2/dw2-ranges-func.exp, I added a new .c file which would
cause the "cold" range to be at a higher address than the rest of the
function.  In these tests, the range in question isn't really cold in
the sense that a compiler has determined that it'll be executed less
frequently.  Instead, it's simply the range that does not include the
entry pc.  These tests are intended to mimic the output of such a
compiler, so I'll continue to refer to this range as "cold" in the
following discussion.

The original test case had only tested a cold range placed
at lower addresses than the rest of the function.  During testing of the
new code where the cold range was placed at higher addresses, I found
that I could produce the following backtrace:

    (gdb) bt
    #0  0x0000000000401138 in baz ()
	at dw2-ranges-func-hi-cold.c:72
    #1  0x0000000000401131 in foo_cold ()
	at dw2-ranges-func-hi-cold.c:64
    #2  0x000000000040111e in foo ()
	at dw2-ranges-func-hi-cold.c:50
    #3  0x0000000000401144 in main ()
	at dw2-ranges-func-hi-cold.c:78

This is correct, except that we'd like to see foo() listed instead
of foo_cold().  (I handle that problem in another patch.)

Now look at what happens for a similar backtrace where the cold range
is at a lower address than the foo's entry pc:

    (gdb) bt
    #0  0x000000000040110a in baz ()
	at dw2-ranges-func-lo-cold.c:48
    #1  0x0000000000401116 in foo ()
	at dw2-ranges-func-lo-cold.c:54
    #2  0x00007fffffffd4c0 in ?? ()
    #3  0x0000000000401138 in foo ()
	at dw2-ranges-func-lo-cold.c:70

Note that the backtrace doesn't go all the way back to main().  Moreover,
frame #2 is messed up.

I had seen this behavior when I had worked on the non-contiguous
address problem last year.  At the time I convinced myself that the
mangled backtrace was "okay" since we're doing strange things with
the DWARF assembler.  We're taking a function called foo_cold (though
it was originally called foo_low - my recent changes to the test case
changed the name) and via the magic of the DWARF assembler, we're
combining it into a separate (non-contiguous) range for foo.  Thus,
it was a surprise to me when I got a good and complete backtrace when
the cold symbol is placed at an address that's greater than entry pc.

The function dwarf2_frame_cache (in dwarf2-frame.c) is making this
call:

    if (get_frame_func_if_available (this_frame, &entry_pc)) ...

If that call succeeds (returns a true value), the FDE is then
processed up to the entry pc.  It doesn't make sense to do this,
however, when the FDE in question does not contain the entry pc.  This
can happen when the function in question is comprised of more than one
(non-contiguous) address range.

My fix is to add some comparisons to the test above to ensure that
ENTRY_PC is within the address range covered by the FDE.

gdb/ChangeLog:

	* dwarf2-frame.c (dwarf2_frame_cache): Don't decode FDE instructions
	for entry pc when entry pc is out of range for that FDE.
---
 gdb/dwarf2-frame.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 0f2502b4c6..c98e2f232f 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1023,7 +1023,13 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
   /* Save the initialized register set.  */
   fs.initial = fs.regs;
 
-  if (get_frame_func_if_available (this_frame, &entry_pc))
+  /* Fetching the entry pc for THIS_FRAME won't necessarily result
+     in an address that's within the range of FDE locations.  This
+     is due to the possibility of the function occupying non-contiguous
+     ranges.  */
+  if (get_frame_func_if_available (this_frame, &entry_pc)
+      && fde->initial_location <= entry_pc
+      && entry_pc < fde->initial_location + fde->address_range)
     {
       /* Decode the insns in the FDE up to the entry PC.  */
       instr = execute_cfa_program (fde, fde->instructions, fde->end, gdbarch,
-- 
2.21.0

[PATCH 3/4] Allow display of negative offsets in print_address_symbolic()

[Kevin Buettner]

When examining addresses associated with blocks with non-contiguous
address ranges, it's not uncommon to see large positive offsets which,
for some address width, actually represent a smaller negative offset.
Here's an example taken from the test case:

    (gdb) x/i foo_cold
       0x40110d <foo+4294967277>:	push   %rbp

This commit causes cases like the above to be displayed like this (below)
instead:

    (gdb) x/i foo_cold
       0x40110d <foo-19>:	push   %rbp

gdb/ChangeLog:

	* printcmd.c (print_address_symbolic): Print negative offsets.
	(build_address_symbolic): Force signed arithmetic when computing
	offset.
---
 gdb/printcmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index e00a9c671a..8ceddd633a 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -537,7 +537,7 @@ print_address_symbolic (struct gdbarch *gdbarch, CORE_ADDR addr,
     fputs_filtered ("<", stream);
   fputs_styled (name.c_str (), function_name_style.style (), stream);
   if (offset != 0)
-    fprintf_filtered (stream, "+%u", (unsigned int) offset);
+    fprintf_filtered (stream, "%+d", offset);
 
   /* Append source filename and line number if desired.  Give specific
      line # of this addr, if we have it; else line # of the nearest symbol.  */
@@ -666,7 +666,7 @@ build_address_symbolic (struct gdbarch *gdbarch,
       && name_location + max_symbolic_offset > name_location)
     return 1;
 
-  *offset = addr - name_location;
+  *offset = (LONGEST) addr - name_location;
 
   *name = name_temp;
 
-- 
2.21.0

Re: [PATCH 1/4] Prefer symtab symbol over minsym for function names in non-contiguous blocks

[Pedro Alves]

On 6/8/19 8:54 PM, Kevin Buettner wrote:
> 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 information.

Yuck!

> 
> 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.
> 

Yeah, I'd be inclined to try removing it too.  The comment
smells of a.out or stabs limitations.  But I'm not 100% sure,
and I'm sympathetic with forward incremental progress.

> That code is already disabled for inline functions.  I added a
> condition which disables it for non-contiguous functions as well.
> 
> The change to find_frame_funname in stack.c fixes this problem for
> stack traces.  A similar change to print_address_symbolic in
> printcmd.c fixes this problem for the "x" command and other commands
> which use print_address_symbolic().
> 
> gdb/ChangeLog:
> 
> 	* stack.c (find_frame_funname): Disable use of minsym for function
> 	name in functions comprised of non-contiguous blocks.
> 	* printcmd.c (print_address_symbolic): Likewise.
> ---
>  gdb/printcmd.c |  4 +++-
>  gdb/stack.c    | 15 ++++++++++++---
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 9e84594fe6..e00a9c671a 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -627,7 +627,9 @@ build_address_symbolic (struct gdbarch *gdbarch,
>  
>    if (msymbol.minsym != NULL)
>      {
> -      if (BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location || symbol == NULL)
> +      if (symbol == NULL
> +          || (BLOCK_CONTIGUOUS_P (SYMBOL_BLOCK_VALUE (symbol))
> +	      && BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location))

I think this warrants a comment somewhere.  Maybe a bit higher up,
where it reads:

  /* First try to find the address in the symbol table, then
     in the minsyms.  Take the closest one.  */

Or better yet, abstract out the relevant bits in 
build_address_symbolic and find_frame_funname to a separate
function, and then use that function in both places?

IIUC, in both cases, we're looking to see if there's a minimal
symbol that would be better than the debug symbol we start with.

> @@ -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.  */

I don't find this "incorrect" here very useful -- why is it incorrect
is my immediate question when I read this.

Maybe that old comment:

         So look in the minimal symbol tables as well, and if it comes
         up with a larger address for the function use that instead.
         I don't think this can ever cause any problems; there
         shouldn't be any minimal symbols in the middle of a function;

should be updated.

Thanks,
Pedro Alves

Re: [PATCH 2/4] dwarf2-frame.c: Fix FDE processing bug involving non-contiguous ranges

[Pedro Alves]

On 6/8/19 8:54 PM, Kevin Buettner wrote:
> In the course of revising the test case for
> gdb.dwarf2/dw2-ranges-func.exp, I added a new .c file which would
> cause the "cold" range to be at a higher address than the rest of the
> function.  In these tests, the range in question isn't really cold in
> the sense that a compiler has determined that it'll be executed less
> frequently.  Instead, it's simply the range that does not include the
> entry pc.  These tests are intended to mimic the output of such a
> compiler, so I'll continue to refer to this range as "cold" in the
> following discussion.
> 
> The original test case had only tested a cold range placed
> at lower addresses than the rest of the function.  During testing of the
> new code where the cold range was placed at higher addresses, I found
> that I could produce the following backtrace:
> 
>     (gdb) bt
>     #0  0x0000000000401138 in baz ()
> 	at dw2-ranges-func-hi-cold.c:72
>     #1  0x0000000000401131 in foo_cold ()
> 	at dw2-ranges-func-hi-cold.c:64
>     #2  0x000000000040111e in foo ()
> 	at dw2-ranges-func-hi-cold.c:50
>     #3  0x0000000000401144 in main ()
> 	at dw2-ranges-func-hi-cold.c:78
> 
> This is correct, except that we'd like to see foo() listed instead
> of foo_cold().  (I handle that problem in another patch.)
> 
> Now look at what happens for a similar backtrace where the cold range
> is at a lower address than the foo's entry pc:
> 
>     (gdb) bt
>     #0  0x000000000040110a in baz ()
> 	at dw2-ranges-func-lo-cold.c:48
>     #1  0x0000000000401116 in foo ()
> 	at dw2-ranges-func-lo-cold.c:54
>     #2  0x00007fffffffd4c0 in ?? ()
>     #3  0x0000000000401138 in foo ()
> 	at dw2-ranges-func-lo-cold.c:70
> 
> Note that the backtrace doesn't go all the way back to main().  Moreover,
> frame #2 is messed up.
> 
> I had seen this behavior when I had worked on the non-contiguous
> address problem last year.  At the time I convinced myself that the
> mangled backtrace was "okay" since we're doing strange things with
> the DWARF assembler.  We're taking a function called foo_cold (though
> it was originally called foo_low - my recent changes to the test case
> changed the name) and via the magic of the DWARF assembler, we're
> combining it into a separate (non-contiguous) range for foo.  Thus,
> it was a surprise to me when I got a good and complete backtrace when
> the cold symbol is placed at an address that's greater than entry pc.
> 
> The function dwarf2_frame_cache (in dwarf2-frame.c) is making this
> call:
> 
>     if (get_frame_func_if_available (this_frame, &entry_pc)) ...
> 
> If that call succeeds (returns a true value), the FDE is then
> processed up to the entry pc.  It doesn't make sense to do this,
> however, when the FDE in question does not contain the entry pc.  This
> can happen when the function in question is comprised of more than one
> (non-contiguous) address range.
> 
> My fix is to add some comparisons to the test above to ensure that
> ENTRY_PC is within the address range covered by the FDE.

Looks reasonable.

Thanks,
Pedro Alves

Re: [PATCH 3/4] Allow display of negative offsets in print_address_symbolic()

[Pedro Alves]

On 6/8/19 8:54 PM, Kevin Buettner wrote:
> When examining addresses associated with blocks with non-contiguous
> address ranges, it's not uncommon to see large positive offsets which,
> for some address width, actually represent a smaller negative offset.
> Here's an example taken from the test case:
> 
>     (gdb) x/i foo_cold
>        0x40110d <foo+4294967277>:	push   %rbp
> 
> This commit causes cases like the above to be displayed like this (below)
> instead:
> 
>     (gdb) x/i foo_cold
>        0x40110d <foo-19>:	push   %rbp
> 
> gdb/ChangeLog:
> 
> 	* printcmd.c (print_address_symbolic): Print negative offsets.
> 	(build_address_symbolic): Force signed arithmetic when computing
> 	offset.

Seems reasonable to me, if we assume that the symbol name to put
within <> is "foo".

This change makes makes me doubt that, though.  We're looking at
the lower level, disassembly code.  I think I'd want to see

  0x40110d <foo_cold+0>:

there?

E.g., I might want to follow up with
disassemble foo_cold.

But the present state of things, I wouldn't be able to see the
foo_cold symbol, where it starts?

Maybe a larger disassemble output including several cold sections
in view would help determine the best output.

Thanks,
Pedro Alves

Re: [PATCH 0/4] Non-contiguous address range bug fixes / improvements

[Tom Tromey]

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

Kevin> This four part series fixes some bugs associated with GDB's non-contiguous
Kevin> address range support.

This is not really related to your patches, but since you have been
working in this area, I thought I'd ask.

I ran into a case where gdb will mis-report a breakpoint location in a
certain situation (that is, "info b" will show something odd for the
source location).  After debugging for a while, my theory is that the
problem occurs because the executable has non-contiguous address ranges.

In particular, find_pc_sect_line is written to first find the symtab
with the smallest overall range that encloses the PC, and then to find a
matching symbol in the symtab.  But, with non-contiguous ranges, this
can yield a sub-optimal result -- because the overall range of a symtab
not longer really says anything about whether it holds the best symbol.

Have you seen anything like this?  (I guess not since I'd imagine you'd
have written a patch ;-)

I was thinking perhaps the best fix would be to search the blockvectors
for a definitively enclosing block.  I wonder what you think.

thanks,
Tom

Re: [PATCH 1/4] Prefer symtab symbol over minsym for function names in non-contiguous blocks

[Tom Tromey]

>> 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 information.

Pedro> Yuck!

>> 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.

Pedro> Yeah, I'd be inclined to try removing it too.  The comment
Pedro> smells of a.out or stabs limitations.  But I'm not 100% sure,
Pedro> and I'm sympathetic with forward incremental progress.

That code dates to the creation of the sourceware repository.

I think it could be deleted.  And, if it's still a bug somehow, it seems
better to fix it some other way, and not let stabs and/or a.out
weirdness into the generic code.

Tom

Re: [PATCH 0/4] Non-contiguous address range bug fixes / improvements

[Kevin Buettner]

On Wed, 26 Jun 2019 11:24:46 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> Kevin> This four part series fixes some bugs associated with GDB's non-contiguous
> Kevin> address range support.  
> 
> This is not really related to your patches, but since you have been
> working in this area, I thought I'd ask.
> 
> I ran into a case where gdb will mis-report a breakpoint location in a
> certain situation (that is, "info b" will show something odd for the
> source location).  After debugging for a while, my theory is that the
> problem occurs because the executable has non-contiguous address ranges.
> 
> In particular, find_pc_sect_line is written to first find the symtab
> with the smallest overall range that encloses the PC, and then to find a
> matching symbol in the symtab.  But, with non-contiguous ranges, this
> can yield a sub-optimal result -- because the overall range of a symtab
> not longer really says anything about whether it holds the best symbol.
> 
> Have you seen anything like this?  (I guess not since I'd imagine you'd
> have written a patch ;-)
> 
> I was thinking perhaps the best fix would be to search the blockvectors
> for a definitively enclosing block.  I wonder what you think.

Hi Tom,

I hadn't encountered this (yet), but I have no doubt that there are still
some things to be fixed.

I'll play around with my test case to see if I can reproduce the behavior
that you've described.

Kevin

Re: [PATCH 3/4] Allow display of negative offsets in print_address_symbolic()

[Kevin Buettner]

On Fri, 21 Jun 2019 15:45:13 +0100
Pedro Alves <palves@redhat.com> wrote:

> On 6/8/19 8:54 PM, Kevin Buettner wrote:
> > When examining addresses associated with blocks with non-contiguous
> > address ranges, it's not uncommon to see large positive offsets which,
> > for some address width, actually represent a smaller negative offset.
> > Here's an example taken from the test case:
> > 
> >     (gdb) x/i foo_cold
> >        0x40110d <foo+4294967277>:	push   %rbp
> > 
> > This commit causes cases like the above to be displayed like this (below)
> > instead:
> > 
> >     (gdb) x/i foo_cold
> >        0x40110d <foo-19>:	push   %rbp
> > 
> > gdb/ChangeLog:
> > 
> > 	* printcmd.c (print_address_symbolic): Print negative offsets.
> > 	(build_address_symbolic): Force signed arithmetic when computing
> > 	offset.  
> 
> Seems reasonable to me, if we assume that the symbol name to put
> within <> is "foo".
> 
> This change makes makes me doubt that, though.  We're looking at
> the lower level, disassembly code.  I think I'd want to see
> 
>   0x40110d <foo_cold+0>:
> 
> there?
> 
> E.g., I might want to follow up with
> disassemble foo_cold.
> 
> But the present state of things, I wouldn't be able to see the
> foo_cold symbol, where it starts?
> 
> Maybe a larger disassemble output including several cold sections
> in view would help determine the best output.

I've been conducting some experiments with this patch...

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 886e4464df..e6599493ff 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -629,9 +629,15 @@ build_address_symbolic (struct gdbarch *gdbarch,
 
   if (msymbol.minsym != NULL)
     {
+#if 1
       if (symbol == NULL
           || (BLOCK_CONTIGUOUS_P (SYMBOL_BLOCK_VALUE (symbol))
 	      && BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location))
+#else
+      if (symbol == NULL
+          || !BLOCK_CONTIGUOUS_P (SYMBOL_BLOCK_VALUE (symbol))
+	  || BMSYMBOL_VALUE_ADDRESS (msymbol) > name_location)
+#endif
 	{
 	  /* If this is a function (i.e. a code address), strip out any
 	     non-address bits.  For instance, display a pointer to the

...applied on top of the other patches in this set.

As shown, GDB will prefer the symtab symbol over the minsym for
display of symbols in disassembled code.  When I change the #if 1
to #if 0, GDB will always prefer the minsym for functions with
non-contiguous ranges.  (Whatever it is that we do, I want the
"lo-cold" and "hi-cold" cases to behave the same.)

Then, using the "lo-cold" executable for the dw2-ranges-func test,
I've been doing the following:

./gdb testsuite/outputs/gdb.dwarf2/dw2-ranges-func/dw2-ranges-func-lo-cold
b 70
b baz
run
set var e=1
c
bt
up
x/5i $pc
disassemble foo
x/i foo_cold
disassemble foo_cold

I don't see anything interesting until we get to the "bt" command,  For "bt", we
see (as expected) the same output for both cases:

(gdb) bt
#0  0x000000000040110a in baz ()
    at worktree-ranges/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c:48
#1  0x0000000000401116 in foo ()
    at worktree-ranges/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c:54
#2  0x0000000000401138 in foo ()
    at worktree-ranges/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c:70
#3  0x0000000000401144 in main ()
    at worktree-ranges/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c:78

(I've shortened the paths for readability.)

The thing to note here is that the call of foo at frame #1 is actually a
call to foo_cold.  Showing foo_cold in the backtrace is the behavior that
Eli found objectionable.

Likewise, "up" shows the same behavior for both cases:

(gdb) up
#1  0x0000000000401116 in foo ()
    at worktree-ranges/gdb/testsuite/gdb.dwarf2/dw2-ranges-func-lo-cold.c:54
54	  baz ();					/* foo_cold baz call */

"x/5i" shows some differences in output.  I'll show the "prefer symtab
sym" version first, followed by the "prefer minsym" version second:

(gdb) x/5i $pc
=> 0x401116 <foo-10>:	nop
   0x401117 <foo-9>:	pop    %rbp
   0x401118 <foo-8>:	retq   
   0x401119 <bar>:	push   %rbp
   0x40111a <bar+1>:	mov    %rsp,%rbp

   --- versus ---

(gdb) x/5i $pc
=> 0x401116 <foo_cold+9>:	nop
   0x401117 <foo_cold+10>:	pop    %rbp
   0x401118 <foo_cold+11>:	retq   
   0x401119 <bar>:	push   %rbp
   0x40111a <bar+1>:	mov    %rsp,%rbp

The thing to observe in the above output is that offsets from foo are
used in the first case, where as offsets from foo_cold are shown for
the "prefer minsym" version.

Both versions show similar output for the "disassemble foo" command.
Here is the output for the "prefer minsym" version:

(gdb) disassemble foo
Dump of assembler code for function foo:
Address range 0x401120 to 0x40113b:
   0x0000000000401120 <+0>:	push   %rbp
   0x0000000000401121 <+1>:	mov    %rsp,%rbp
   0x0000000000401124 <+4>:	callq  0x401119 <bar>
   0x0000000000401129 <+9>:	mov    0x2ef1(%rip),%eax        # 0x404020 <e>
   0x000000000040112f <+15>:	test   %eax,%eax
   0x0000000000401131 <+17>:	je     0x401138 <foo+24>
   0x0000000000401133 <+19>:	callq  0x40110d <foo_cold>
   0x0000000000401138 <+24>:	nop
   0x0000000000401139 <+25>:	pop    %rbp
   0x000000000040113a <+26>:	retq   
Address range 0x40110d to 0x401119:
   0x000000000040110d <+0>:	push   %rbp
   0x000000000040110e <+1>:	mov    %rsp,%rbp
   0x0000000000401111 <+4>:	callq  0x401106 <baz>
=> 0x0000000000401116 <+9>:	nop
   0x0000000000401117 <+10>:	pop    %rbp
   0x0000000000401118 <+11>:	retq   
End of assembler dump.

The only line where there's a difference is:

   0x0000000000401133 <+19>:	callq  0x40110d <foo-19>

   --- versus ---

   0x0000000000401133 <+19>:	callq  0x40110d <foo_cold>

I think I prefer the negative offset in this case.

"x/i foo_cold" produces different outputs...

(gdb) x/i foo_cold
   0x40110d <foo-19>:	push   %rbp

   --- versus ---

(gdb) x/i foo_cold
   0x40110d <foo_cold>:	push   %rbp

The version that prefers the symtab sym shows foo versus foo_cold for
the version that prefers the minsym sym.

"disassemble foo_cold" shows the same output as "disassemble foo"
above.  I won't show it here since it's the same as what's shown
earlier.  I was sort of surprised that it showed the entire function
(both) ranges, but after thinking about it, this made sense since you
see the entire function when you disassemble some address that's in
the middle of the function.

My thoughts...

When I say "x/i foo_cold", I do think I'd prefer to see <foo_cold> instead
of <foo-19>.

However, when I do "x/5i $pc" after doing "up" from the baz frame, I think
I somewhat prefer seeing foo with negative offsets.

What would you think about this behavior?

(gdb) x/5i foo_cold
   0x40110d <foo_cold>:	push   %rbp
   0x40110e <foo-18>:	mov    %rsp,%rbp
   0x401111 <foo-15>:	callq  0x401106 <baz>
=> 0x401116 <foo-10>:	nop
   0x401117 <foo-9>:	pop    %rbp

I.e. prefer the minsym for offset 0, but use the function symbol for
the non-zero offsets.

Another possibility:

(gdb) x/5i foo_cold
   0x40110d <foo-19> <foo_cold>: push   %rbp
   0x40110e <foo-18>:	mov    %rsp,%rbp
   0x401111 <foo-15>:	callq  0x401106 <baz>
=> 0x401116 <foo-10>:	nop
   0x401117 <foo-9>:	pop    %rbp

I.e, show both the function symbol (plus/minus offset) AND the minsym,
but only show the minsym for the zero offset.

I haven't tried implementing either of these approaches yet, but
I can take a look at it if we have some concensus over what the output
should look like.

Kevin

Re: [PATCH 1/4] Prefer symtab symbol over minsym for function names in non-contiguous blocks

[Kevin Buettner]

On Wed, 26 Jun 2019 11:30:51 -0600
Tom Tromey <tom@tromey.com> wrote:

> >> 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 information.  
> 
> Pedro> Yuck!  
> 
> >> 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.  
> 
> Pedro> Yeah, I'd be inclined to try removing it too.  The comment
> Pedro> smells of a.out or stabs limitations.  But I'm not 100% sure,
> Pedro> and I'm sympathetic with forward incremental progress.  
> 
> That code dates to the creation of the sourceware repository.
> 
> I think it could be deleted.  And, if it's still a bug somehow, it seems
> better to fix it some other way, and not let stabs and/or a.out
> weirdness into the generic code.

Okay, for v2 of this patch series, I'll get rid of that code.

Thanks,

Kevin

Re: [PATCH 3/4] Allow display of negative offsets in print_address_symbolic()

[Kevin Buettner]

On Wed, 3 Jul 2019 16:09:21 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> When I say "x/i foo_cold", I do think I'd prefer to see <foo_cold> instead
> of <foo-19>.
> 
> However, when I do "x/5i $pc" after doing "up" from the baz frame, I think
> I somewhat prefer seeing foo with negative offsets.
> 
> What would you think about this behavior?
> 
> (gdb) x/5i foo_cold
>    0x40110d <foo_cold>:	push   %rbp
>    0x40110e <foo-18>:	mov    %rsp,%rbp
>    0x401111 <foo-15>:	callq  0x401106 <baz>
> => 0x401116 <foo-10>:	nop  
>    0x401117 <foo-9>:	pop    %rbp
> 
> I.e. prefer the minsym for offset 0, but use the function symbol for
> the non-zero offsets.

For the v2 version of this series, I've implemented the behavior shown
above.

I (hopefully) provide a good rationale for this behavior in the
commit comment.  (So, if you don't immediately like it, stay tuned
for the v2 patch series.)

> Another possibility:
> 
> (gdb) x/5i foo_cold
>    0x40110d <foo-19> <foo_cold>: push   %rbp
>    0x40110e <foo-18>:	mov    %rsp,%rbp
>    0x401111 <foo-15>:	callq  0x401106 <baz>
> => 0x401116 <foo-10>:	nop  
>    0x401117 <foo-9>:	pop    %rbp
> 
> I.e, show both the function symbol (plus/minus offset) AND the minsym,
> but only show the minsym for the zero offset.
> 
> I haven't tried implementing either of these approaches yet, but
> I can take a look at it if we have some concensus over what the output
> should look like.

I sort of like this one too, but it's harder to implement, causes some
of the lines to be longer, and will also make it less likely that all
of the instructions associated with a given function will line up.

Kevin