[v6,2/3] Disable breakpoint locations in unloaded jit objects

Message ID 20201210192807.30372-3-mihails.strasuns@intel.com
State New
Headers show
Series
  • Disable breakpoint locations in unloaded jit objects
Related show

Commit Message

Lancelot SIX via Gdb-patches Dec. 10, 2020, 7:28 p.m.
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                              | 30 +++++--
 gdb/jit.c                                     | 31 +++++--
 gdb/observable.c                              |  1 +
 gdb/observable.h                              |  3 +
 gdb/testsuite/gdb.base/jit-elf-reregister.c   | 65 +++++++++++++++
 gdb/testsuite/gdb.base/jit-elf-reregister.exp | 81 +++++++++++++++++++
 6 files changed, 196 insertions(+), 15 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-elf-reregister.c
 create mode 100644 gdb/testsuite/gdb.base/jit-elf-reregister.exp

-- 
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

Comments

Lancelot SIX via Gdb-patches Jan. 8, 2021, 8:03 p.m. | #1
On 2020-12-10 2:28 p.m., Mihails Strasuns via Gdb-patches wrote:
> 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>


Make sure to update those dates before pushing.  When I prepare ChangeLog
entries, I usually omit this line, since the date will likely change
anyway, and just add it to the ChangeLog file when pushing the patch.  In
fact, I use that little script:

  https://github.com/simark/gnu-changelog-tools/tree/master/changelog-git-amend

> 

> 	* 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.


No need to put that much details in the ChangeLog, just use "New." if the file
is new.  All "human-readable" explanation should be put in the actual commit log.

> ---

>  gdb/breakpoint.c                              | 30 +++++--

>  gdb/jit.c                                     | 31 +++++--

>  gdb/observable.c                              |  1 +

>  gdb/observable.h                              |  3 +

>  gdb/testsuite/gdb.base/jit-elf-reregister.c   | 65 +++++++++++++++

>  gdb/testsuite/gdb.base/jit-elf-reregister.exp | 81 +++++++++++++++++++

>  6 files changed, 196 insertions(+), 15 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 881686ff7a0..84559069b6e 100644

> --- a/gdb/breakpoint.c

> +++ b/gdb/breakpoint.c

> @@ -7631,22 +7631,25 @@ 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.  */

> +/* Disable any breakpoints and tracepoints that are in OBJFILE.

> +   Only apply to enabled breakpoints, disabled one can just stay disabled.

> +   Used for both shared library and JIT module unloading.  */

>  

>  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;

>  

> +  if (objfile == nullptr)

> +    return;


Can you remind me when objfile is nullptr here?  It would be good to have a
comment to say when that happens.  Because intuitively, I would say that it
makes no sense to call this function with a nullptr argument (and I'd turn
that if into a gdb_assert), but maybe there's a good reason to.

> +

>    ALL_BP_LOCATIONS (loc, locp_tmp)

