Fix PR11094: JIT breakpoint is not properly recreated on reruns
authorPedro Alves <palves@redhat.com>
Thu, 6 Oct 2016 11:53:52 +0000 (12:53 +0100)
committerPedro Alves <palves@redhat.com>
Thu, 6 Oct 2016 11:53:52 +0000 (12:53 +0100)
Even though this was supposedly in the gdb 7.2 timeframe, the testcase
in PR11094 crashes current GDB with a segfault:

  Program received signal SIGSEGV, Segmentation fault.
  0x00000000005ee894 in event_location_to_string (location=0x0) at
  src/gdb/location.c:412
  412       if (EL_STRING (location) == NULL)
  (top-gdb) bt
  #0  0x00000000005ee894 in event_location_to_string (location=0x0) at
  src/gdb/location.c:412
  #1  0x000000000057411a in print_breakpoint_location (b=0x18288e0, loc=0x0) at
  src/gdb/breakpoint.c:6201
  #2  0x000000000057483f in print_one_breakpoint_location (b=0x18288e0,
  loc=0x182cf10, loc_number=0, last_loc=0x7fffffffd258, allflag=1)
      at src/gdb/breakpoint.c:6473
  #3  0x00000000005751e1 in print_one_breakpoint (b=0x18288e0,
  last_loc=0x7fffffffd258, allflag=1) at
  src/gdb/breakpoint.c:6707
  #4  0x000000000057589c in breakpoint_1 (args=0x0, allflag=1, filter=0x0) at
  src/gdb/breakpoint.c:6947
  #5  0x0000000000575aa8 in maintenance_info_breakpoints (args=0x0, from_tty=0)
  at src/gdb/breakpoint.c:7026
  [...]

This is GDB trying to print the location spec of the JIT event
breakpoint, but that's an internal breakpoint without one.

If I add a NULL check, then we see that the JIT breakpoint is now
pending (because its location has shlib_disabled set):

  (gdb) maint info breakpoints
  Num     Type           Disp Enb Address            What
  [...]
  -8      jit events     keep y   <PENDING>           inf 1
  [...]

But that's incorrect.  GDB should have managed to recreate the JIT
breakpoint's location for the second run.  So the problem is
elsewhere.

The problem is that if the JIT loads at the same address on the second
run, we never recreate the JIT breakpoint, because we hit this early
return:

  static int
  jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
  struct jit_program_space_data *ps_data)
  {
    [...]
    if (ps_data->cached_code_address == addr)
      return 0;

    [...]
      delete_breakpoint (ps_data->jit_breakpoint);
    [...]
    ps_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr);

Fix this by deleting the breakpoint and discarding the cached code
address when the objfile where the previous JIT breakpoint was found
is deleted/unloaded in the first place.

The test that was originally added for PR11094 doesn't trip on this
because:

  #1 - It doesn't test the case of the JIT descriptor's address _not_
       changing between reruns.

  #2 - And then it doesn't do "maint info breakpoints", or really
       anything with the JIT at all.

  #3 - and even then, to trigger the problem the JIT descriptor needs
       to be in a separate library, while the current test puts it in
       the main program.

The patch extends the test to cover all combinations of these
scenarios.

gdb/ChangeLog:
2016-10-06  Pedro Alves  <palves@redhat.com>

* jit.c (free_objfile_data): Delete the JIT breakpoint and clear
the cached code address.

gdb/testsuite/ChangeLog:
2016-10-06  Pedro Alves  <palves@redhat.com>

* gdb.base/jit-simple-dl.c: New file.
* gdb.base/jit-simple-jit.c: New file, factored out from ...
* gdb.base/jit-simple.c: ... this.
* gdb.base/jit-simple.exp (jit_run): Delete.
(build_jit): New proc.
(jit_test_reread): Recompile either the main program or the shared
library, depending on what is being tested.  Skip changing address
if caller wants to.  Compare before/after addresses.  If testing
standalone, explicitly load the binary.  Test "maint info
breakpoints".
(top level): Add "standalone vs shared lib" and "change address"
vs "same address" axes.

gdb/ChangeLog
gdb/jit.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/jit-simple-dl.c [new file with mode: 0644]
gdb/testsuite/gdb.base/jit-simple-jit.c [new file with mode: 0644]
gdb/testsuite/gdb.base/jit-simple.c
gdb/testsuite/gdb.base/jit-simple.exp

index 084ab130418018f380bda9ce27bdfe474d8474c0..3f222d6b9635481484275abc8fc95a7abe99b2bc 100644 (file)
@@ -1,3 +1,8 @@
+2016-10-06  Pedro Alves  <palves@redhat.com>
+
+       * jit.c (free_objfile_data): Delete the JIT breakpoint and clear
+       the cached code address.
+
 2016-10-06  Doug Evans  <dje@google.com>
 
        * features/aarch64-core.xml (cpsr_flags): Elide "type" and specify
