* [PATCH] Don't drop static function bp locations w/o debug info
@ 2020-06-12 19:21 Pedro Alves
2020-06-12 19:53 ` Simon Marchi
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2020-06-12 19:21 UTC (permalink / raw)
To: gdb-patches
Currently, with a program built from these sources:
$ cat extern.c
void foo () {}
$ cat static.c
static void foo () {}
$ cat main.c
int main () { return 0; }
... if you set a breakpoint on "foo", like:
(gdb) break foo
.. when there's debug info, GDB creates a breakpoint with two
locations, one for each of the external and static functions.
But, when there's no debug info, GDB creates a breakpoint with a
single location, for the external foo. Vis:
$ gcc extern.c static.c main.c -o classify.nodebug
$ gcc extern.c static.c main.c -o classify.debug -g
$ gdb classify.nodebug
Reading symbols from classify.nodebug...
(No debugging symbols found in classify.nodebug)
(gdb) b foo
Breakpoint 1 at 0x40048b
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x000000000040048b <foo+4>
(gdb)
$ gdb classify.debug
Reading symbols from classify.debug...
(gdb) b foo
Breakpoint 1 at 0x40048b: foo. (2 locations)
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x000000000040048b in foo at extern.c:1
1.2 y 0x0000000000400492 in foo at static.c:1
GDB drops the static function is search_minsyms_for_name, where at the
very end of that function we pick only the locations with highest
classification, according to classify_type.
The classify_type logic was introduced here:
https://sourceware.org/pipermail/gdb-patches/2011-December/087864.html
which said:
"Previously, linespec was trying to filter out minsyms as they were
seen. However, this isn't faithful to gdb's historical approach,
which is to priority-order minsyms; see lookup_minimal_symbol."
lookup_minimal_symbol's intro says, in the .c file:
/* Look through all the current minimal symbol tables and find the
first minimal symbol that matches NAME. If OBJF is non-NULL, limit
the search to that objfile. If SFILE is non-NULL, the only file-scope
symbols considered will be from that source file (global symbols are
still preferred). Returns a pointer to the minimal symbol that
matches, or NULL if no match is found.
Note: One instance where there may be duplicate minimal symbols with
the same name is when the symbol tables for a shared library and the
symbol tables for an executable contain global symbols with the same
names (the dynamic linker deals with the duplication).
It's also possible to have minimal symbols with different mangled
names, but identical demangled names. For example, the GNU C++ v3
ABI requires the generation of two (or perhaps three) copies of
constructor functions --- "in-charge", "not-in-charge", and
"allocate" copies; destructors may be duplicated as well.
Obviously, there must be distinct mangled names for each of these,
but the demangled names are all the same: S::S or S::~S. */
struct bound_minimal_symbol
lookup_minimal_symbol (const char *name, const char *sfile,
struct objfile *objf)
{
If you look inside this function, you'll see:
/* External symbols are best. */
...
/* File-local symbols are next best. */
...
/* Symbols for shared library trampolines are next best. */
...
While this logic is good when you're looking for the single "best"
symbol by name, I question it for linespecs, since we want to set
breakpoints in all the multiple locations that match. I see no point
in hidding static functions.
Now, for breakpoints, it does make sense to filter out PLT/trampoline
symbols if we find the actual global matching function symbol.
Otherwise, if we did no filtering (i.e., just removed the
classify_type logic), you would end up with e.g.:
(gdb) b printf
Breakpoint 1 at 0x413a60 (2 locations)
(top-gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000413a60 <printf@plt>
1.2 y 0x00007ffff4653640 in __printf at printf.c:28
instead of this (which is what we get currently) before the shared
library is loaded (a location set in the PLT):
(gdb) b printf
Breakpoint 1 at 0x413a60
Num Type Disp Enb Address What
1 breakpoint keep y 0x0000000000413a60 <printf@plt>
and this after the library is loaded (only one location, no breakpoint
in the PLT):
(gdb) b printf
Breakpoint 1 at 0x7ffff4653640: file printf.c, line 28.
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y 0x00007ffff4653640 in __printf at printf.c:28
This patch fixes the missing breakpoint locations issue by replacing
the classify_type logic in linespec.c with a different logic.
Instead, discard a trampoline symbol if we also found a
global/external symbol with the same name. The patch adds a couple of
testcases testing locations in external vs static functions vs
trampolines/PLTs.
We now get:
For the msym-bp.exp testcase (extern vs static), without debug info:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x000000000040048b <foo+4> ### missing before patch
1.2 y 0x000000000040049d <foo+4>
For the msym-bp.exp testcase (extern vs static), with debug info:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x000000000040048b in foo at src/gdb/testsuite/gdb.base/msym-bp.c:21
1.2 y 0x000000000040049d in foo at src/gdb/testsuite/gdb.base/msym-bp-2.c:21
For the msym-bp-shl.exp testcase (static vs plt), without debug info, before running to main:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x00000000004004e0 <foo@plt> ### missing before patch
1.2 y 0x00000000004005db <foo+4>
For the msym-bp-shl.exp testcase (static vs plt), without debug info, after running to main:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x00000000004005db <foo+4> ### missing before patch
1.2 y 0x00007ffff7bd65de <foo+4>
For the msym-bp-shl.exp testcase (static vs plt), with debug info, before running to main:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x00000000004004e0 <foo@plt> ### missing before patch
1.2 y 0x00000000004005db in foo at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c:21
For the msym-bp-shl.exp testcase (static vs plt), with debug info, after running to main:
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x00000000004005db in foo at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c:21
1.2 y 0x00007ffff7bd65de in foo at /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/msym-bp-shl-lib.c:21
gdb/ChangeLog:
* linespec.c (classify_mtype, compare_msyms): Delete.
(search_minsyms_for_name): Remove classification logic. Instead
filter out trampoline symbols if we also found an external
function of the same name.
gdb/testsuite/ChangeLog:
* gdb.base/msym-bp-2.c: New.
* gdb.base/msym-bp-shl-lib.c: New file.
* gdb.base/msym-bp-shl-main-2.c: New file.
* gdb.base/msym-bp-shl-main.c: New file.
* gdb.base/msym-bp-shl.exp: New file.
* gdb.base/msym-bp.c: New file.
* gdb.base/msym-bp.exp: New file.
---
gdb/linespec.c | 90 ++++++++++++-----------
gdb/testsuite/gdb.base/msym-bp-2.c | 21 ++++++
gdb/testsuite/gdb.base/msym-bp-shl-lib.c | 21 ++++++
gdb/testsuite/gdb.base/msym-bp-shl-main-2.c | 21 ++++++
gdb/testsuite/gdb.base/msym-bp-shl-main.c | 25 +++++++
gdb/testsuite/gdb.base/msym-bp-shl.exp | 107 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/msym-bp.c | 27 +++++++
gdb/testsuite/gdb.base/msym-bp.exp | 83 +++++++++++++++++++++
8 files changed, 349 insertions(+), 46 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/msym-bp-2.c
create mode 100644 gdb/testsuite/gdb.base/msym-bp-shl-lib.c
create mode 100644 gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
create mode 100644 gdb/testsuite/gdb.base/msym-bp-shl-main.c
create mode 100644 gdb/testsuite/gdb.base/msym-bp-shl.exp
create mode 100644 gdb/testsuite/gdb.base/msym-bp.c
create mode 100644 gdb/testsuite/gdb.base/msym-bp.exp
diff --git a/gdb/linespec.c b/gdb/linespec.c
index ddca05b3dbf..36461abb2c0 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -4248,41 +4248,6 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
add_sal_to_sals (self, result, &sal, msymbol->natural_name (), 0);
}
-/* A helper function to classify a minimal_symbol_type according to
- priority. */
-
-static int
-classify_mtype (enum minimal_symbol_type t)
-{
- switch (t)
- {
- case mst_file_text:
- case mst_file_data:
- case mst_file_bss:
- /* Intermediate priority. */
- return 1;
-
- case mst_solib_trampoline:
- /* Lowest priority. */
- return 2;
-
- default:
- /* Highest priority. */
- return 0;
- }
-}
-
-/* Callback for std::sort that sorts symbols by priority. */
-
-static bool
-compare_msyms (const bound_minimal_symbol &a, const bound_minimal_symbol &b)
-{
- enum minimal_symbol_type ta = MSYMBOL_TYPE (a.minsym);
- enum minimal_symbol_type tb = MSYMBOL_TYPE (b.minsym);
-
- return classify_mtype (ta) < classify_mtype (tb);
-}
-
/* Helper for search_minsyms_for_name that adds the symbol to the
result. */
@@ -4371,21 +4336,54 @@ search_minsyms_for_name (struct collect_info *info,
if (!minsyms.empty ())
{
- int classification;
+ /* Return true if TYPE is a static symbol. */
+ auto msymbol_type_is_static = [] (enum minimal_symbol_type type)
+ {
+ switch (type)
+ {
+ case mst_file_text:
+ case mst_file_data:
+ case mst_file_bss:
+ return true;
+ default:
+ return false;
+ }
+ };
+
+ /* Add minsyms to the result set, but filter out trampoline
+ symbols if we also found extern symbols with the same name.
+ I.e., don't set a breakpoint on both '<foo@plt>' and 'foo',
+ assuming that 'foo' is the symbol that the plt resolves
+ to. */
+ for (const bound_minimal_symbol &item : minsyms)
+ {
+ bool skip = false;
+ if (MSYMBOL_TYPE (item.minsym) == mst_solib_trampoline)
+ {
+ for (const bound_minimal_symbol &item2 : minsyms)
+ {
+ if (&item2 == &item)
+ continue;
- std::sort (minsyms.begin (), minsyms.end (), compare_msyms);
+ /* Trampoline symbols can only jump to exported
+ symbols. */
+ if (msymbol_type_is_static (MSYMBOL_TYPE (item2.minsym)))
+ continue;
- /* Now the minsyms are in classification order. So, we walk
- over them and process just the minsyms with the same
- classification as the very first minsym in the list. */
- classification = classify_mtype (MSYMBOL_TYPE (minsyms[0].minsym));
+ if (strcmp (item.minsym->linkage_name (),
+ item2.minsym->linkage_name ()) != 0)
+ continue;
- for (const bound_minimal_symbol &item : minsyms)
- {
- if (classify_mtype (MSYMBOL_TYPE (item.minsym)) != classification)
- break;
+ /* Found a global minsym with the same name as the
+ trampoline. Don't create a location for this
+ trampoline. */
+ skip = true;
+ break;
+ }
+ }
- info->result.minimal_symbols->push_back (item);
+ if (!skip)
+ info->result.minimal_symbols->push_back (item);
}
}
}
diff --git a/gdb/testsuite/gdb.base/msym-bp-2.c b/gdb/testsuite/gdb.base/msym-bp-2.c
new file mode 100644
index 00000000000..b5a399ee492
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp-2.c
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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/>. */
+
+void
+foo (void)
+{
+}
diff --git a/gdb/testsuite/gdb.base/msym-bp-shl-lib.c b/gdb/testsuite/gdb.base/msym-bp-shl-lib.c
new file mode 100644
index 00000000000..b5a399ee492
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp-shl-lib.c
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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/>. */
+
+void
+foo (void)
+{
+}
diff --git a/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c b/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
new file mode 100644
index 00000000000..520701e725c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
@@ -0,0 +1,21 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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 void
+foo (void)
+{
+}
diff --git a/gdb/testsuite/gdb.base/msym-bp-shl-main.c b/gdb/testsuite/gdb.base/msym-bp-shl-main.c
new file mode 100644
index 00000000000..d6f32229c22
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp-shl-main.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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 void foo (void);
+
+int
+main ()
+{
+ foo ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
new file mode 100644
index 00000000000..fee1d7233fd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
@@ -0,0 +1,107 @@
+# Copyright 2020 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 that when setting a breakpoint at "foo", GDB creates a location
+# for an unresolved <foo@plt> PLT in the main binary, even when a
+# static function named "foo" exists in the shared library. Tests
+# both with and without debug info.
+
+if {[skip_shlib_tests]} {
+ return 0
+}
+
+standard_testfile msym-bp-shl-main.c msym-bp-shl-main-2.c msym-bp-shl-lib.c
+set srcfile ${srcdir}/${subdir}/${srcfile}
+set srcfile2 ${srcdir}/${subdir}/${srcfile2}
+set libsrc ${srcdir}/${subdir}/${srcfile3}
+
+# Run "info breakpoints", and check that we find the two locations,
+# LOC_A and LOC_B, in either order.
+proc test_info_break_2 {loc_a loc_b} {
+ set re1 ".*\.1.*${loc_a}\r\n.*\.2.*${loc_b}"
+ set re2 ".*\.1.*${loc_b}\r\n.*\.2.*${loc_a}"
+ gdb_test "info breakpoint" "$re1|$re2"
+}
+
+proc test {debug} {
+ global testfile binfile srcfile srcfile2 libsrc
+ global decimal
+
+ if {$debug} {
+ set options "debug"
+ } else {
+ set options ""
+ }
+
+ set bin ${binfile}-$debug
+ set lib [standard_output_file msym-bp-shl-lib-$debug.sl]
+
+ set exec_opts [list $options shlib=${lib}]
+
+ if { [gdb_compile_shlib $libsrc $lib $options] != ""
+ || [gdb_compile [list $srcfile $srcfile2] $bin \
+ executable $exec_opts] != ""} {
+ untested "failed to compile"
+ return
+ }
+
+ clean_restart $bin
+ gdb_load_shlib $lib
+
+ # Should find two locations: the static foo in the
+ # msym-bp-shl-main-2 file, and <foo@plt>, both in the main binary.
+ with_test_prefix "before run" {
+ gdb_test "break foo" "\\(2 locations\\)"
+
+ if {$debug} {
+ test_info_break_2 \
+ "<foo@plt.*>" \
+ "in foo at .*msym-bp-shl-main-2.c:$decimal"
+ } else {
+ test_info_break_2 \
+ "<foo@plt.*>" \
+ "<foo\\+$decimal>"
+ }
+ }
+
+ if ![runto_main] {
+ fail "can't run to main"
+ return
+ }
+
+ delete_breakpoints
+
+ # Should still find two locations, but the <foo@plt> PLT location
+ # should not be present anymore. I.e., we should find the static
+ # foo in the msym-bp-shl-main-2 file, and the extern foo in the
+ # shared library.
+ with_test_prefix "at main" {
+ gdb_test "break foo" "\\(2 locations\\)"
+
+ if {$debug} {
+ test_info_break_2 \
+ "in foo at .*msym-bp-shl-main-2.c:$decimal" \
+ "in foo at .*msym-bp-shl-lib.c:$decimal"
+ } else {
+ test_info_break_2 \
+ "<foo\\+$decimal>" \
+ "<foo\\+$decimal>"
+ }
+ }
+}
+
+foreach_with_prefix debug {0 1} {
+ test $debug
+}
diff --git a/gdb/testsuite/gdb.base/msym-bp.c b/gdb/testsuite/gdb.base/msym-bp.c
new file mode 100644
index 00000000000..2257d3f7e9b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp.c
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2020 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 void
+foo (void)
+{
+}
+
+int
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/msym-bp.exp b/gdb/testsuite/gdb.base/msym-bp.exp
new file mode 100644
index 00000000000..c4bef7c188b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/msym-bp.exp
@@ -0,0 +1,83 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2020 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 that when setting breakpoints, setting a breakpoint on "foo"
+# creates locations for all external and static functions called "foo"
+# in the program, with and without debug info.
+
+standard_testfile .c msym-bp-2.c
+
+# Run "info breakpoints", and check that we find the two locations,
+# LOC_A and LOC_B, in either order.
+proc test_info_break_2 {loc_a loc_b} {
+ set re1 ".*\.1.*${loc_a}\r\n.*\.2.*${loc_b}"
+ set re2 ".*\.1.*${loc_b}\r\n.*\.2.*${loc_a}"
+ gdb_test "info breakpoint" "$re1|$re2"
+}
+
+proc test {debug} {
+ global testfile srcfile srcfile2
+ global decimal
+
+ if {$debug} {
+ set options "debug"
+ } else {
+ set options ""
+ }
+
+ if { [prepare_for_testing "failed to prepare" $testfile-$debug \
+ [list $srcfile $srcfile2] $options] } {
+ return -1
+ }
+
+ # Should find two locations: the static foo in the msym-bp.c file,
+ # and the extern foo in the msym-bp-2.c file. Same result
+ # expected before and after running to main.
+ proc test_break {prefix} {
+ global decimal
+ upvar debug debug
+
+ with_test_prefix $prefix {
+ gdb_test "break foo" "\\(2 locations\\)"
+
+ if {$debug} {
+ test_info_break_2 \
+ "in foo at .*msym-bp.c:$decimal" \
+ "in foo at .*msym-bp-2.c:$decimal"
+ } else {
+ test_info_break_2 \
+ "<foo\\+$decimal>" \
+ "<foo\\+$decimal>"
+ }
+ }
+ }
+
+ test_break "before run"
+
+ if ![runto_main] {
+ fail "can't run to main"
+ return
+ }
+
+ delete_breakpoints
+
+ test_break "at main"
+}
+
+foreach_with_prefix debug {0 1} {
+ test $debug
+}
base-commit: 914592f9ff054c71bd1c8f0e352812ae20b699cb
--
2.14.5
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-06-12 19:21 [PATCH] Don't drop static function bp locations w/o debug info Pedro Alves
@ 2020-06-12 19:53 ` Simon Marchi
2020-09-13 13:25 ` Pedro Alves
0 siblings, 1 reply; 8+ messages in thread
From: Simon Marchi @ 2020-06-12 19:53 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2020-06-12 3:21 p.m., Pedro Alves wrote:
> @@ -4371,21 +4336,54 @@ search_minsyms_for_name (struct collect_info *info,
>
> if (!minsyms.empty ())
You could remove this check and de-indent the rest. If minsyms is empty, the loop
will just be skipped over.
Otherwise, LGTM.
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-06-12 19:53 ` Simon Marchi
@ 2020-09-13 13:25 ` Pedro Alves
2020-09-22 20:20 ` Luis Machado
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2020-09-13 13:25 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 6/12/20 8:53 PM, Simon Marchi wrote:
> On 2020-06-12 3:21 p.m., Pedro Alves wrote:
>> @@ -4371,21 +4336,54 @@ search_minsyms_for_name (struct collect_info *info,
>>
>> if (!minsyms.empty ())
>
> You could remove this check and de-indent the rest. If minsyms is empty, the loop
> will just be skipped over.
>
> Otherwise, LGTM.
Thanks, I did that, and merged it now.
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-09-13 13:25 ` Pedro Alves
@ 2020-09-22 20:20 ` Luis Machado
2020-09-24 15:16 ` Pedro Alves
0 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2020-09-22 20:20 UTC (permalink / raw)
To: Pedro Alves, Simon Marchi, gdb-patches
The new tests are failing for aarch64-linux-gnu on Ubuntu 18.04 (GCC 7.5.0).
The breakpoint location patterns don't seem to be matching. Here's what
I see...
info breakpoint^M
Num Type Disp Enb Address What^M
1 breakpoint keep y <MULTIPLE> ^M
1.1 y 0x0000000000000794 <foo@plt+4>^M
1.2 y 0x00000000000008bc <foo>^M
(gdb) FAIL: gdb.base/msym-bp-shl.exp: debug=0: before run: info breakpoint
info breakpoint^M
Num Type Disp Enb Address What^M
3 breakpoint keep y <MULTIPLE> ^M
3.1 y 0x0000aaaaaaaaa8bc <foo>^M
3.2 y 0x0000ffffb7fc065c <foo>^M
(gdb) FAIL: gdb.base/msym-bp-shl.exp: debug=0: at main: info breakpoint
info breakpoint^M
Num Type Disp Enb Address What^M
1 breakpoint keep y <MULTIPLE> ^M
1.1 y 0x00000000000006e4 <foo>^M
1.2 y 0x00000000000006f4 <foo>^M
(gdb) FAIL: gdb.base/msym-bp.exp: debug=0: before run: info breakpoint
info breakpoint^M
Num Type Disp Enb Address What^M
3 breakpoint keep y <MULTIPLE> ^M
3.1 y 0x0000aaaaaaaaa6e4 <foo>^M
3.2 y 0x0000aaaaaaaaa6f4 <foo>^M
(gdb) FAIL: gdb.base/msym-bp.exp: debug=0: at main: info breakpoint
Anything obvious?
On 9/13/20 10:25 AM, Pedro Alves wrote:
> On 6/12/20 8:53 PM, Simon Marchi wrote:
>> On 2020-06-12 3:21 p.m., Pedro Alves wrote:
>>> @@ -4371,21 +4336,54 @@ search_minsyms_for_name (struct collect_info *info,
>>>
>>> if (!minsyms.empty ())
>>
>> You could remove this check and de-indent the rest. If minsyms is empty, the loop
>> will just be skipped over.
>>
>> Otherwise, LGTM.
>
> Thanks, I did that, and merged it now.
>
> Pedro Alves
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-09-22 20:20 ` Luis Machado
@ 2020-09-24 15:16 ` Pedro Alves
2020-09-24 15:32 ` Luis Machado
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2020-09-24 15:16 UTC (permalink / raw)
To: Luis Machado, Simon Marchi, gdb-patches
On 9/22/20 9:20 PM, Luis Machado wrote:
> The new tests are failing for aarch64-linux-gnu on Ubuntu 18.04 (GCC 7.5.0).
>
> The breakpoint location patterns don't seem to be matching. Here's what I see...
>
> info breakpoint^M
> Num Type Disp Enb Address What^M
> 1 breakpoint keep y <MULTIPLE> ^M
> 1.1 y 0x0000000000000794 <foo@plt+4>^M
> 1.2 y 0x00000000000008bc <foo>^M
The test is:
test_info_break_2 \
"<foo@plt.*>" \
"<foo\\+$decimal>"
So it should be trivial to adjust with
- "<foo\\+$decimal>"
+ "<foo(\\+$decimal)?>"
However, I wonder whether we're actually seeing a bug here.
It seems like on x86, the breakpoint is set past the
prologue (thus the "+$decimal"):
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000001050 <foo@plt>
1.2 y 0x0000000000001165 <foo+8>
while it is set exactly at the function's address on Aarch64.
Why didn't GDB skip the prologue of <foo> on Aarch64?
Is there really no instruction to skip?
Here's what I see on x86-64, without debug info
(outputs/gdb.base/msym-bp-shl/msym-bp-shl-0):
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000001050 <foo@plt>
1.2 y 0x0000000000001165 <foo+8>
and on x86-64, with debug info
(outputs/gdb.base/msym-bp-shl/msym-bp-shl-1):
(gdb) info breakpoints
Num Type Disp Enb Address What
1 breakpoint keep y <MULTIPLE>
1.1 y 0x0000000000001050 <foo@plt>
1.2 y 0x0000000000001165 in foo at src/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c:21
Note the location addresses are the same without vs with
debug info: 0x1050 and 0x1165. Is that also the case on Aarch64?
Pedro Alves
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-09-24 15:16 ` Pedro Alves
@ 2020-09-24 15:32 ` Luis Machado
2020-09-24 15:35 ` Pedro Alves
0 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2020-09-24 15:32 UTC (permalink / raw)
To: Pedro Alves, Simon Marchi, gdb-patches
On 9/24/20 12:16 PM, Pedro Alves wrote:
> On 9/22/20 9:20 PM, Luis Machado wrote:
>> The new tests are failing for aarch64-linux-gnu on Ubuntu 18.04 (GCC 7.5.0).
>>
>> The breakpoint location patterns don't seem to be matching. Here's what I see...
>>
>> info breakpoint^M
>> Num Type Disp Enb Address What^M
>> 1 breakpoint keep y <MULTIPLE> ^M
>> 1.1 y 0x0000000000000794 <foo@plt+4>^M
>> 1.2 y 0x00000000000008bc <foo>^M
>
> The test is:
>
> test_info_break_2 \
> "<foo@plt.*>" \
> "<foo\\+$decimal>"
>
> So it should be trivial to adjust with
>
> - "<foo\\+$decimal>"
> + "<foo(\\+$decimal)?>"
>
>
> However, I wonder whether we're actually seeing a bug here.
> It seems like on x86, the breakpoint is set past the
> prologue (thus the "+$decimal"):
>
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 y 0x0000000000001050 <foo@plt>
> 1.2 y 0x0000000000001165 <foo+8>
>
> while it is set exactly at the function's address on Aarch64.
>
> Why didn't GDB skip the prologue of <foo> on Aarch64?
> Is there really no instruction to skip?
>
> Here's what I see on x86-64, without debug info
> (outputs/gdb.base/msym-bp-shl/msym-bp-shl-0):
>
> (gdb) info breakpoints
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 y 0x0000000000001050 <foo@plt>
> 1.2 y 0x0000000000001165 <foo+8>
>
> and on x86-64, with debug info
> (outputs/gdb.base/msym-bp-shl/msym-bp-shl-1):
>
> (gdb) info breakpoints
> Num Type Disp Enb Address What
> 1 breakpoint keep y <MULTIPLE>
> 1.1 y 0x0000000000001050 <foo@plt>
> 1.2 y 0x0000000000001165 in foo at src/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c:21
>
> Note the location addresses are the same without vs with
> debug info: 0x1050 and 0x1165. Is that also the case on Aarch64?
It is the same case for AArch64. The reason for not skipping a prologue
is because there doesn't seem to be one. The generated code for aarch64
is the following:
(gdb) disass foo
0x00000000000008bc <+0>: nop
0x00000000000008c0 <+4>: ret
End of assembler dump.
I guess the assumptions of the test are valid for x86, but they're not
valid for aarch64.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-09-24 15:32 ` Luis Machado
@ 2020-09-24 15:35 ` Pedro Alves
2020-09-24 15:44 ` Luis Machado
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2020-09-24 15:35 UTC (permalink / raw)
To: Luis Machado, Simon Marchi, gdb-patches
On 9/24/20 4:32 PM, Luis Machado wrote:
> On 9/24/20 12:16 PM, Pedro Alves wrote:
>
> It is the same case for AArch64. The reason for not skipping a prologue is because there doesn't seem to be one. The generated code for aarch64 is the following:
>
> (gdb) disass foo
> 0x00000000000008bc <+0>: nop
> 0x00000000000008c0 <+4>: ret
> End of assembler dump.
>
> I guess the assumptions of the test are valid for x86, but they're not valid for aarch64.
>
OK. So the
- "<foo\\+$decimal>"
+ "<foo(\\+$decimal)?>"
fix should be all that's necessary. Do you want to give it a try?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't drop static function bp locations w/o debug info
2020-09-24 15:35 ` Pedro Alves
@ 2020-09-24 15:44 ` Luis Machado
0 siblings, 0 replies; 8+ messages in thread
From: Luis Machado @ 2020-09-24 15:44 UTC (permalink / raw)
To: Pedro Alves, Simon Marchi, gdb-patches
On 9/24/20 12:35 PM, Pedro Alves wrote:
> On 9/24/20 4:32 PM, Luis Machado wrote:
>> On 9/24/20 12:16 PM, Pedro Alves wrote:
>>
>> It is the same case for AArch64. The reason for not skipping a prologue is because there doesn't seem to be one. The generated code for aarch64 is the following:
>>
>> (gdb) disass foo
>> 0x00000000000008bc <+0>: nop
>> 0x00000000000008c0 <+4>: ret
>> End of assembler dump.
>>
>> I guess the assumptions of the test are valid for x86, but they're not valid for aarch64.
>>
>
> OK. So the
>
> - "<foo\\+$decimal>"
> + "<foo(\\+$decimal)?>"
>
> fix should be all that's necessary. Do you want to give it a try?
>
Sure. I'll give that a try and will push.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-24 15:44 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-12 19:21 [PATCH] Don't drop static function bp locations w/o debug info Pedro Alves
2020-06-12 19:53 ` Simon Marchi
2020-09-13 13:25 ` Pedro Alves
2020-09-22 20:20 ` Luis Machado
2020-09-24 15:16 ` Pedro Alves
2020-09-24 15:32 ` Luis Machado
2020-09-24 15:35 ` Pedro Alves
2020-09-24 15:44 ` Luis Machado
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).