From 8c1c720faa4fb1ddc0c89dd60979361a8e11268a Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 22 Jul 2020 15:56:07 +0200 Subject: [PATCH] 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 * jit.c (jiter_objfile_data::~jiter_objfile_data): Remove some checks. (jit_read_descriptor): Remove NULL check. (jit_event_handler): Add an assertion. --- gdb/ChangeLog | 7 +++++++ gdb/jit.c | 33 ++++++++++++++++----------------- 2 files changed, 23 insertions(+), 17 deletions(-) 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 + + * 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 * 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_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; -- 2.30.2