This changes GDB to use frame_info_ptr instead of frame_info *
The substitution was done with multiple sequential `sed` commands:
sed 's/^struct frame_info;/class frame_info_ptr;/'
sed 's/struct frame_info \*/frame_info_ptr /g' - which left some
issues in a few files, that were manually fixed.
sed 's/\<frame_info \*/frame_info_ptr /g'
sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace
problems.
The changed files were then manually checked and some 'sed' changes
undone, some constructors and some gets were added, according to what
made sense, and what Tromey originally did
Co-Authored-By: Bruno Larsen <blarsen@redhat.com>
Approved-by: Tom Tomey <tom@tromey.com>
gdbarch implements its own registry-like approach. This patch changes
it to instead use registry.h. It's a rather large patch but largely
uninteresting -- it's mostly a straightforward conversion from the old
approach to the new one.
The main benefit of this change is that it introduces type safety to
the gdbarch registry. It also removes a bunch of code.
One possible drawback is that, previously, the gdbarch registry
differentiated between pre- and post-initialization setup. This
doesn't seem very important to me, though.
This changes struct objfile to use a gdb_bfd_ref_ptr. In addition to
removing some manual memory management, this fixes a use-after-free
that was introduced by the registry rewrite series. The issue there
was that, in some cases, registry shutdown could refer to memory that
had already been freed. This help fix the bug by delaying the
destruction of the BFD reference (and thus the per-bfd object) until
after the registry has been shut down.
Replace with calls to blockvector::blocks, and the appropriate method
call on the returned array_view.
Change-Id: I04d1f39603e4d4c21c96822421431d9a029d8ddd
This changes jit.c to use new and delete, rather than XCNEW. This
simplifies the code a little. This was useful for another patch I'm
working on, and I thought it would make sense to send it separately.
Regression tested on x86-64 Fedora 34.
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
This commit updates the output of 'maint info jit' to print not just
the jit_code_entry address, but also the symfile address, and the
symfile size.
The new information could be obtained by looking into target memory at
the contents of the jit_code_entry, but, by storing this information
within gdb at the time the jit object is loaded, it is now possible to
check if the jit_code_entry has been modified in target memory behind
gdb's back.
Additionally, the symfile address is the same address that is now used
in the objfile names after commit 4a620b7e.
One test that relies on the output of 'maint info jit' was updated to
allow for the new output format.
Add a getter and a setter for a symbol's type. Remove the corresponding
macro and adjust all callers.
Change-Id: Ie1a137744c5bfe1df4d4f9ae5541c5299577c8de
Add a getter and a setter for a symbol's domain. Remove the
corresponding macro and adjust all callers.
Change-Id: I54465b50ac89739c663859a726aef8cdc6e4b8f3
Add a getter and a setter for a symbol's aclass index. Remove the
corresponding macro and adjust all callers.
Change-Id: Ie8c8d732624cfadb714aba5ddafa3d29409b3d39
Add a getter and a setter for a symtab's linetable. Remove the
corresponding macro and adjust all callers.
Change-Id: I159183fc0ccd8e18ab937b3c2f09ef2244ec6e9c
Add a getter and a setter for a compunit_symtab's blockvector. Remove
the corresponding macro and adjust all callers.
Change-Id: I99484c6619dcbbea7c5d89c72aa660316ca62f64
Add a getter and a setter for a compunit_symtab's dirname. Remove the
corresponding macro and adjust all callers.
Change-Id: If2f39b295fd26822586485e04a8b8b5aa5cc9b2e
I think that most remaining uses of COMPUNIT_FILETABS intend to get the
primary filetab of the compunit_symtab specifically (and not to iterate
over all filetabs, for example, those cases would use compunit_filetabs,
which has been converted to compunit_symtab::filetabs), so replace mosts
uses with compunit_symtab::primary_filetab.
In jit.c, function finalize_symtab, we can save the symtab object
returned by allocate_symtab and use it, it makes things simpler.
Change-Id: I4e51d6d4b40759de8768b61292e5e13c8eae2e38
This commit includes the JIT object's symfile address in the names of
objfiles created by JIT reader API (e.g., << JIT compiled code at
0x7ffd8a0c77a0 >>). This allows one to at least differentiate one from
another.
The address is the one that the debugged program has put in
jit_code_entry::symfile_addr, and that the JIT reader's read function
receives. As we can see in gdb.base/jit-reader-host.c and
gdb.base/jit-reader.c, that may not be the actual value of where the
JIT-ed code is. But it is a value chosen by the author of the JIT
engine and the JIT reader, so including this value in the objfile name
may help them correlate the JIT objfiles created by with their logs /
data structures.
To access this field, we need to pass down a reference to the
jit_code_entry. So make jit_dbg_reader_data a structure (instead of an
alias for a CORE_ADDR) that includes the address of the code entry in
the inferior's address space (the previous meaning of
jit_dbg_reader_data) plus a reference to the jit_code_entry as read into
GDB's address space. And while at it, pass down the gdbarch, so that we
don't have to call target_gdbarch.
Co-Authored-By: Jan Vrany <jan.vrany@labware.com>
Change-Id: Ib26c4d1bd8de503d651aff89ad2e500cb312afa5
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
GDB doesn't handle well the case of an inferior using the JIT interface
to register JIT-ed objfiles and forking. If an inferior registers a
code object using the JIT interface and then forks, the child process
conceptually has the same code object loaded, so GDB should look it up
and learn about it (it currently doesn't).
To achieve this, I think it would make sense to have the
inferior_created observable called when an inferior is created due to a
fork in follow_fork_inferior. The inferior_created observable is
currently called both after starting a new inferior and after attaching
to an inferior, allowing various sub-components to learn about that new
executing inferior. We can see handling a fork child just like
attaching to it, so any work done when attaching should also be done in
the case of a fork child.
Instead of just calling the inferior_created observable, this patch
makes follow_fork_inferior call the whole post_create_inferior function.
This way, the attach and follow-fork code code paths are more alike.
Given that post_create_inferior calls solib_create_inferior_hook,
follow_fork_inferior doesn't need to do it itself, so those calls to
solib_create_inferior_hook are removed.
One question you may have: why not just call post_create_inferior at the
places where solib_create_inferior_hook is currently called, instead of
after target_follow_fork?
- there's something fishy for the second solib_create_inferior_hook
call site: at this point we have switched the current program space
to the child's, but not the current inferior nor the current thread.
So solib_create_inferior_hook (and everything under, including
check_for_thread_db, for example) is called with inferior 1 as the
current inferior and inferior 2's program space as the current
program space. I think that's wrong, because at this point we are
setting up inferior 2, and all that code relies on the current
inferior. We could just add a switch_to_thread call before it to
make inferior 2 the current one, but there are other problems (see
below).
- solib_create_inferior_hook is currently not called on the
`follow_child && detach_fork` path. I think we need to call it,
because we still get a new inferior in that case (even though we
detach the parent). If we only call post_create_inferior where
solib_create_inferior_hook used to be called, then the JIT
subcomponent doesn't get informed about the new inferior, and that
introduces a failure in the new gdb.base/jit-elf-fork.exp test.
- if we try to put the post_create_inferior just after the
switch_to_thread that was originally at line 662, or just before the
call to target_follow_fork, we introduce a subtle failure in
gdb.threads/fork-thread-pending.exp. What happens then is that
libthread_db gets loaded (somewhere under post_create_inferior)
before the linux-nat target learns about the LWPs (which happens in
linux_nat_target::follow_fork). As a result, the ALL_LWPS loop in
try_thread_db_load_1 doesn't see the child LWP, and the thread-db
target doesn't have the chance to fill in thread_info::priv. A bit
later, when the test does "info threads", and
thread_db_target::pid_to_str is called, the thread-db target doesn't
recognize the thread as one of its own, and delegates the request to
the target below. Because the pid_to_str output is not the expected
one, the test fails.
This tells me that we need to call the process target's follow_fork
first, to make the process target create the necessary LWP and thread
structures. Then, we can call post_create_inferior to let the other
components of GDB do their thing.
But then you may ask: check_for_thread_db is already called today,
somewhere under solib_create_inferior_hook, and that is before
target_follow_fork, why don't we see this ordering problem!? Well,
because of the first bullet point: when check_for_thread_db /
thread_db_load are called, the current inferior is (erroneously)
inferior 1, the parent. Because libthread_db is already loaded for
the parent, thread_db_load early returns. check_for_thread_db later
gets called by linux_nat_target::follow_fork. At this point, the
current inferior is the correct one and the child's LWP exists, so
all is well.
Since we now call post_create_inferior after target_follow_fork, which
calls the inferior_created observable, which calls check_for_thread_db,
I don't think linux_nat_target needs to explicitly call
check_for_thread_db itself, so that is removed.
In terms of testing, this patch adds a new gdb.base/jit-elf-fork.exp
test. It makes an inferior register a JIT code object and then fork.
It then verifies that whatever the detach-on-fork and follow-fork-child
parameters are, GDB knows about the JIT code object in all the inferiors
that survive the fork. It verifies that the inferiors can unload that
code object.
There isn't currently a way to get visibility into GDB's idea of the JIT
code objects for each inferior. For the purpose of this test, add the
"maintenance info jit" command. There isn't much we can print about the
JIT code objects except their load address. So the output looks a bit
bare, but it's good enough for the test.
gdb/ChangeLog:
* NEWS: Mention "maint info jit" command.
* infrun.c (follow_fork_inferior): Don't call
solib_create_inferior_hook, call post_create_inferior if a new
inferior was created.
* jit.c (maint_info_jit_cmd): New.
(_initialize_jit): Register new command.
* linux-nat.c (linux_nat_target::follow_fork): Don't call
check_for_thread_db.
* linux-nat.h (check_for_thread_db): Remove declaration.
* linux-thread-db.c (check_thread_signals): Make static.
gdb/doc/ChangeLog:
* gdb.texinfo (Maintenance Commands): Mention "maint info jit".
gdb/testsuite/ChangeLog:
* gdb.base/jit-elf-fork-main.c: New test.
* gdb.base/jit-elf-fork-solib.c: New test.
* gdb.base/jit-elf-fork.exp: New test.
Change-Id: I9a192e55b8a451c00e88100669283fc9ca60de5c
I wrote this while debugging a problem where the expected unwinder for a
frame wasn't used. It adds messages to show which unwinders are
considered for a frame, why they are not selected (if an exception is
thrown), and finally which unwinder is selected in the end.
To be able to show a meaningful, human-readable name for the unwinders,
add a "name" field to struct frame_unwind, and update all instances to
include a name.
Here's an example of the output:
[frame] frame_unwind_find_by_frame: this_frame=0
[frame] frame_unwind_try_unwinder: trying unwinder "dummy"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "dwarf2 tailcall"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "inline"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "jit"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "python"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "amd64 epilogue"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "i386 epilogue"
[frame] frame_unwind_try_unwinder: no
[frame] frame_unwind_try_unwinder: trying unwinder "dwarf2"
[frame] frame_unwind_try_unwinder: yes
gdb/ChangeLog:
* frame-unwind.h (struct frame_unwind) <name>: New. Update
instances everywhere to include this field.
* frame-unwind.c (frame_unwind_try_unwinder,
frame_unwind_find_by_frame): Add debug messages.
Change-Id: I813f17777422425f0d08b22499817b23922e8ddb
Add the breakpoint::locations method, which returns a range that can be
used to iterate over a breakpoint's locations. This shortens
for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
into
for (bp_location *loc : b->locations ())
Change all the places that I found that could use it.
gdb/ChangeLog:
* breakpoint.h (bp_locations_range): New.
(struct breakpoint) <locations>: New. Use where possible.
Change-Id: I1ba2f7d93d57e544e1f8609124587dcf2e1da037
Consider a minimal test-case test.c:
...
int main (void) { return 0; }
...
which we can compile into llvm byte code using clang:
...
$ clang -g -S -emit-llvm --target=x86_64-unknown-unknown-elf test.c
...
and then run using lli, which uses the llvm jit:
...
$ lli test.ll
...
If we run this under gdb, we run into an assert:
...
$ gdb -q -batch -ex run --args /usr/bin/lli test.ll
Dwarf Error: Cannot not find DIE at 0x18a936e7 \
[from module libLLVM.so.10-10.0.1-lp152.30.4.x86_64.debug]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
src/gdb/jit.c:1178: internal-error: \
void jit_event_handler(gdbarch*, objfile*): \
Assertion `jiter->jiter_data != nullptr' failed.
...
This is caused by the following.
When running jit_breakpoint_re_set_internal, we first handle
libLLVM.so.10.debug, and set a jit breakpoint.
Next we handle libLLVM.so.10:
...
(gdb) p the_objfile.original_name
$42 = 0x2494170 "libLLVM.so.10"
...
but the minimal symbols we find are from libLLVM.so.10.debug:
...
(gdb) p reg_symbol.objfile.original_name
$43 = 0x38e7c50 "libLLVM.so.10-10.0.1-lp152.30.4.x86_64.debug"
(gdb) p desc_symbol.objfile.original_name
$44 = 0x38e7c50 "libLLVM.so.10-10.0.1-lp152.30.4.x86_64.debug"
...
and consequently, the objf_data is the one from libLLVM.so.10.debug:
...
jiter_objfile_data *objf_data
= get_jiter_objfile_data (reg_symbol.objfile);
...
and so we hit this:
...
if (objf_data->cached_code_address == addr)
continue;
...
and no second jit breakpoint is inserted.
Subsequently, the jit breakpoint is triggered and handled, but when finding
the symbol for the breakpoint address we get:
...
(gdb) p jit_bp_sym.objfile.original_name
$52 = 0x2494170 "libLLVM.so.10"
...
The assert 'jiter->jiter_data != nullptr' triggers because it checks
libLLVM.so.10 while the one with jiter_data setup is libLLVM.so.10.debug.
This fixes the assert:
...
jiter_objfile_data *objf_data
- = get_jiter_objfile_data (reg_symbol.objfile);
- = get_jiter_objfile_data (the_objfile);
...
but consequently we'll have two jit breakpoints, so we also make sure we don't
set a jit breakpoint on separate debug objects like libLLVM.so.10.debug.
Tested on x86_64-linux.
gdb/ChangeLog:
2021-05-21 Tom de Vries <tdevries@suse.de>
PR breakpoint/27889
* jit.c (jit_breakpoint_re_set_internal): Skip separate debug
objects. Call get_jiter_objfile_data with the_objfile.
Give a name to each observer, this will help produce more meaningful
debug message.
gdbsupport/ChangeLog:
* observable.h (class observable) <struct observer> <observer>:
Add name parameter.
<name>: New field.
<attach>: Add name parameter, update all callers.
Change-Id: Ie0cc4664925215b8d2b09e026011b7803549fba0
This commits the result of running gdb/copyright.py as per our Start
of New Year procedure...
gdb/ChangeLog
Update copyright year range in copyright header of all GDB files.
This function allows to create a BFD handle using an accessible memory
range in a target memory. It is currently contained in a JIT module but
this functionality may be of wider usefullness - for example, reading
ELF binaries contained within a core dump.
gdb/ChangeLog:
2020-12-07 Mihails Strasuns <mihails.strasuns@intel.com>
* jit.c (mem_bfd*, bfd_open_from_target_memory): Removed.
* gdb_bfd.h (gdb_bfd_open_from_target_memory): New function.
* gdb_bfd.c (mem_bfd*, gdb_bfd_open_from_target_memory): New functions.
I want to add another action (clearing displaced stepping state) that
happens when an inferior execs. I think it would be cleaner to have an
observer for this event, rather than have infrun know about each other
sub-component.
Replace the calls to solib_create_inferior_hook and
jit_inferior_created_hook in follow_exec by observers.
gdb/ChangeLog:
* observable.h (inferior_execd): Declare new observable.
* observable.c (inferior_execd): Declare new observable.
* infrun.c (follow_exec): Notify inferior_execd observer.
* jit.c (jit_inferior_created_hook): Make static.
(_initialize_jit): Register inferior_execd observer.
* jit.h (jit_inferior_created_hook): Remove declaration.
* solib.c (_initialize_solib): Register inferior_execd observer.
Change-Id: I000cce00094e23baa67df693d912646b6ae38e44
Use the inferior parameter now available in jit_inferior_created_hook.
It is passed down to jit_inferior_init, which uses it as much as
possible instead of the current inferior or current program space.
gdb/ChangeLog:
* jit.c (jit_reader_load_command): Pass current inferior.
(jit_inferior_init): Change parameter type to inferior, use it.
(jit_inferior_created): Remove.
(jit_inferior_created_hook): Pass inferior parameter down.
(_initialize_jit): Use jit_inferior_created_hook instead of
jit_inferior_created.
* jit.h (jit_inferior_created_hook): Add inferior parameter.
* infrun.c (follow_exec): Pass inferior to
jit_inferior_created_hook.
Change-Id: If3a2114a933370dd313d5abd623136d273cdb8fa
I think it would make sense for the inferior_created observable to say
which inferior is being dealt with, rather than relying on it being the
current inferior.
This patch adds an inferior parameter to inferior_created, but does not
change the callbacks to use it.
gdb/ChangeLog:
* aix-thread.c (aix_thread_inferior_created): Add inferior
parameter.
* bsd-uthread.c (bsd_uthread_inferior_created): Likewise.
* dummy-frame.c (cleanup_dummy_frames): Likewise.
* jit.c (jit_inferior_created): Likewise.
* linux-thread-db.c (thread_db_inferior_created): Likewise.
* m68k-linux-tdep.c (m68k_linux_inferior_created): Likewise.
* observable.h (inferior_created): Likewise.
* ravenscar-thread.c (ravenscar_inferior_created): Likewise.
* symfile-mem.c (add_vsyscall_page): Likewise.
* infcmd.c (post_create_inferior): Pass inferior argument.
Change-Id: I2543d19ff055a9df6b269929faea10b27d2adc5e
I noticed that non of the listeners of the inferior_created observable
used either of the arguments. Remove them. This in turn allows
removing the target parameter of post_create_inferior.
Tested only by rebuilding.
gdb/ChangeLog:
* observable.h <inferior_created>: Remove parameters. Update all
listeners.
* inferior.h (post_create_inferior): Remove target parameter.
Update all callers.
Change-Id: I8944cefdc4447ed5347dc927b75abf1e7a0e27e6
My understanding is that it's mildly better to use a static const
array, as opposed to a "const char *", for a global string constant,
when possible. This makes sense to me because the pointer requires a
load from an address, whereas the array is just the address.
So, I searched for these in gdb and gdbserver. This patch fixes the
ones I found.
gdb/ChangeLog
2020-09-15 Tom Tromey <tromey@adacore.com>
* unittests/memory-map-selftests.c (valid_mem_map): Now array.
* ui-style.c (ansi_regex_text): Now array.
* rust-exp.y (number_regex_text): Now array.
* linespec.c (linespec_quote_characters): Now array.
* jit.c (jit_break_name, jit_descriptor_name, reader_init_fn_sym):
Now arrays.
gdbserver/ChangeLog
2020-09-15 Tom Tromey <tromey@adacore.com>
* linux-x86-low.cc (xmltarget_i386_linux_no_xml)
(xmltarget_amd64_linux_no_xml): Now arrays.
To detect whether an objfile is a JITer, we lookup JIT interface
symbols in the objfile. If an objfile does not have these symbols, we
conclude that it is not a JITer. An objfile that does not have the
symbols will never have them. Therefore, once we do a lookup and find
out that the objfile does not have JIT symbols, just set a flag so
that we can skip symbol lookup for that objfile the next time we reset
JIT breakpoints.
gdb/ChangeLog:
2020-07-22 Simon Marchi <simon.marchi@polymtl.ca>
Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* objfiles.h (struct objfile) <skip_jit_symbol_lookup>: New field.
* jit.c (jit_breakpoint_re_set_internal): Use the
`skip_jit_symbol_lookup` field.
gdb/ChangeLog:
2020-07-22 Simon Marchi <simon.marchi@polymtl.ca>
Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* jit.c (jit_read_descriptor): Define the descriptor address once,
use twice.
(jit_breakpoint_deleted): Move the declaration of the loop variable
`iter` into the loop header.
(jit_breakpoint_re_set_internal): Move the declaration of the local
variable `objf_data` to the first point of definition.
(jit_event_handler): Move the declaration of local variables
`code_entry`, `entry_addr`, and `objf` to their first point of use.
Rename `objf` to `jited`.
This is no longer needed, remove it.
gdb/ChangeLog:
2020-07-22 Simon Marchi <simon.marchi@polymtl.ca>
* jit.h (struct jiter_objfile_data) <jiter_objfile_data, objfile>:
Remove.
* jit.c (get_jiter_objfile_data): Update.
GDB's JIT handler stores an objfile (and data associated with it) per
program space to keep track of JIT breakpoint information. This assumes
that there is at most one JITer objfile in the program space. However,
there may be multiple. If so, only the first JITer's hook breakpoints
would be realized and the JIT events from the other JITers would be
missed.
This patch removes that assumption, allowing an arbitrary number of
objfiles within a program space to be JITers.
- The "unique" program_space -> JITer objfile pointer in
jit_program_space_data is removed. In fact, jit_program_space_data
becomes empty, so it is removed entirely.
- jit_breakpoint_deleted is modified, it now has to assume that any
objfile in a program space is a potential JITer. It now iterates on
all objfiles, checking if they are indeed JITers, and if they are,
whether the deleted breakpoint belongs to them.
- jit_breakpoint_re_set_internal also has to assume that any objfile in
a program space is a potential JITer. It creates (or updates) one
jiter_objfile_data structure for each JITer it finds.
- Same for jit_inferior_init. It now iterates all objfiles to read the
initial JIT object list.
gdb/ChangeLog:
2020-07-22 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Simon Marchi <simon.marchi@polymtl.ca>
* jit.c (struct jit_program_space_data): Remove.
(jit_program_space_key): Remove.
(jiter_objfile_data::~jiter_objfile_data): Remove program space
stuff.
(get_jit_program_space_data): Remove.
(jit_breakpoint_deleted): Iterate on all of the program space's
objfiles.
(jit_inferior_init): Likewise.
(jit_breakpoint_re_set_internal): Likewise. Also change return
type to void.
(jit_breakpoint_re_set): Pass current_program_space to
jit_breakpoint_re_set_internal.
gdb/testsuite/ChangeLog:
2020-07-22 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* gdb.base/jit-reader-simple.exp: Add a scenario for a binary that
loads two JITers.
This is in preparation for allowing more than one JITer objfile per
program space. Once we do that, each JITer objfile will have its own
JIT breakpoint (on the __jit_debug_register_code function it provides).
The cached_code_address field is just the runtime / relocated address of
that symbol.
Since they are going to become JITer-objfile-specific and not
program-space-specific, move these fields from jit_program_space_data to
jiter_objfile_data.
gdb/ChangeLog:
2020-07-22 Simon Marchi <simon.marchi@polymtl.ca>
* jit.h (struct jiter_objfile_data) <cached_code_address,
jit_breakpoint>: Move to here from ...
* jit.c (jit_program_space_data): ... here.
(jiter_objfile_data::~jiter_objfile_data): Update.
(jit_breakpoint_deleted): Update.
(jit_breakpoint_re_set_internal): Update.
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.
The jit_objfile_data is currently used to hold information about both
objfiles that are the result of JIT compilation (JITed) and objfiles
that can produce JITed objfiles (JITers). I think that this double use
of the type is confusing, and that things would be more obvious if we
had one type for each role.
This patch splits it into:
- jited_objfile_data: for data about an objfile that is the result of a
JIT compilation
- jiter_objfile_data: for data about an objfile which produces JITed
objfiles
There are now two JIT-related fields in an objfile, one for each kind.
With this change, the following invariants hold:
- an objfile has a non-null `jiter_data` field iff it defines the required
symbols of the JIT interface
- an objfile has a non-null `jited_data` field iff it is the product of
JIT compilation (has been produced by some JITer)
gdb/ChangeLog:
2020-07-22 Simon Marchi <simon.marchi@polymtl.ca>
* jit.h (struct jit_objfile_data): Split into...
(struct jiter_objfile_data): ... this ...
(struct jited_objfile_data): ... and this.
* objfiles.h (struct objfile) <jit_data>: Remove.
<jiter_data, jited_data>: New fields.
* jit.c (jit_objfile_data::~jit_objfile_data): Rename to ...
(jiter_objfile_data::~jiter_objfile_data): ... this.
(get_jit_objfile_data): Rename to ...
(get_jiter_objfile_data): ... this.
(add_objfile_entry): Update.
(jit_read_descriptor): Use get_jiter_objfile_data.
(jit_find_objf_with_entry_addr): Use objfile's jited_data field.
(jit_breakpoint_re_set_internal): Use get_jiter_objfile_data.
(jit_inferior_exit_hook): Use objfile's jited_data field.
Remove the use of objfile_data to associate a jit_objfile_data with an
objfile. Instead, directly link to a jit_objfile_data from an objfile
struct. The goal is to eliminate unnecessary abstraction.
The free_objfile_data function naturally becomes the destructor of
jit_objfile_data. However, free_objfile_data accesses the objfile to
which the data is attached, which the destructor of jit_objfile_data
doesn't have access to. To work around this, add a backlink to the
owning objfile in jit_objfile_data. This is however temporary, it goes
away in a subsequent patch.
gdb/ChangeLog:
2020-07-22 Simon Marchi <simon.marchi@polymtl.ca>
* jit.h: Forward-declare `struct minimal_symbol`.
(struct jit_objfile_data): Migrate to here from jit.c; also add a
constructor, destructor, and an objfile* field.
* jit.c (jit_objfile_data): Remove.
(struct jit_objfile_data): Migrate from here to jit.h.
(jit_objfile_data::~jit_objfile_data): New destructor
implementation with code moved from free_objfile_data.
(free_objfile_data): Delete.
(get_jit_objfile_data): Update to use the jit_data field of objfile.
(jit_find_objf_with_entry_addr): Ditto.
(jit_inferior_exit_hook): Ditto.
(_initialize_jit): Remove the call to
register_objfile_data_with_cleanup.
* objfiles.h (struct objfile) <jit_data>: New field.
This is a refactoring that adds a new parameter to the `jit_event_handler`
function: the JITer objfile. The goal is to distinguish which JITer
triggered the JIT event, in case there are multiple JITers -- a capability
that is added in a subsequent patch.
gdb/ChangeLog:
2020-07-22 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* jit.h: Forward-declare `struct objfile`.
(jit_event_handler): Add a second parameter, the JITer objfile.
* jit.c (jit_read_descriptor): Change the signature to take the
JITer objfile as an argument instead of the jit_program_space_data.
(jit_inferior_init): Update the call to jit_read_descriptor.
(jit_event_handler): Use the new JITer objfile argument when calling
jit_read_descriptor.
* breakpoint.c (handle_jit_event): Update the call to
jit_event_handler to pass the JITer objfile.
