From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Wed, 22 Jul 2020 13:56:07 +0000 (+0200)
Subject: gdb/jit: apply some simplifications and assertions
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=8c1c720faa4fb1ddc0c89dd60979361a8e11268a;p=binutils-gdb.git

gdb/jit: apply some simplifications and assertions

Following patch "gdb/jit: split jit_objfile_data in two", there are some
simplifications we can make.  The invariants described there mean that
we can assume / assert some things instead of checking them using
conditionals.

If an instance of jiter_objfile_data exists for a given objfile, it's
because the required JIT interface symbols were found.  Therefore, in
~jiter_objfile_data, the `register_code` field can't be NULL.  It was
previously used to differentiate a jit_objfile_data object used for a
JITer vs a JITed.  We can remove that check.

If an instance of jiter_objfile_data exists for a given objfile, it's
because it's the sole JITer objfile in the scope of its program space
(jit_program_space_data::objfile points to it).  At the moment,
jit_breakpoint_re_set_internal won't create a second instance of
jiter_objfile_data for a given program space.  Therefore, it's not
necessary to check for `ps_data != NULL` in ~jiter_objfile_data: we know
a jit_program_space_data for that program space exists.  We also don't
need to check for `ps_data->objfile == this->objfile`, because we know
the objfile is the sole JITer in this program space.  Replace these two
conditions with assertions.

A pre-condition for calling the jit_read_descriptor function (which is
respected in the two call sites) is that the objfile `jiter` _is_ a
JITer - it already has a jiter_objfile_data attached to it.  When a
jiter_objfile_data exists, its `descriptor` field is necessarily set:
had the descriptor symbol not been found, jit_breakpoint_re_set_internal
would not have created the jiter_objfile_data.  Remove the check and
early return in jit_read_descriptor.  Access objfile's `jiter_data` field
directly instead of calling `get_jiter_objfile_data` (which creates the
jiter_objfile_data if it doesn't exist yet) and assert that the result
is not nullptr.

Finally, `jit_event_handler` is always passed a JITer objfile.  So, add
an assertion to ensure that.

gdb/ChangeLog:
2020-07-22  Simon Marchi  <simon.marchi@polymtl.ca>

	* jit.c (jiter_objfile_data::~jiter_objfile_data): Remove some
	checks.
	(jit_read_descriptor): Remove NULL check.
	(jit_event_handler): Add an assertion.
---

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 54b75aa212d..5d1a843d2b3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2020-07-22  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* jit.c (jiter_objfile_data::~jiter_objfile_data): Remove some
+	checks.
+	(jit_read_descriptor): Remove NULL check.
+	(jit_event_handler): Add an assertion.
+
 2020-07-22  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* jit.h (struct jit_objfile_data):  Split into...
diff --git a/gdb/jit.c b/gdb/jit.c
index 7c8bfcb51bb..9ac282ae534 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -267,20 +267,17 @@ static program_space_key<jit_program_space_data> jit_program_space_key;
 
 jiter_objfile_data::~jiter_objfile_data ()
 {
-  /* Free the data allocated in the jit_program_space_data slot.  */
-  if (this->register_code != NULL)
-    {
-      struct jit_program_space_data *ps_data;
+  jit_program_space_data *ps_data
+    = jit_program_space_key.get (this->objfile->pspace);
 
-      ps_data = jit_program_space_key.get (this->objfile->pspace);
-      if (ps_data != NULL && ps_data->objfile == this->objfile)
-	{
-	  ps_data->objfile = NULL;
-	  if (ps_data->jit_breakpoint != NULL)
-	    delete_breakpoint (ps_data->jit_breakpoint);
-	  ps_data->cached_code_address = 0;
-	}
-    }
+  gdb_assert (ps_data != nullptr);
+  gdb_assert (ps_data->objfile == this->objfile);
+
+  ps_data->objfile = NULL;
+  if (ps_data->jit_breakpoint != NULL)
+    delete_breakpoint (ps_data->jit_breakpoint);
+
+  ps_data->cached_code_address = 0;
 }
 
 /* Fetch the jiter_objfile_data associated with OBJF.  If no data exists
@@ -336,10 +333,8 @@ jit_read_descriptor (gdbarch *gdbarch,
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
   gdb_assert (jiter != nullptr);
-  jiter_objfile_data *objf_data = get_jiter_objfile_data (jiter);
-
-  if (objf_data->descriptor == NULL)
-    return false;
+  jiter_objfile_data *objf_data = jiter->jiter_data.get ();
+  gdb_assert (objf_data != nullptr);
 
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog,
@@ -1326,6 +1321,10 @@ jit_event_handler (gdbarch *gdbarch, objfile *jiter)
   CORE_ADDR entry_addr;
   struct objfile *objf;
 
+  /* If we get a JIT breakpoint event for this objfile, it is necessarily a
+     JITer.  */
+  gdb_assert (jiter->jiter_data != nullptr);
+
   /* Read the descriptor from remote memory.  */
   if (!jit_read_descriptor (gdbarch, &descriptor, jiter))
     return;