FYI: fix PR 13431

[Tom Tromey <tromey at redhat dot com>]

I'm checking this in.

This fixes PR 13431.  The bug is that modifying the executable and
re-running can cause errors if the __jit_debug_descriptor moves.
This happens because gdb caches the address but does not always properly
clear the cache.

After trying a few options, I decided on the appended approach.  It
changes the JIT code to note the symbols corresponding to the JIT
objects; and then the per-inferior JIT data points to the objfile
holding the symbols.  This lets us handle the lifetime issues more
simply, because we can simply detect when the objfile goes away; and
secondarily this would fix any (hypothetical AFAIK) problems with
objfile relocation.

Built and regtested on x86-64 Fedora 15.
New test case included.

Tom

2012-02-01  Tom Tromey  <tromey@redhat.com>

	PR gdb/13431:
	* jit.c (struct jit_inferior_data): Rewrite.
	(struct jit_objfile_data): New.
	(get_jit_objfile_data): New function.
	(add_objfile_entry): Update.
	(jit_read_descriptor): Return int.  Replace descriptor_addr
	argument with inf_data.  Update.  Don't call error.
	(jit_breakpoint_re_set_internal): Reorder logic.  Update.  Look up
	descriptor here.
	(jit_inferior_init): Don't look up descriptor.  Don't call error.
	(jit_reset_inferior_data_and_breakpoints)
	(jit_inferior_created_observer): Remove.
	(jit_inferior_exit_hook): Update.
	(jit_executable_changed_observer): Remove.
	(jit_event_handler): Update.
	(free_objfile_data): Reset inferior data if needed.
	(_initialize_jit): Update.

2012-02-01  Tom Tromey  <tromey@redhat.com>

	* gdb.base/jit-simple.exp: New file.
	* gdb.base/jit-simple.c: New file.

>From c8d2c70155ab9bdbe56e178140dce3b73b90559c Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@redhat.com>
Date: Wed, 1 Feb 2012 13:12:19 -0700
Subject: [PATCH 2/2] fix PR 13431

---
 gdb/ChangeLog                         |   20 +++
 gdb/jit.c                             |  215 ++++++++++++++++++--------------
 gdb/testsuite/ChangeLog               |    5 +
 gdb/testsuite/gdb.base/jit-simple.c   |   37 ++++++
 gdb/testsuite/gdb.base/jit-simple.exp |   81 ++++++++++++
 5 files changed, 264 insertions(+), 94 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-simple.c
 create mode 100644 gdb/testsuite/gdb.base/jit-simple.exp

diff --git a/gdb/jit.c b/gdb/jit.c
index 44b080f..6920a82 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -231,14 +231,49 @@ jit_reader_unload_command (char *args, int from_tty)
   loaded_jit_reader = NULL;
 }
 
-/* Per-inferior structure recording the addresses in the inferior.  */
+/* Per-inferior structure recording which objfile has the JIT
+   symbols.  */
 
 struct jit_inferior_data
 {
-  CORE_ADDR breakpoint_addr;  /* &__jit_debug_register_code()  */
-  CORE_ADDR descriptor_addr;  /* &__jit_debug_descriptor  */
+  /* The objfile.  This is NULL if no objfile holds the JIT
+     symbols.  */
+
+  struct objfile *objfile;
+};
+
+/* Per-objfile structure recording the addresses in the inferior.  */
+
+struct jit_objfile_data
+{
+  /* Symbol for __jit_debug_register_code.  */
+  struct minimal_symbol *register_code;
+
+  /* Symbol for __jit_debug_descriptor.  */
+  struct minimal_symbol *descriptor;
+
+  /* Address of struct jit_code_entry in this objfile.  */
+  CORE_ADDR addr;
 };
 
+/* Fetch the jit_objfile_data associated with OBJF.  If no data exists
+   yet, make a new structure and attach it.  */
+
+static struct jit_objfile_data *
+get_jit_objfile_data (struct objfile *objf)
+{
+  struct jit_objfile_data *objf_data;
+
+  objf_data = objfile_data (objf, jit_objfile_data);
+  if (objf_data == NULL)
+    {
+      objf_data = XZALLOC (struct jit_objfile_data);
+      set_objfile_data (objf, jit_objfile_data, objf_data);
+    }
+
+  return objf_data;
+}
+
 /* Remember OBJFILE has been created for struct jit_code_entry located
    at inferior address ENTRY.  */
 
