[PATCH 2/2] Disable breakpoint locations in unloaded jit objects

Mihails Strasuns mihails.strasuns@intel.com
Tue Jun 16 11:45:30 GMT 2020


Fixes the rare problem when identical JIT code is registered and
unregistered several times and its symbols happen to be relocated to the
exactly the same addresses as previous time.  In such situation gdb
would still think that old breakpoint locations are still inserted
(because `inserted` flag is set and objfile/address is the same) despite
the fact that code memory was overwritten and does not contain a
breakpoint trap anymore.

This fix was suggested by Simon Marchi <simark@simark.ca> and extends
`disable_breakpoints_in_unloaded_shlib` to work on any object file and
uses it to disable breakpoints for both shared libraries and jit
objects.

gdb/ChangeLog:
2020-06-16  Mihails Strasuns  <mihails.strasuns@intel.com>

	* breakpoint.c (disable_breakpoints_in_unloaded_shlib): Renamed
	to 'disable_breakpoints_in_unloaded_objfile` and changed to
	accept an objfile as an argument.
	(disable_breakpoints_in_unloaded_shlib): New function.
	disable_breakpoints_in_unloaded_jit_objects): New function.
	* jit.c (jit_unregister_code): New function, called to handle
	JIT_UNREGISTER and discard a jit objfile properly.
	* observable.c, observable.h: New observable, 'jit_object_unloaded'.

gdb/testsuite/ChangeLog:
2020-06-16  Mihails Strasuns  <mihails.strasuns@intel.com>

	* gdb.base/jit-elf-reregister.c, gdb.base/jit-elf-reregister.exp: New
	test case, verifies that breakpoints still hit if a jit object
	that contains them gets reloaded and mapped again to the same
	base address.
---
 gdb/breakpoint.c                              | 29 +++++--
 gdb/jit.c                                     | 31 +++++---
 gdb/observable.c                              |  1 +
 gdb/observable.h                              |  2 +
 gdb/testsuite/gdb.base/jit-elf-reregister.c   | 66 ++++++++++++++++
 gdb/testsuite/gdb.base/jit-elf-reregister.exp | 78 +++++++++++++++++++
 6 files changed, 190 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-elf-reregister.c
 create mode 100644 gdb/testsuite/gdb.base/jit-elf-reregister.exp

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index aead882acd..cc15c47303 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7487,12 +7487,8 @@ disable_breakpoints_in_shlibs (void)
   }
 }
 
-/* Disable any breakpoints and tracepoints that are in SOLIB upon
-   notification of unloaded_shlib.  Only apply to enabled breakpoints,
-   disabled ones can just stay disabled.  */
-
 static void
-disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
+disable_breakpoints_in_unloaded_objfile (objfile *objfile)
 {
   struct bp_location *loc, **locp_tmp;
   int disabled_shlib_breaks = 0;
@@ -7502,7 +7498,7 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
     /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL.  */
     struct breakpoint *b = loc->owner;
 
-    if (solib->pspace == loc->pspace
+    if (objfile->pspace == loc->pspace
 	&& !loc->shlib_disabled
 	&& (((b->type == bp_breakpoint
 	      || b->type == bp_jit_event
@@ -7510,7 +7506,7 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
 	     && (loc->loc_type == bp_loc_hardware_breakpoint
 		 || loc->loc_type == bp_loc_software_breakpoint))
 	    || is_tracepoint (b))
-	&& solib_contains_address_p (solib, loc->address))
+	&& is_addr_in_objfile (loc->address, objfile))
       {
 	loc->shlib_disabled = 1;
 	/* At this point, we cannot rely on remove_breakpoint
@@ -7526,13 +7522,29 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
 	    target_terminal::ours_for_output ();
 	    warning (_("Temporarily disabling breakpoints "
 		       "for unloaded shared library \"%s\""),
-		     solib->so_name);
+		     objfile->original_name);
 	  }
 	disabled_shlib_breaks = 1;
       }
   }
 }
 
+/* Disable any breakpoints and tracepoints that are in SOLIB upon
+   notification of unloaded_shlib.  Only apply to enabled breakpoints,
+   disabled ones can just stay disabled.  */
+
+static void
+disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
+{
+  disable_breakpoints_in_unloaded_objfile (solib->objfile);
+}
+
+static void
+disable_breakpoints_in_unloaded_jit_object (objfile *objfile)
+{
+  disable_breakpoints_in_unloaded_objfile (objfile);
+}
+
 /* Disable any breakpoints and tracepoints in OBJFILE upon
    notification of free_objfile.  Only apply to enabled breakpoints,
    disabled ones can just stay disabled.  */
@@ -15435,6 +15447,7 @@ _initialize_breakpoint ()
   initialize_breakpoint_ops ();
 
   gdb::observers::solib_unloaded.attach (disable_breakpoints_in_unloaded_shlib);
+  gdb::observers::jit_object_unloaded.attach (disable_breakpoints_in_unloaded_jit_object);
   gdb::observers::free_objfile.attach (disable_breakpoints_in_freed_objfile);
   gdb::observers::memory_changed.attach (invalidate_bp_value_on_memory_change);
 
diff --git a/gdb/jit.c b/gdb/jit.c
index 1b5ef46469..16e38078bb 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -925,6 +925,27 @@ jit_find_objf_with_entry_addr (CORE_ADDR entry_addr)
   return NULL;
 }
 
+/* This function unregisters JITed code and does the necessary cleanup.  */
+
+static void
+jit_unregister_code (struct gdbarch *gdbarch, CORE_ADDR entry_addr)
+{
+  struct objfile *objfile = nullptr;
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "jit_unregister_code (%s)\n",
+			host_address_to_string (objfile));
+  objfile = jit_find_objf_with_entry_addr (entry_addr);
+  if (objfile == NULL)
+    printf_unfiltered (_ ("Unable to find JITed code "
+			  "entry at address: %s\n"),
+		       paddress (gdbarch, entry_addr));
+  else
+    {
+      gdb::observers::jit_object_unloaded.notify (objfile);
+      objfile->unlink ();
+    }
+}
+
 /* This is called when a breakpoint is deleted.  It updates the
    inferior's cache, if needed.  */
 
@@ -1335,7 +1356,6 @@ jit_event_handler (struct gdbarch *gdbarch)
   struct jit_descriptor descriptor;
   struct jit_code_entry code_entry;
   CORE_ADDR entry_addr;
-  struct objfile *objf;
 
   /* Read the descriptor from remote memory.  */
   if (!jit_read_descriptor (gdbarch, &descriptor,
@@ -1353,14 +1373,7 @@ jit_event_handler (struct gdbarch *gdbarch)
       jit_register_code (gdbarch, entry_addr, &code_entry);
       break;
     case JIT_UNREGISTER:
-      objf = jit_find_objf_with_entry_addr (entry_addr);
-      if (objf == NULL)
-	printf_unfiltered (_("Unable to find JITed code "
-			     "entry at address: %s\n"),
-			   paddress (gdbarch, entry_addr));
-      else
-	objf->unlink ();
-
+      jit_unregister_code (gdbarch, entry_addr);
       break;
     default:
       error (_("Unknown action_flag value in JIT descriptor!"));
diff --git a/gdb/observable.c b/gdb/observable.c
index 81aa392cc2..92354c64a3 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -46,6 +46,7 @@ DEFINE_OBSERVABLE (inferior_created);
 DEFINE_OBSERVABLE (record_changed);
 DEFINE_OBSERVABLE (solib_loaded);
 DEFINE_OBSERVABLE (solib_unloaded);
+DEFINE_OBSERVABLE (jit_object_unloaded);
 DEFINE_OBSERVABLE (new_objfile);
 DEFINE_OBSERVABLE (free_objfile);
 DEFINE_OBSERVABLE (new_thread);
diff --git a/gdb/observable.h b/gdb/observable.h
index 070cf0f18e..b60ee7f3ef 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -111,6 +111,8 @@ extern observable<struct so_list */* solib */> solib_loaded;
    been unloaded yet, and thus are still available.  */
 extern observable<struct so_list */* solib */> solib_unloaded;
 
+extern observable<objfile */* objfile */> jit_object_unloaded;
+
 /* The symbol file specified by OBJFILE has been loaded.  Called
    with OBJFILE equal to NULL to indicate previously loaded symbol
    table data has now been invalidated.  */
diff --git a/gdb/testsuite/gdb.base/jit-elf-reregister.c b/gdb/testsuite/gdb.base/jit-elf-reregister.c
new file mode 100644
index 0000000000..2416259dcf
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-reregister.c
@@ -0,0 +1,66 @@
+/* This test program 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/>.  */
+
+#include "jit-protocol.h"
+#include "jit-elf-util.h"
+
+/* Must be defined by .exp file when compiling to know
+   what address to map the ELF binary to.  */
+#ifndef LOAD_ADDRESS
+#error "Must define LOAD_ADDRESS"
+#endif
+
+int
+main (int argc, char *argv[])
+{
+  /* Used as backing storage for GDB to populate argv.  */
+  char *fake_argv[2];
+  
+  size_t obj_size;
+  void *load_addr = (void *) (size_t) LOAD_ADDRESS;
+  void *addr = load_elf (argv[1], &obj_size, load_addr);
+  int (*jit_function) ()
+      = (int (*) ()) load_symbol (addr, "jit_function_0001");
+
+  struct jit_code_entry *const entry
+      = (struct jit_code_entry *) calloc (1, sizeof (*entry));
+
+  entry->symfile_addr = (const char *) addr;
+  entry->symfile_size = obj_size;
+  entry->prev_entry = __jit_debug_descriptor.relevant_entry;
+  __jit_debug_descriptor.relevant_entry = entry;
+  __jit_debug_descriptor.first_entry = entry;
+  __jit_debug_descriptor.action_flag = JIT_REGISTER;
+  __jit_debug_register_code ();
+
+  jit_function (); /* first-call */
+
+  __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
+  __jit_debug_register_code ();
+
+  addr = load_elf (argv[1], &obj_size, addr);
+  jit_function = (int (*) ()) load_symbol (addr, "jit_function_0001");
+
+  entry->symfile_addr = (const char *) addr;
+  entry->symfile_size = obj_size;
+  __jit_debug_descriptor.relevant_entry = entry;
+  __jit_debug_descriptor.first_entry = entry;
+  __jit_debug_descriptor.action_flag = JIT_REGISTER;
+  __jit_debug_register_code ();
+
+  jit_function ();
+}
diff --git a/gdb/testsuite/gdb.base/jit-elf-reregister.exp b/gdb/testsuite/gdb.base/jit-elf-reregister.exp
new file mode 100644
index 0000000000..614b7b9795
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-reregister.exp
@@ -0,0 +1,78 @@
+# Copyright 2019 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/>.  */
+#
+# Tests global symbol lookup in case a symbol's name in the kernel
+# coincides with another in the main executable.
+
+if {[skip_shlib_tests]} {
+    untested "skipping shared library tests"
+    return -1
+}
+
+load_lib jit-elf-helpers.exp
+
+# The main code that loads and registers JIT objects.
+set main_basename "jit-elf-reregister"
+set main_srcfile ${srcdir}/${subdir}/${main_basename}.c
+set main_binfile [standard_output_file ${main_basename}]
+
+# The shared library that gets loaded as JIT objects.
+set jit_solib_basename jit-elf-solib
+set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
+
+# Compile one shared library to use as JIT object.
+set jit_solibs_target [compile_and_download_n_jit_so \
+		      $jit_solib_basename $jit_solib_srcfile 1 {debug}]
+if { $jit_solibs_target == -1 } {
+    return
+}
+
+proc test_inferior_initial {num} {
+    global jit_solibs_target main_srcfile main_binfile jit_solib_basename
+
+    gdb_test "inferior ${num}"
+
+    if { ![runto_main] } {
+	fail "can't run to main for the inferior ${num}"
+	return
+    }
+
+    # Poke desired values directly into inferior instead of using "set args"
+    # because "set args" does not work under gdbserver.
+    gdb_test_no_output "set var argc=1" "forging argc in inferior ${num}"
+    gdb_test_no_output "set var argv=fake_argv" "forging argv in inferior ${num}"
+    set jit_solib_target [lindex $jit_solibs_target 0]
+    gdb_test_no_output "set var argv\[1\]=\"${jit_solib_target}\"" \
+    "forging argv\[1\] in inferior ${num}"
+
+    gdb_breakpoint [gdb_get_line_number "first-call" $main_srcfile] {temporary}
+    gdb_continue_to_breakpoint "first-call in inferior ${num}"
+    gdb_breakpoint "jit_function_0001"
+    gdb_continue_to_breakpoint "hit before reload inferior ${num}" ".*$jit_solib_basename.*"
+}
+
+# Compile the main code (which loads the JIT objects).
+if { [compile_jit_main ${main_srcfile} ${main_binfile} {}] != 0 } {
+    return
+}
+
+clean_restart ${main_binfile}
+gdb_test "add-inferior -exec ${main_binfile}" "Added inferior 2.*" \
+"Add second inferior"
+test_inferior_initial 1
+test_inferior_initial 2
+gdb_continue_to_breakpoint "hit after reload inferior 2" ".*$jit_solib_basename.*"
+gdb_test "inferior 1" "Switching to inferior 1.*" "finishing another inferior"
+gdb_continue_to_breakpoint "hit after reload inferior 1" ".*$jit_solib_basename.*"
\ No newline at end of file
-- 
2.17.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928



More information about the Gdb-patches mailing list