index 81a2c26ccf6c59a8cffd862013faa60dfe74bd9b..03b6bd80c6924cbfaa57d9e0f5e72f727e5e5faa 100644 (file)
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1482,7 +1482,11 @@ free_objfile_data (struct objfile *objfile, void *data)
        = ((struct jit_program_space_data *)
           program_space_data (objfile->pspace, jit_program_space_data));
       if (ps_data != NULL && ps_data->objfile == objfile)
-       ps_data->objfile = NULL;
+       {
+         ps_data->objfile = NULL;
+         delete_breakpoint (ps_data->jit_breakpoint);
+         ps_data->cached_code_address = 0;
+       }
     }
 
   xfree (data);
index 5335344a06a40b021e580394dfef38db2633e428..b27ca79eb19bb4370b45044aa464d2b4b5ab7d87 100644 (file)
@@ -1,3 +1,18 @@
+2016-10-06  Pedro Alves  <palves@redhat.com>
+
+       * gdb.base/jit-simple-dl.c: New file.
+       * gdb.base/jit-simple-jit.c: New file, factored out from ...
+       * gdb.base/jit-simple.c: ... this.
+       * gdb.base/jit-simple.exp (jit_run): Delete.
+       (build_jit): New proc.
+       (jit_test_reread): Recompile either the main program or the shared
+       library, depending on what is being tested.  Skip changing address
+       if caller wants to.  Compare before/after addresses.  If testing
+       standalone, explicitly load the binary.  Test "maint info
+       breakpoints".
+       (top level): Add "standalone vs shared lib" and "change address"
+       vs "same address" axes.
+
 2016-10-06  Pedro Alves  <palves@redhat.com>
 
        * gdb.base/jit-simple.exp (top level) Delete get_compiler_info
diff --git a/gdb/testsuite/gdb.base/jit-simple-dl.c b/gdb/testsuite/gdb.base/jit-simple-dl.c
new file mode 100644 (file)
index 0000000..074dcdd
--- /dev/null
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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/>.  */
+
+/* A stub program that links with a simple library that uses the JIT
+   API.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/jit-simple-jit.c b/gdb/testsuite/gdb.base/jit-simple-jit.c
new file mode 100644 (file)
index 0000000..93b846c
--- /dev/null
@@ -0,0 +1,50 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2016 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/>.  */
+
+/* Simple library using the JIT API.  */
+
+#include <stdint.h>
+
+struct jit_code_entry
+{
+  struct jit_code_entry *next_entry;
+  struct jit_code_entry *prev_entry;
+  const char *symfile_addr;
+  uint64_t symfile_size;
+};
+
+struct jit_descriptor
+{
+  uint32_t version;
+  /* This type should be jit_actions_t, but we use uint32_t
+     to be explicit about the bitwidth.  */
+  uint32_t action_flag;
+  struct jit_code_entry *relevant_entry;
+  struct jit_code_entry *first_entry;
+};
+
+#ifdef SPACER
+/* This exists to change the address of __jit_debug_descriptor.  */
+int spacer = 4;
+#endif
+
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+
+void
+__jit_debug_register_code (void)
+{
+}
index 3893c3d7d8b1f8c306a50f808b6dd3a5c2c8ceec..9f1d27b7ac93effb7e1256b4f8f25a7004e54d73 100644 (file)
@@ -1,37 +1,26 @@
-/* Simple program using the JIT API.  */
+/* This testcase is part of GDB, the GNU debugger.
 
-#include <stdint.h>
+   Copyright 2016 Free Software Foundation, Inc.
 
-struct jit_code_entry
-{
-  struct jit_code_entry *next_entry;
-  struct jit_code_entry *prev_entry;
-  const char *symfile_addr;
-  uint64_t symfile_size;
-};
+   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.
 
-struct jit_descriptor
-{
-  uint32_t version;
-  /* This type should be jit_actions_t, but we use uint32_t
-     to be explicit about the bitwidth.  */
-  uint32_t action_flag;
-  struct jit_code_entry *relevant_entry;
-  struct jit_code_entry *first_entry;
-};
+   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.
 
-#ifdef SPACER
-/* This exists to change the address of __jit_debug_descriptor.  */
-int spacer = 4;
-#endif
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+/* Simple standalone program using the JIT API.  */
 
-void __jit_debug_register_code()
-{
-}
+#include "jit-simple-jit.c"
 
-int main()
+int
+main (void)
 {
   return 0;
 }
index 6fe6011baa18a84e95785edbcf231ea2bcd56030..c1a079a67a8fe3354ec7736a9a9a563418756d98 100644 (file)
 # 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 re-running an inferior with a JIT descriptor, where the JIT