@@ -246,10 +281,10 @@ static void
 add_objfile_entry (struct objfile *objfile, CORE_ADDR entry)
 {
   CORE_ADDR *entry_addr_ptr;
+  struct jit_objfile_data *objf_data;
 
-  entry_addr_ptr = xmalloc (sizeof (CORE_ADDR));
-  *entry_addr_ptr = entry;
-  set_objfile_data (objfile, jit_objfile_data, entry_addr_ptr);
+  objf_data = get_jit_objfile_data (objfile);
+  objf_data->addr = entry;
 }
 
 /* Return jit_inferior_data for current inferior.  Allocate if not already
@@ -279,12 +314,12 @@ jit_inferior_data_cleanup (struct inferior *inf, void *arg)
 }
 
 /* Helper function for reading the global JIT descriptor from remote
-   memory.  */
+   memory.  Returns 1 if all went well, 0 otherwise.  */
 
-static void
+static int
 jit_read_descriptor (struct gdbarch *gdbarch,
 		     struct jit_descriptor *descriptor,
-		     CORE_ADDR descriptor_addr)
+		     struct jit_inferior_data *inf_data)
 {
   int err;
   struct type *ptr_type;
@@ -292,6 +327,18 @@ jit_read_descriptor (struct gdbarch *gdbarch,
   int desc_size;
   gdb_byte *desc_buf;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct jit_objfile_data *objf_data;
+
+  if (inf_data->objfile == NULL)
+    return 0;
+  objf_data = get_jit_objfile_data (inf_data->objfile);
+  if (objf_data->descriptor == NULL)
+    return 0;
+
+  if (jit_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"jit_read_descriptor, descriptor_addr = %s\n",
+			paddress (gdbarch, SYMBOL_VALUE_ADDRESS (objf_data->descriptor)));
 
   /* Figure out how big the descriptor is on the remote and how to read it.  */
   ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
@@ -300,9 +347,14 @@ jit_read_descriptor (struct gdbarch *gdbarch,
   desc_buf = alloca (desc_size);
 
   /* Read the descriptor.  */
-  err = target_read_memory (descriptor_addr, desc_buf, desc_size);
+  err = target_read_memory (SYMBOL_VALUE_ADDRESS (objf_data->descriptor),
+			    desc_buf, desc_size);
   if (err)
-    error (_("Unable to read JIT descriptor from remote memory!"));
+    {
+      printf_unfiltered (_("Unable to read JIT descriptor from "
+			   "remote memory\n"));
+      return 0;
+    }
 
   /* Fix the endianness to match the host.  */
   descriptor->version = extract_unsigned_integer (&desc_buf[0], 4, byte_order);
@@ -311,6 +363,8 @@ jit_read_descriptor (struct gdbarch *gdbarch,
   descriptor->relevant_entry = extract_typed_address (&desc_buf[8], ptr_type);
   descriptor->first_entry =
       extract_typed_address (&desc_buf[8 + ptr_size], ptr_type);
+
+  return 1;
 }
 
 /* Helper function for reading a JITed code entry from remote memory.  */
@@ -884,8 +938,10 @@ jit_find_objf_with_entry_addr (CORE_ADDR entry_addr)
 
   ALL_OBJFILES (objf)
     {
-      objf_entry_addr = (CORE_ADDR *) objfile_data (objf, jit_objfile_data);
-      if (objf_entry_addr != NULL && *objf_entry_addr == entry_addr)
+      struct jit_objfile_data *objf_data;
+
+      objf_data = objfile_data (objf, jit_objfile_data);
+      if (objf_data != NULL && objf_data->addr == entry_addr)
         return objf;
     }
   return NULL;
@@ -898,35 +954,39 @@ static int
 jit_breakpoint_re_set_internal (struct gdbarch *gdbarch,
 				struct jit_inferior_data *inf_data)
 {
-  if (inf_data->breakpoint_addr == 0)
-    {
-      struct minimal_symbol *reg_symbol;
-
-      /* Lookup the registration symbol.  If it is missing, then we assume
-	 we are not attached to a JIT.  */
-      reg_symbol = lookup_minimal_symbol (jit_break_name, NULL, NULL);
-      if (reg_symbol == NULL)
-	return 1;
-      inf_data->breakpoint_addr = SYMBOL_VALUE_ADDRESS (reg_symbol);
-      if (inf_data->breakpoint_addr == 0)
-	return 2;
-
-      /* If we have not read the jit descriptor yet (e.g. because the JITer
-	 itself is in a shared library which just got loaded), do so now.  */
-      if (inf_data->descriptor_addr == 0)
-	jit_inferior_init (gdbarch);
-    }
-  else
+  struct minimal_symbol *reg_symbol, *desc_symbol;
+  struct objfile *objf;
+  struct jit_objfile_data *objf_data;
+
+  if (inf_data->objfile != NULL)
     return 0;
 
+  /* Lookup the registration symbol.  If it is missing, then we assume
+     we are not attached to a JIT.  */
+  reg_symbol = lookup_minimal_symbol_and_objfile (jit_break_name, &objf);
+  if (reg_symbol == NULL || SYMBOL_VALUE_ADDRESS (reg_symbol) == 0)
+    return 1;
+
+  desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, objf);
+  if (desc_symbol == NULL || SYMBOL_VALUE_ADDRESS (desc_symbol) == 0)
+    return 1;
+
+  objf_data = get_jit_objfile_data (objf);
+  objf_data->register_code = reg_symbol;
+  objf_data->descriptor = desc_symbol;
+
+  inf_data->objfile = objf;
+
+  jit_inferior_init (gdbarch);
+
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
 			"jit_breakpoint_re_set_internal, "
 			"breakpoint_addr = %s\n",
-			paddress (gdbarch, inf_data->breakpoint_addr));
+			paddress (gdbarch, SYMBOL_VALUE_ADDRESS (reg_symbol)));
 
   /* Put a breakpoint in the registration symbol.  */
-  create_jit_event_breakpoint (gdbarch, inf_data->breakpoint_addr);
+  create_jit_event_breakpoint (gdbarch, SYMBOL_VALUE_ADDRESS (reg_symbol));
 
   return 0;
 }