>    {

>      /* 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

> @@ -7654,7 +7657,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

> @@ -7670,13 +7673,25 @@ 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;

>        }

>    }

>  }

>  

> +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);

> +}


Please add a comment for these functions.

  /* solib_unloaded observer.  */

and

/* jit_object_unloaded observer.  */

would be enough.

> +

>  /* Disable any breakpoints and tracepoints in OBJFILE upon

>     notification of free_objfile.  Only apply to enabled breakpoints,

>     disabled ones can just stay disabled.  */

> @@ -15683,6 +15698,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 e36b3c3b68a..34ad3f34f72 100644

> --- a/gdb/jit.c

> +++ b/gdb/jit.c

> @@ -779,6 +779,28 @@ 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 = jit_find_objf_with_entry_addr (entry_addr);

> +  if (jit_debug)

> +    fprintf_unfiltered (gdb_stdlog, "%s: objfile = %s\n",

> +			__func__,

> +			host_address_to_string (objfile));

> +

> +  if (objfile == NULL)

> +    printf_unfiltered (_ ("Unable to find JITed code "


We don't use a space for the "_" macro call, don't ask me why :).

> +			  "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.  */

>  

> @@ -1214,14 +1236,7 @@ jit_event_handler (gdbarch *gdbarch, objfile *jiter)

>  

>      case JIT_UNREGISTER:

>        {

> -	objfile *jited = jit_find_objf_with_entry_addr (entry_addr);

> -	if (jited == nullptr)

> -	  printf_unfiltered (_("Unable to find JITed code "

> -			       "entry at address: %s\n"),

> -			     paddress (gdbarch, entry_addr));

> -	else

> -	  jited->unlink ();

> -

> +	jit_unregister_code (gdbarch, entry_addr);

>  	break;

>        }

>  

> diff --git a/gdb/observable.c b/gdb/observable.c

> index 231f955fa26..f329461469d 100644

> --- a/gdb/observable.c

> +++ b/gdb/observable.c

> @@ -47,6 +47,7 @@ DEFINE_OBSERVABLE (inferior_execd);

>  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 1dce6746ff3..f1e733e6682 100644

> --- a/gdb/observable.h

> +++ b/gdb/observable.h

> @@ -114,6 +114,9 @@ extern observable<struct so_list */* solib */> solib_loaded;

>     been unloaded yet, and thus are still available.  */

>  extern observable<struct so_list */* solib */> solib_unloaded;

>  

> +/* The JIT module represented by OBJFILE has been unregistered.  */

> +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 00000000000..37e8277babe

> --- /dev/null

> +++ b/gdb/testsuite/gdb.base/jit-elf-reregister.c

> @@ -0,0 +1,65 @@

> +/* 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[])

> +{

> +  size_t obj_size;

> +  void *load_addr = (void *) (size_t) LOAD_ADDRESS;

> +  printf("HERE %s\n", argv[1]);

> +  void *addr = load_elf (argv[1], &obj_size, load_addr);

> +  printf("HEAR %p\n", addr);


You can probably remove these printfs.  If not, add a space before
the parenthesis.

> +  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 00000000000..6735ce2a191

> --- /dev/null

> +++ b/gdb/testsuite/gdb.base/jit-elf-reregister.exp

> @@ -0,0 +1,81 @@

> +# 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/>.  */

> +# 

> +# Checks that breakpoints keep working if a JIT module is repeatedly

> +# being registered and unregistered at the same base address.

> +

> +if {[use_gdb_stub]} {

> +    return 0

> +}

> +

> +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

> +}

> +

> +# Prepare two inferiors using the same JIT module.  Hit an initial breakpoint

> +# in a JIT-loaded function as a sanity check that basic functionality is working.

> +proc test_inferior_initial {num} {

> +    global jit_solibs_target main_srcfile main_binfile jit_solib_basename

> +

> +    gdb_test "inferior ${num}"

> +    gdb_test_no_output "set args $jit_solibs_target" \

> +	"passing solib list as an argument ($num)"


Don't finish the test name with parenthesis, as the trailing parenthesis
is not considered part of the test name:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages

Also, say "pass" instead of "passing".

I suggest that you define this proc with "proc_with_prefix", that will
make it easier to map test names to the code.  And at the top-level, use
foreach_with_prefix to de-duplicate the two runs.  That avoids having to
add "for inferior $num" everywhere.  So:

proc_with_prefix test_inferior_initial {num} {
    global jit_solibs_target main_srcfile main_binfile jit_solib_basename

    gdb_test "inferior ${num}"
    gdb_test_no_output "set args $jit_solibs_target" \
	"pass solib list as an argument"

    if { ![runto_main] } {
	fail "can't run to main"
	return
    }

    gdb_breakpoint [gdb_get_line_number "first-call" $main_srcfile] {temporary}
    gdb_continue_to_breakpoint "first-call"
    gdb_breakpoint "jit_function_0001"
    gdb_continue_to_breakpoint "hit before reload" ".*$jit_solib_basename.*"
}

...

foreach_with_prefix inferior {1 2} {
    test_inferior_initial $inferior
}


And instead of having the intermediary breakpoint at "first-call", you could
set a pending breakpoint on jit_function_0001:

    gdb_breakpoint "jit_function_0001" allow-pending
    gdb_continue_to_breakpoint "hit before reload" ".*$jit_solib_basename.*"

> +clean_restart ${main_binfile}

> +gdb_test "add-inferior -exec ${main_binfile}" "Added inferior 2.*" \

> +    "Add a second inferior"

> +test_inferior_initial 1

> +test_inferior_initial 2

> +# $main_srcfile is written so that the JIT module gets reloaded after

> +# `jit_function_0001` finishes - and the function is called again. Test that

> +# the breakpoint still remains valid (for both inferiors):

> +gdb_continue_to_breakpoint "hit after reload inferior 2" ".*$jit_solib_basename.*"

> +gdb_test "inferior 1" "Switching to inferior 1.*" "finishing another inferior"


This can become just

  gdb_test "inferior 1" "Switching to inferior 1.*"

with my suggestion above, as the test names in test_inferior_initial will be
scoped.

Simon

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 881686ff7a0..84559069b6e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7631,22 +7631,25 @@  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.  */
+/* Disable any breakpoints and tracepoints that are in OBJFILE.
+   Only apply to enabled breakpoints, disabled one can just stay disabled.
+   Used for both shared library and JIT module unloading.  */
 
 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;
 
+  if (objfile == nullptr)
+    return;
+
   ALL_BP_LOCATIONS (loc, locp_tmp)
   {
     /* 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
@@ -7654,7 +7657,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
@@ -7670,13 +7673,25 @@  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;
       }
   }
 }
 
+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.  */
@@ -15683,6 +15698,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 e36b3c3b68a..34ad3f34f72 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -779,6 +779,28 @@  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 = jit_find_objf_with_entry_addr (entry_addr);
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog, "%s: objfile = %s\n",
+			__func__,
+			host_address_to_string (objfile));
+
+  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.  */
 
@@ -1214,14 +1236,7 @@  jit_event_handler (gdbarch *gdbarch, objfile *jiter)
 
     case JIT_UNREGISTER:
       {
-	objfile *jited = jit_find_objf_with_entry_addr (entry_addr);
-	if (jited == nullptr)
-	  printf_unfiltered (_("Unable to find JITed code "
-			       "entry at address: %s\n"),
-			     paddress (gdbarch, entry_addr));
-	else
-	  jited->unlink ();
-
+	jit_unregister_code (gdbarch, entry_addr);
 	break;
       }
 
diff --git a/gdb/observable.c b/gdb/observable.c
index 231f955fa26..f329461469d 100644
--- a/gdb/observable.c
+++ b/gdb/observable.c
@@ -47,6 +47,7 @@  DEFINE_OBSERVABLE (inferior_execd);
 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 1dce6746ff3..f1e733e6682 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -114,6 +114,9 @@  extern observable<struct so_list */* solib */> solib_loaded;
    been unloaded yet, and thus are still available.  */
 extern observable<struct so_list */* solib */> solib_unloaded;
 
+/* The JIT module represented by OBJFILE has been unregistered.  */
+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 00000000000..37e8277babe
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-reregister.c
@@ -0,0 +1,65 @@ 
+/* 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[])
+{
+  size_t obj_size;
+  void *load_addr = (void *) (size_t) LOAD_ADDRESS;
+  printf("HERE %s\n", argv[1]);
+  void *addr = load_elf (argv[1], &obj_size, load_addr);
+  printf("HEAR %p\n", 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 00000000000..6735ce2a191
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-reregister.exp
@@ -0,0 +1,81 @@ 
+# 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/>.  */
+# 
+# Checks that breakpoints keep working if a JIT module is repeatedly
+# being registered and unregistered at the same base address.
+
+if {[use_gdb_stub]} {
+    return 0
+}
+
+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
+}
+
+# Prepare two inferiors using the same JIT module.  Hit an initial breakpoint
+# in a JIT-loaded function as a sanity check that basic functionality is working.
+proc test_inferior_initial {num} {
+    global jit_solibs_target main_srcfile main_binfile jit_solib_basename
+
+    gdb_test "inferior ${num}"
+    gdb_test_no_output "set args $jit_solibs_target" \
+	"passing solib list as an argument ($num)"
+
+    if { ![runto_main] } {
+	fail "can't run to main for the inferior ${num}"
+	return
+    }
+
+    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 a second inferior"
+test_inferior_initial 1
+test_inferior_initial 2
+# $main_srcfile is written so that the JIT module gets reloaded after
+# `jit_function_0001` finishes - and the function is called again. Test that
+# the breakpoint still remains valid (for both inferiors):
+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.*"