+# descriptor changes address between runs.
+# http://sourceware.org/bugzilla/show_bug.cgi?id=13431
+
+# Test both the case of the JIT reader being included in the main
+# program directly, and the case of the JIT reader being split out to
+# a shared library.
+
+# For completeness, also test when the JIT descriptor does not change
+# address between runs.
+
 if {[skip_shlib_tests]} {
     untested jit-simple.exp
     return -1
@@ -20,25 +31,81 @@ if {[skip_shlib_tests]} {
 
 standard_testfile
 
-if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} {
+set libname $testfile-jit
+set srcfile_lib $srcdir/$subdir/$libname.c
+set binfile_lib [standard_output_file $libname.so]
+
+# Build a standalone JIT binary.
+
+proc build_standalone_jit {{options ""}} {
+    global testfile srcfile binfile
+
+    lappend options "debug"
+
+    if {[build_executable $testfile.exp $testfile $srcfile $options] == -1} {
+       return -1
+    }
+
+    return 0
+}
+
+# Build the shared library JIT.
+
+proc build_shared_jit {{options ""}} {
+    global testfile
+    global srcfile_lib binfile_lib
+
+    lappend options "debug additional_flags=-fPIC"
+    if { [gdb_compile_shlib $srcfile_lib $binfile_lib $options] != "" } {
+       return -1
+    }
+
+    return 0
+}
+
+if {[build_standalone_jit] == -1} {
+    untested "could not compile $binfile"
+    return
+}
+
+if {[build_shared_jit] == -1} {
+    untested "could not compile $binfile_lib"
+    return
+}
+
+# Built the program that loads the JIT library.
+set srcfile_dl $testfile-dl.c
+set binfile_dl $binfile-dl
+set options [list debug shlib=${binfile_lib}]
+if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl executable \
+        [list debug shlib=$binfile_lib]] == -1 } {
     untested jit-simple.exp
     return -1
 }
 
-# Test re-running an inferior with a JIT descriptor, where the JIT
-# descriptor changes address between runs.
-# http://sourceware.org/bugzilla/show_bug.cgi?id=13431
-proc jit_test_reread {} {
-    global testfile binfile subdir srcfile srcdir
+# STANDALONE is true when the JIT reader is included directly in the
+# main program.  False when the JIT reader is in a separate shared
+# library.  If CHANGE_ADDR is true, force changing the JIT descriptor
+# changes address between runs.
+proc jit_test_reread {standalone change_addr} {
+    global testfile binfile subdir srcfile srcdir binfile_lib binfile_dl
     global hex
 
     with_test_prefix "initial run" {
-       clean_restart $testfile
+       if {$standalone} {
+           clean_restart $binfile
+       } else {
+           clean_restart $binfile_dl
+       }
 
        runto_main
 
        set addr_before [get_hexadecimal_valueof "&__jit_debug_descriptor" 0 \
                             "get address of __jit_debug_descriptor"]
+
+       gdb_test "maint info breakpoints" \
+           "jit events     keep y   $hex <__jit_debug_register_code>.*" \
+           "maint info breakpoints shows jit breakpoint"
     }
 
     with_test_prefix "second run" {
@@ -47,21 +114,49 @@ proc jit_test_reread {} {
        # second, gdb might not reload the executable automatically.
        sleep 1
 
-       gdb_rename_execfile $binfile ${binfile}x
-       if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DSPACER}] != "" } {
-           fail "recompile"
-           return
-       } else {
-           pass "recompile"
+       if ${change_addr} {
+           set options "additional_flags=-DSPACER"
+           if {$standalone} {
+               gdb_rename_execfile $binfile ${binfile}x
+               set res [build_standalone_jit $options]
+           } else {
+               gdb_rename_execfile $binfile_lib ${binfile_lib}x
+               set res [build_shared_jit $options]
+           }
+           if  { $res == -1 } {
+               fail "recompile"
+               return
+           } else {
+               pass "recompile"
+           }
        }
 
        runto_main
 
        set addr_after [get_hexadecimal_valueof "&__jit_debug_descriptor" 0 \
                            "get address of __jit_debug_descriptor"]
+
+       # This used to crash in the JIT-in-shared-library case:
+       # https://sourceware.org/bugzilla/show_bug.cgi?id=11094
+       gdb_test "maint info breakpoints" \
+           "jit events     keep y   $hex <__jit_debug_register_code>.*" \
+           "maint info breakpoints shows jit breakpoint"
     }
 
-    gdb_assert {$addr_before != $addr_after} "address changed"
+    if ${change_addr} {
+       gdb_assert {$addr_before != $addr_after} "address changed"
+    } else {
+       gdb_assert {$addr_before == $addr_after} "address didn't change"
+    }
 }
 
-jit_test_reread
+foreach standalone {1 0} {
+    with_test_prefix [expr ($standalone)?"standalone":"shared"] {
+       with_test_prefix "change addr" {
+           jit_test_reread $standalone 1
+       }
+       with_test_prefix "same addr" {
+           jit_test_reread $standalone 0
+       }
+    }
+}