@@ -1176,6 +1236,7 @@ jit_inferior_init (struct gdbarch *gdbarch)
   struct jit_code_entry cur_entry;
   struct jit_inferior_data *inf_data;
   CORE_ADDR cur_entry_addr;
+  struct jit_objfile_data *objf_data;
 
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog, "jit_inferior_init\n");
@@ -1186,33 +1247,19 @@ jit_inferior_init (struct gdbarch *gdbarch)
   if (jit_breakpoint_re_set_internal (gdbarch, inf_data) != 0)
     return;
 
-  if (inf_data->descriptor_addr == 0)
-    {
-      struct minimal_symbol *desc_symbol;
-
-      /* Lookup the descriptor symbol and cache the addr.  If it is
-	 missing, we assume we are not attached to a JIT and return early.  */
-      desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, NULL);
-      if (desc_symbol == NULL)
-	return;
-
-      inf_data->descriptor_addr = SYMBOL_VALUE_ADDRESS (desc_symbol);
-      if (inf_data->descriptor_addr == 0)
-	return;
-    }
-
-  if (jit_debug)
-    fprintf_unfiltered (gdb_stdlog,
-			"jit_inferior_init, descriptor_addr = %s\n",
-			paddress (gdbarch, inf_data->descriptor_addr));
-
   /* Read the descriptor so we can check the version number and load
      any already JITed functions.  */
-  jit_read_descriptor (gdbarch, &descriptor, inf_data->descriptor_addr);
+  if (!jit_read_descriptor (gdbarch, &descriptor, inf_data))
+    return;
 
   /* Check that the version number agrees with that we support.  */
   if (descriptor.version != 1)
-    error (_("Unsupported JIT protocol version in descriptor!"));
+    {
+      printf_unfiltered (_("Unsupported JIT protocol version %ld "
+			   "in descriptor (expected 1)\n"),
+			 (long) descriptor.version);
+      return;
+    }
 
   /* If we've attached to a running program, we need to check the descriptor
      to register any functions that were already generated.  */
@@ -1249,33 +1296,6 @@ jit_breakpoint_re_set (void)
 				  get_jit_inferior_data ());
 }
 
-/* Reset inferior_data, so sybols will be looked up again, and jit_breakpoint
-   will be reset.  */
-
-static void
-jit_reset_inferior_data_and_breakpoints (void)
-{
-  struct jit_inferior_data *inf_data;
-
-  /* Force jit_inferior_init to re-lookup of jit symbol addresses.  */
-  inf_data = get_jit_inferior_data ();
-  inf_data->breakpoint_addr = 0;
-  inf_data->descriptor_addr = 0;
-
-  /* Remove any existing JIT breakpoint(s).  */
-  remove_jit_event_breakpoints ();
-
-  jit_inferior_init (target_gdbarch);
-}
-
-/* Wrapper to match the observer function pointer prototype.  */
-
-static void
-jit_inferior_created_observer (struct target_ops *objfile, int from_tty)
-{
-  jit_reset_inferior_data_and_breakpoints ();
-}
-
 /* This function cleans up any code entries left over when the
    inferior exits.  We get left over code when the inferior exits
    without unregistering its code, for example when it crashes.  */
