[patch] Fix a crash when displaying variables from shared library.
Paul Pluzhnikov
ppluzhnikov@google.com
Wed Mar 4 19:26:00 GMT 2009
On Tue, Mar 3, 2009 at 4:48 PM, Tom Tromey <tromey@redhat.com> wrote:
> Paul> Is it ok to move observer notification to before
> Paul> objfile_purge_solibs, or should I add a new notification?
>
> IMO it is ok to move this notification if you audit the existing users
> to make sure they don't break.
I ended up just swapping the order of clear_solib and objfile_purge_solibs
in no_shared_libraries. AFAICT, objfile_purge_solibs is "lower level"
and doesn't need anything from the solib list.
Regtested on Linux/x86_64 with no regressions.
> However, it seems to me that you could also do this another way, by
> noting at parse time which objfiles are referenced by a given display,
> and then arranging to require a re-parse when an objfile is destroyed.
> I think the existing objfile_data machinery could be used for this.
Thanks for the pointer.
I tried that, but it ended up being very tangled and inefficient mess,
and I abandoned this approach.
Thanks,
--
Paul Pluzhnikov
ChangeLog:
2009-03-04 Paul Pluzhnikov <ppluzhnikov@google.com>
* printcmd.c (do_one_display): Reparse exp_string.
(display_uses_solib_p): New function.
(clear_dangling_display_expressions): New function.
(_initialize_printcmd): Add observer.
* solib.c (no_shared_libraries): Swap order of calls to
clear_solib and objfile_purge_solibs.
testsuite/ChangeLog:
2009-02-04 Paul Pluzhnikov <ppluzhnikov@google.com>
* solib-display.exp: New file.
* solib-display-main.c: New file.
* solib-display-lib.c: New file.
-------------- next part --------------
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.144
diff -u -p -u -r1.144 printcmd.c
--- printcmd.c 25 Feb 2009 18:26:53 -0000 1.144
+++ printcmd.c 4 Mar 2009 19:17:56 -0000
@@ -43,6 +43,11 @@
#include "disasm.h"
#include "dfp.h"
#include "valprint.h"
+#include "exceptions.h"
+#include "observer.h"
+#include "solist.h"
+#include "solib.h"
+#include "parser-defs.h"
#ifdef TUI
#include "tui/tui.h" /* For tui_active et.al. */
@@ -1395,7 +1400,7 @@ display_command (char *exp, int from_tty
fmt.count = 0;
}
- innermost_block = 0;
+ innermost_block = NULL;
expr = parse_expression (exp);
new = (struct display *) xmalloc (sizeof (struct display));
@@ -1519,6 +1524,25 @@ do_one_display (struct display *d)
if (d->enabled_p == 0)
return;
+ if (d->exp == NULL)
+ {
+ volatile struct gdb_exception ex;
+ TRY_CATCH (ex, RETURN_MASK_ALL)
+ {
+ innermost_block = NULL;
+ d->exp = parse_expression (d->exp_string);
+ d->block = innermost_block;
+ }
+ if (ex.reason < 0)
+ {
+ /* Can't re-parse the expression. Disable this display item. */
+ d->enabled_p = 0;
+ warning (_("Unable to display \"%s\": %s"),
+ d->exp_string, ex.message);
+ return;
+ }
+ }
+
if (d->block)
within_current_scope = contained_in (get_selected_block (0), d->block);
else
@@ -1731,6 +1755,74 @@ disable_display_command (char *args, int
p++;
}
}
+
+/* Return 1 if D uses SOLIB (and will become dangling when SOLIB
+ is unloaded), otherwise return 0. */
+
+static int
+display_uses_solib_p (const struct display *d,
+ const struct so_list *solib)
+{
+ int i;
+ struct expression *const exp = d->exp;
+
+ if (d->block != NULL &&
+ solib_address (d->block->startaddr) == solib->so_name)
+ return 1;
+
+ for (i = 0; i < exp->nelts; )
+ {
+ int args, oplen = 0;
+ const union exp_element *const elts = exp->elts;
+
+ if (elts[i].opcode == OP_VAR_VALUE)
+ {
+ const struct block *const block = elts[i + 1].block;
+ const struct symbol *const symbol = elts[i + 2].symbol;
+ const struct obj_section *const section =
+ SYMBOL_OBJ_SECTION (symbol);
+
+ gdb_assert (elts[i + 3].opcode == OP_VAR_VALUE);
+
+ if (block != NULL
+ && solib_address (block->startaddr) == solib->so_name)
+ return 1;
+
+ if (section && section->objfile == solib->objfile)
+ return 1;
+ }
+ exp->language_defn->la_exp_desc->operator_length (exp, i + 1,
+ &oplen, &args);
+ gdb_assert (oplen > 0);
+ i += oplen;
+ }
+ return 0;
+}
+
+/* display_chain items point to blocks and expressions. Some expressions in
+ turn may point to symbols.
+ Both symbols and blocks are obstack_alloc'd on objfile_stack, and are
+ obstack_free'd when a shared library is unloaded.
+ Clear pointers that are about to become dangling.
+ Both .exp and .block fields will be restored next time we need to display
+ an item by re-parsing .exp_string field in the new execution context. */
+
+static void
+clear_dangling_display_expressions (struct so_list *solib)
+{
+ struct display *d;
+ struct objfile *objfile = NULL;
+
+ for (d = display_chain; d; d = d->next)
+ {
+ if (d->exp && display_uses_solib_p (d, solib))
+ {
+ xfree (d->exp);
+ d->exp = NULL;
+ d->block = NULL;
+ }
+ }
+}
/* Print the value in stack frame FRAME of a variable specified by a
@@ -2367,6 +2459,8 @@ _initialize_printcmd (void)
current_display_number = -1;
+ observer_attach_solib_unloaded (clear_dangling_display_expressions);
+
add_info ("address", address_info,
_("Describe where symbol SYM is stored."));
Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.111
diff -u -p -u -r1.111 solib.c
--- solib.c 21 Feb 2009 16:14:49 -0000 1.111
+++ solib.c 4 Mar 2009 19:17:56 -0000
@@ -999,8 +999,13 @@ sharedlibrary_command (char *args, int f
void
no_shared_libraries (char *ignored, int from_tty)
{
- objfile_purge_solibs ();
+ /* The order of the two routines below is important: clear_solib
+ will notify observers, and at least clear_dangling_display_expressions
+ observer needs access to objfiles associated with solibs being
+ cleared. */
+
clear_solib ();
+ objfile_purge_solibs ();
}
static void
Index: testsuite/gdb.base/solib-display-lib.c
===================================================================
RCS file: testsuite/gdb.base/solib-display-lib.c
diff -N testsuite/gdb.base/solib-display-lib.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/solib-display-lib.c 4 Mar 2009 19:17:56 -0000
@@ -0,0 +1,30 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2009 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 a_global = 41;
+#ifndef NO_B_GLOBAL
+int b_global = 42;
+#endif
+int c_global = 43;
+
+int foo () {
+ return a_global +
+#ifndef NO_B_GLOBAL
+ b_global +
+#endif
+ c_global;
+}
Index: testsuite/gdb.base/solib-display-main.c
===================================================================
RCS file: testsuite/gdb.base/solib-display-main.c
diff -N testsuite/gdb.base/solib-display-main.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/solib-display-main.c 4 Mar 2009 19:17:56 -0000
@@ -0,0 +1,32 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2009 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 foo ();
+
+int main_global = 44;
+int bar ()
+{
+ int a_local = 45;
+ static int a_static = 46;
+ return main_global + a_local + a_static; /* break here */
+}
+
+int main ()
+{
+ bar ();
+ return foo ();
+}
Index: testsuite/gdb.base/solib-display.exp
===================================================================
RCS file: testsuite/gdb.base/solib-display.exp
diff -N testsuite/gdb.base/solib-display.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/solib-display.exp 4 Mar 2009 19:17:56 -0000
@@ -0,0 +1,115 @@
+# Copyright 2009 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/>.
+#
+# Contributed by Paul Pluzhnikov <ppluzhnikov@google.com>
+#
+
+# This test case verifies that if a display is active on a variable
+# which belongs in a shared library, and that shared library is
+# reloaded (e.g. due to re-execution of the program), GDB will continue
+# to display it (gdb-6.8 crashed under this scenario).
+
+# Also test that a display of variable which is currently present in
+# a shared library, but disappears before re-run, doesn't cause GDB
+# difficulties, and that it continues to display other variables.
+
+# Finally, test that displays which refer to main executable
+# (and thus aren't affected by shared library unloading) are not
+# disabled prematurely.
+
+if [skip_shlib_tests] then {
+ return 0
+}
+
+# Library file.
+set libname "solib-display-lib"
+set srcfile_lib ${srcdir}/${subdir}/${libname}.c
+set binfile_lib ${objdir}/${subdir}/${libname}.so
+set lib_flags [list debug]
+# Binary file.
+set testfile "solib-display-main"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+set bin_flags [list debug shlib=${binfile_lib}]
+
+if [get_compiler_info ${binfile}] {
+ return -1
+}
+
+if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != ""
+ || [gdb_compile ${srcfile} ${binfile} executable $bin_flags] != "" } {
+ untested "Could not compile $binfile_lib or $binfile."
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+ fail "Can't run to main"
+ return 0
+}
+
+gdb_test "display a_global" "1: a_global = 41"
+gdb_test "display b_global" "2: b_global = 42"
+gdb_test "display c_global" "3: c_global = 43"
+
+if ![runto_main] then {
+ fail "Can't run to main (2)"
+ return 0
+}
+
+gdb_test "stepi" "3: c_global = 43\\r\\n2: b_global = 42\\r\\n1: a_global = 41" "after rerun"
+
+# Now rebuild the library without b_global
+if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} \
+ "$lib_flags additional_flags=-DNO_B_GLOBAL"] != ""} {
+ fail "Can't rebuild $binfile_lib"
+}
+
+if ![runto_main] then {
+ fail "Can't run to main (3)"
+ return 0
+}
+
+gdb_test "stepi" "3: c_global = 43\\r\\n1: a_global = 41" "after rebuild"
+
+# Now verify that displays which are not in the shared library
+# are not cleared permaturely.
+
+gdb_test "break [gdb_get_line_number "break here" ${testfile}.c]" \
+ ".*Breakpoint.* at .*"
+
+gdb_test "continue"
+gdb_test "display main_global" "4: main_global = 44"
+gdb_test "display a_local" "5: a_local = 45"
+gdb_test "display a_static" "6: a_static = 46"
+
+if ![runto_main] then {
+ fail "Can't run to main (4)"
+ return 0
+}
+
+gdb_test "stepi" "6: a_static = 46\\r\\n4: main_global = 44\\r\\n.*"
+gdb_test "break [gdb_get_line_number "break here" ${testfile}.c]" \
+ ".*Breakpoint.* at .*"
+gdb_test "continue" "6: a_static = 46\\r\\n5: a_local = 45\\r\\n4: main_global = 44\\r\\n.*"
+
+gdb_exit
+
+return 0
+
+
More information about the Gdb-patches
mailing list