[RFA] Ignore data minimal symbols for breakpoint linespecs

[Joel Brobecker <brobecker at adacore dot com>]

Hello,

This patch fixes regressions that showed up gdb.ada/task_bp.exp.

    FAIL: gdb.ada/task_bp.exp: break pck.dummy_task - from psymtab
    FAIL: gdb.ada/task_bp.exp: break pck.dummy_task - from full symtab

Basically, trying to insert a breakpoint on a task using its name
as the linespec yields two locations:

    (gdb) break pck.dummy_task
    Breakpoint 1 at 0x40321f: pck.dummy_task. (2 locations)

One of those locations is invalid, and comes from minimal symtabs:

    (gdb) info break
    Num     Type           Disp Enb Address            What
    1       breakpoint     keep y   <MULTIPLE>
    1.1                         y     0x000000000040321f in pck.dummy_task
                                                       at pck.adb:4
    1.2                         y     0x0000000000639da0 <pck__dummy_task>

The second location is unexpected. The debugger correctly rejected
pck__dummy_task from the debug info (this is variable Pck.Dummy_Task),
but still fished it out of the minimal symbol table.  The reason why
it should be rejected in this case is because this is a data symbol,
not a text one. So inserting a breakpoint at this address is unexpected,
and also has the effect of modifying this variable's value, thus modifying
the inferior's behavior.

For easier testing, I reproduced the problem with C files (testcase).
The reproducer also demonstrates how the program behavior is affected
(the global variable value gets corrupted).

The fix consists in filtering out those data symbols, unless we're
trying to print the line info of a given linespec. Without the fix,
the testcase fails as follow:

    FAIL: gdb.base/dmsym.exp: break pck__foo__bar__minsym
    FAIL: gdb.base/dmsym.exp: print val

gdb/ChangeLog:

        * linespec.c (struct collect_minsyms) [list_mode]: New field.
        (add_minsym): Ignore data symbols if not in list mode.
        (search_minsyms_for_name): Set local.list_mode.

gdb/testsuite/ChangeLog:

        * gdb.base/dmsym.c, gdb.base/dmsym_main.c, gdb.base/dmsym.exp:
        New files.

Tested on x86_64-linux. OK to commit?
Thanks,
-- 
Joel

---
 gdb/linespec.c                      |   17 +++++++
 gdb/testsuite/gdb.base/dmsym.c      |   25 ++++++++++
 gdb/testsuite/gdb.base/dmsym.exp    |   87 +++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/dmsym_main.c |   36 ++++++++++++++
 4 files changed, 165 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/dmsym.c
 create mode 100644 gdb/testsuite/gdb.base/dmsym.exp
 create mode 100644 gdb/testsuite/gdb.base/dmsym_main.c

diff --git a/gdb/linespec.c b/gdb/linespec.c
index e49292e..9d753e5 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2749,6 +2749,9 @@ struct collect_minsyms
   /* The funfirstline setting from the initial call.  */
   int funfirstline;
 
+  /* The list_mode setting from the initial call.  */
+  int list_mode;
+
   /* The resulting symbols.  */
   VEC (minsym_and_objfile_d) *msyms;
 };
@@ -2799,6 +2802,19 @@ add_minsym (struct minimal_symbol *minsym, void *d)
   struct collect_minsyms *info = d;
   minsym_and_objfile_d mo;
 
+  /* Exclude data symbols when looking for breakpoint locations.   */
+  if (!info->list_mode)
+    switch (minsym->type)
+      {
+	case mst_slot_got_plt:
+	case mst_data:
+	case mst_bss:
+	case mst_abs:
+	case mst_file_data:
+	case mst_file_bss:
+	return;
+      }
+
   mo.minsym = minsym;
   mo.objfile = info->objfile;
   VEC_safe_push (minsym_and_objfile_d, info->msyms, &mo);
