[binutils-gdb] minsyms.c: Scan backwards over all zero sized symbols.

[Kevin Buettner <kevinb at sourceware dot org>]

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

commit 5506f9f67ec105b0059a0b3a66fbde82cb5a0a15
Author: Kevin Buettner <kevinb@redhat.com>
Date:   Sat Nov 14 13:15:45 2015 -0700

    minsyms.c: Scan backwards over all zero sized symbols.
    
    The comment for the code in question says:
    
    		  /* If the minimal symbol has a zero size, save it
    		     but keep scanning backwards looking for one with
    		     a non-zero size.  A zero size may mean that the
    		     symbol isn't an object or function (e.g. a
    		     label), or it may just mean that the size was not
    		     specified.  */
    
    As written, the code in question will only scan past the first symbol
    of zero size.  My change fixes the implementation to match the
    comment.
    
    Having this correct is important when the compiler generates several
    local labels that are left in place by the linker.  (I've been told
    that the linker should eliminate these symbols, but I know of one
    architecture for which this is not happening.)
    
    I've created a test case called asmlabel.c.  It's pretty simple:
    
    main (int argc, char **argv)
    {
      asm ("L0:");
      v = 0;
      asm ("L1:");
      v = 1;		/* set L1 breakpoint here */
      asm ("L2:");
      v = 2;		/* set L2 breakpoint here */
      return 0;
    }
    
    If breakpoints are placed on the lines indicated by the comments,
    this is the behavior of GDB built without my patch:
    
        (gdb) continue
        Continuing.
    
        Breakpoint 2, L1 () at asmlabel.c:26
        26	  v = 1;		/* set L1 breakpoint here */
    
    Note that L1 appears as the function instead of main.  This is not
    what we want to happen.  With my patch in place, we see the desired
    behavior instead:
    
        (gdb) continue
        Continuing.
    
        Breakpoint 2, main (argc=1, argv=0x7fffffffdb88) at asmlabel.c:26
        26	  v = 1;		/* set L1 breakpoint here */
    
    gdb/ChangeLog:
    
    	* minsyms.c (lookup_minimal_symbol_by_pc_section_1): Scan backwards
    	over all zero-sized symbols.
    
    gdb/testsuite/ChangeLog:
    
    	* gdb.base/asmlabel.exp: New test.
    	* gdb.base/asmlabel.c: New test case.

Diff:
---
 gdb/ChangeLog                       |  5 +++
 gdb/minsyms.c                       |  6 ++--
 gdb/testsuite/ChangeLog             |  5 +++
 gdb/testsuite/gdb.base/asmlabel.c   | 30 +++++++++++++++++
 gdb/testsuite/gdb.base/asmlabel.exp | 64 +++++++++++++++++++++++++++++++++++++
 5 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d648e56..8cc9aa0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-11-23  Kevin Buettner  <kevinb@redhat.com>
+
+	* minsyms.c (lookup_minimal_symbol_by_pc_section_1): Scan backwards
+	over all zero-sized symbols.
+
 2015-11-23  Joel Brobecker  <brobecker@adacore.com>
 
 	* stack.c (print_frame_local_vars): Temporarily set the selected
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index d7097a9..5def1ee 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -697,10 +697,10 @@ lookup_minimal_symbol_by_pc_section_1 (CORE_ADDR pc_in,
 		     symbol isn't an object or function (e.g. a
 		     label), or it may just mean that the size was not
 		     specified.  */
-		  if (MSYMBOL_SIZE (&msymbol[hi]) == 0
-		      && best_zero_sized == -1)
+		  if (MSYMBOL_SIZE (&msymbol[hi]) == 0)
 		    {
-		      best_zero_sized = hi;
+		      if (best_zero_sized == -1)
+			best_zero_sized = hi;
 		      hi--;
 		      continue;
 		    }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 2c59ad8..e5c2b97 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2015-11-23  Kevin Buettner  <kevinb@redhat.com>
+
+	* gdb.base/asmlabel.exp: New test.
+	* gdb.base/asmlabel.c: New test case.
+
 2015-11-23  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.base/wrong_frame_bt_full-main.c: New file.
diff --git a/gdb/testsuite/gdb.base/asmlabel.c b/gdb/testsuite/gdb.base/asmlabel.c
new file mode 100644
index 0000000..b4ea6fc
--- /dev/null
+++ b/gdb/testsuite/gdb.base/asmlabel.c
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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/>.  */
+
+static volatile int v = -1;
+
+int
+main (int argc, char **argv)
+{
+  asm ("L0:");
+  v = 0;
+  asm ("L1:");
+  v = 1;		/* set L1 breakpoint here */
+  asm ("L2:");
+  v = 2;		/* set L2 breakpoint here */
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/asmlabel.exp b/gdb/testsuite/gdb.base/asmlabel.exp
new file mode 100644
index 0000000..5020b72
--- /dev/null
+++ b/gdb/testsuite/gdb.base/asmlabel.exp
@@ -0,0 +1,64 @@
+# Copyright (C) 2015 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/>.
+
+# Test to see that GDB reports the correct function name when stopping
+# in a function with several assembly-defined labels; see the test
+# case asmlabel.c.
+#
+# When setting and then continuing to a breakpoint at either location
+# L1 or L2 - again, see the code - we want to make sure that GDB
+# correctly reports that it's in main, like this:
+#
+# Breakpoint 2, main (argc=1, argv=0x7fffffffe408) at asmlabel.c:26
+# 26        v = 1;                /* set L1 breakpoint here */
+#
+# This test case was written to test for a bug in which GDB printed
+# the following instead:
+#
+# Breakpoint 2, L1 () at asmlabel.c:26
+# 26        v = 1;                /* set L1 breakpoint here */
+#
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return 0
+}
+
+set L1_bploc [gdb_get_line_number "set L1 breakpoint here" $srcfile]
+set L2_bploc [gdb_get_line_number "set L2 breakpoint here" $srcfile]
+
+gdb_test "break $srcfile:$L1_bploc" \
+    "Breakpoint.*at.* file .*$srcfile, line $L1_bploc\\." \
+    "breakpoint at L1"
+
+gdb_test "break $L2_bploc" \
+    "Breakpoint.*at.* file .*$srcfile, line $L2_bploc\\." \
+    "breakpoint at L2"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint \[0-9\]+, main .* at .*$srcfile:$L1_bploc.*" \
+    "continue to L1"
+
+gdb_test "continue" \
+    "Continuing.*Breakpoint \[0-9\]+, main .* at .*$srcfile:$L2_bploc.*" \
+    "continue to L2"
+
+gdb_test "print v" "= 1" "check value of v at L2"