@@ -1287,14 +1307,13 @@ jit_inferior_exit_hook (struct inferior *inf)
   struct objfile *temp;
 
   ALL_OBJFILES_SAFE (objf, temp)
-    if (objfile_data (objf, jit_objfile_data) != NULL)
-      jit_unregister_code (objf);
-}
+    {
+      struct jit_objfile_data *objf_data = objfile_data (objf,
+							 jit_objfile_data);
 
-static void
-jit_executable_changed_observer (void)
-{
-  jit_reset_inferior_data_and_breakpoints ();
+      if (objf_data != NULL && objf_data->addr != 0)
+	jit_unregister_code (objf);
+    }
 }
 
 void
@@ -1306,8 +1325,8 @@ jit_event_handler (struct gdbarch *gdbarch)
   struct objfile *objf;
 
   /* Read the descriptor from remote memory.  */
-  jit_read_descriptor (gdbarch, &descriptor,
-		       get_jit_inferior_data ()->descriptor_addr);
+  if (!jit_read_descriptor (gdbarch, &descriptor, get_jit_inferior_data ()))
+    return;
   entry_addr = descriptor.relevant_entry;
 
   /* Do the corresponding action.  */
@@ -1340,6 +1359,16 @@ jit_event_handler (struct gdbarch *gdbarch)
 static void
 free_objfile_data (struct objfile *objfile, void *data)
 {
+  struct jit_objfile_data *objf_data = data;
+
+  if (objf_data->register_code != NULL)
+    {
+      struct jit_inferior_data *inf_data = get_jit_inferior_data ();
+
+      if (inf_data->objfile == objfile)
+	inf_data->objfile = NULL;
+    }
+
   xfree (data);
 }
 
@@ -1373,9 +1402,7 @@ _initialize_jit (void)
 			    show_jit_debug,
 			    &setdebuglist, &showdebuglist);
 
-  observer_attach_inferior_created (jit_inferior_created_observer);
   observer_attach_inferior_exit (jit_inferior_exit_hook);
-  observer_attach_executable_changed (jit_executable_changed_observer);
   jit_objfile_data =
     register_objfile_data_with_cleanup (NULL, free_objfile_data);
   jit_inferior_data =
diff --git a/gdb/testsuite/gdb.base/jit-simple.c b/gdb/testsuite/gdb.base/jit-simple.c
new file mode 100644
index 0000000..3893c3d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-simple.c
@@ -0,0 +1,37 @@
+/* Simple program 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()
+{
+}
+
+int main()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/jit-simple.exp b/gdb/testsuite/gdb.base/jit-simple.exp
new file mode 100644
index 0000000..d1be1a7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-simple.exp
@@ -0,0 +1,81 @@
+# Copyright 2012 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/>.
+
+if {[skip_shlib_tests]} {
+    untested jit-simple.exp
+    return -1
+}
+
+if {[get_compiler_info not-used]} {
+    warning "Could not get compiler info"
+    untested jit-simple.exp
+    return 1
+}
+
+#
+# test running programs
+#
+
+set testfile jit-simple
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested jit-simple.exp
+    return -1
+}
+
+# A helper for jit_test_reread that invokes gdb_run_cmd.
+proc jit_run {msg} {
+    global decimal gdb_prompt
+
+    gdb_run_cmd
+    gdb_expect {
+	-re "Inferior .* exited.*$gdb_prompt $" {
+	    pass $msg
+	}
+	-re ".*$gdb_prompt $" {
+	    fail $msg
+	}
+    }
+}
+
+# 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
+
+    clean_restart $testfile
+
+    # jit_run "initial run"
+    runto_main
+
+    gdb_test "print &__jit_debug_descriptor" "= .*" "blah 1"
+
+    gdb_rename_execfile $binfile ${binfile}x
+
+    if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DSPACER}] != "" } {
+	fail "recompile $srcfile"
+    } else {
+	pass "recompile $srcfile"
+
+	# jit_run "second run"
+
+	runto_main
+	gdb_test "print &__jit_debug_descriptor" "= .*" "blah 1"
+    }
+}
+
+jit_test_reread
-- 
1.7.6.5