@@ -2829,6 +2845,7 @@ search_minsyms_for_name (struct collect_info *info, const char *name,
 
     memset (&local, 0, sizeof (local));
     local.funfirstline = info->state->funfirstline;
+    local.list_mode = info->state->list_mode;
 
     cleanup = make_cleanup (VEC_cleanup (minsym_and_objfile_d),
 			    &local.msyms);
diff --git a/gdb/testsuite/gdb.base/dmsym.c b/gdb/testsuite/gdb.base/dmsym.c
new file mode 100644
index 0000000..889e800
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dmsym.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+int pck__foo__bar__minsym = 123;
+
+int
+get_pck__foo__bar__minsym (void)
+{
+  pck__foo__bar__minsym++;
+  return pck__foo__bar__minsym;
+}
diff --git a/gdb/testsuite/gdb.base/dmsym.exp b/gdb/testsuite/gdb.base/dmsym.exp
new file mode 100644
index 0000000..2c27f25
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dmsym.exp
@@ -0,0 +1,87 @@
+# Copyright (C) 2011 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/>.
+
+set testfile dmsym_main
+
+# Build dmsym_main using two C files:
+#   - dmsym.c, which needs to be built without debug info;
+#   - dmsym_main.c, which needs to be build with debug info.
+# This is why we use gdb_compile instead of relying on the usual
+# call to prepare_for_testing.
+
+if {[gdb_compile "${srcdir}/${subdir}/dmsym.c" \
+                 ${objdir}/${subdir}/dmsym.o \
+                 object {}] != ""} {
+  untested dmsym.exp
+  return -1
+}
+
+if {[gdb_compile \
+      [list ${srcdir}/${subdir}/dmsym_main.c ${objdir}/${subdir}/dmsym.o] \
+      ${objdir}/${subdir}/${testfile} \
+      executable {debug}] != ""} {
+    untested dmsym.exp
+    return -1
+}
+
+clean_restart ${testfile}
+
+# Some convenient regular expressions...
+set num "\[0-9\]+"
+set addr "0x\[0-9a-zA-Z\]+"
+
+# Although the test program is written in C, the original problem
+# occurs only when the language is Ada. The use of a C program is
+# only a convenience to be able to exercise the original problem
+# without requiring an Ada compiler. In the meantime, temporarily
+# force the language to Ada.
+
+gdb_test_no_output "set lang ada"
+
+# Verify that setting a breakpoint on `pck__foo__bar__minsym' only
+# results in one location found (function pck__foo__bar__minsym__2).
+# A mistake would be to also insert a breakpoint where
+# pck__foo__bar__minsym is defined.  Despite the fact that there is
+# no debugging info available, this is a data symbol and thus should
+# not be used for breakpoint purposes.
+
+gdb_test "break pck__foo__bar__minsym" \
+         "Breakpoint $num at $addr.: file .*dmsym_main\\.c, line $num\\."
+
+# However, verify that the `info line' command, on the other hand,
+# finds both locations.
+
+gdb_test "info line pck__foo__bar__minsym" \
+         "Line $num of \".*dmsym_main\\.c\" .*\r\nNo line number information available for address $addr <pck__foo__bar__minsym>"
+
+gdb_test_no_output "set lang auto"
+
+# Now, run the program until we get past the call to
+# pck__foo__bar__minsym__2. Except when using hardware breakpoints,
+# inferior behavior is going to be affected if a breakpoint was
+# incorrectly inserted at pck__foo__bar__minsym.
+
+gdb_breakpoint dmsym_main.c:[gdb_get_line_number "BREAK" dmsym_main.c]
+
+gdb_run_cmd
+gdb_test "" \
+         "Breakpoint $num, pck__foo__bar__minsym__2 \\(\\) at.*" \
+         "Run until breakpoint at BREAK"
+
+gdb_test "continue" \
+         "Breakpoint $num, main \\(\\) at.*"
+
+gdb_test "print val" \
+         " = 124"
diff --git a/gdb/testsuite/gdb.base/dmsym_main.c b/gdb/testsuite/gdb.base/dmsym_main.c
new file mode 100644
index 0000000..02cfcb7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/dmsym_main.c
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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/>.  */
+
+extern int get_pck__foo__bar__minsym (void);
+
+int
+pck__foo__bar__minsym__2 (void)
+{
+  return get_pck__foo__bar__minsym ();
+}
+
+int
+main (void)
+{
+  int val = pck__foo__bar__minsym__2 ();
+
+  if (val != 124) /* BREAK */
+    return 1;
+  return 0;
+}
+
+
-- 
1.7.1