[PATCH 2/2] Disable breakpoint locations in unloaded jit objects
Strasuns, Mihails
mihails.strasuns@intel.com
Tue Jun 30 12:35:19 GMT 2020
Ping.
> -----Original Message-----
> From: Mihails Strasuns <mihails.strasuns@intel.com>
> Sent: Tuesday, June 16, 2020 1:46 PM
> To: gdb-patches@sourceware.org
> Cc: Strasuns, Mihails <mihails.strasuns@intel.com>
> Subject: [PATCH 2/2] Disable breakpoint locations in unloaded jit objects
>
> 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