From c8474dc35380ff7426ec4c555512c098fa70dd04 Mon Sep 17 00:00:00 2001 From: Tankut Baris Aktemur Date: Wed, 22 Jul 2020 15:56:07 +0200 Subject: [PATCH] gdb/jit: enable tracking multiple JITer objfiles 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 Simon Marchi * 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 * gdb.base/jit-reader-simple.exp: Add a scenario for a binary that loads two JITers. --- gdb/ChangeLog | 16 ++ gdb/jit.c | 171 +++++++------------ gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.base/jit-reader-simple.exp | 43 ++++- 4 files changed, 126 insertions(+), 109 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3a3866a44f7..83f20ac349b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,19 @@ +2020-07-22 Tankut Baris Aktemur + Simon Marchi + + * 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. + 2020-07-22 Simon Marchi * jit.h (struct jiter_objfile_data) jit_program_space_key; - /* Destructor for jiter_objfile_data. */ jiter_objfile_data::~jiter_objfile_data () { - jit_program_space_data *ps_data - = jit_program_space_key.get (this->objfile->pspace); - - gdb_assert (ps_data != nullptr); - gdb_assert (ps_data->objfile == this->objfile); - - ps_data->objfile = nullptr; if (this->jit_breakpoint != nullptr) delete_breakpoint (this->jit_breakpoint); } @@ -290,20 +270,6 @@ add_objfile_entry (struct objfile *objfile, CORE_ADDR entry) objfile->jited_data.reset (new jited_objfile_data (entry)); } -/* Return jit_program_space_data for current program space. Allocate - if not already present. */ - -static struct jit_program_space_data * -get_jit_program_space_data () -{ - struct jit_program_space_data *ps_data; - - ps_data = jit_program_space_key.get (current_program_space); - if (ps_data == NULL) - ps_data = jit_program_space_key.emplace (current_program_space); - return ps_data; -} - /* Helper function for reading the global JIT descriptor from remote memory. Returns true if all went well, false otherwise. */ @@ -908,15 +874,12 @@ jit_breakpoint_deleted (struct breakpoint *b) for (iter = b->loc; iter != NULL; iter = iter->next) { - struct jit_program_space_data *ps_data; - - ps_data = jit_program_space_key.get (iter->pspace); - if (ps_data != nullptr && ps_data->objfile != nullptr) + for (objfile *objf : iter->pspace->objfiles ()) { - objfile *objf = ps_data->objfile; jiter_objfile_data *jiter_data = objf->jiter_data.get (); - if (jiter_data->jit_breakpoint == iter->owner) + if (jiter_data != nullptr + && jiter_data->jit_breakpoint == iter->owner) { jiter_data->cached_code_address = 0; jiter_data->jit_breakpoint = nullptr; @@ -925,62 +888,55 @@ jit_breakpoint_deleted (struct breakpoint *b) } } -/* (Re-)Initialize the jit breakpoint if necessary. - Return true if the jit breakpoint has been successfully initialized. */ +/* (Re-)Initialize the jit breakpoints for JIT-producing objfiles in + PSPACE. */ -static bool -jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, - struct jit_program_space_data *ps_data) +static void +jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, program_space *pspace) { - struct bound_minimal_symbol reg_symbol; - struct bound_minimal_symbol desc_symbol; jiter_objfile_data *objf_data; - CORE_ADDR addr; - if (ps_data->objfile == NULL) + for (objfile *the_objfile : pspace->objfiles ()) { /* Lookup the registration symbol. If it is missing, then we assume we are not attached to a JIT. */ - reg_symbol = lookup_bound_minimal_symbol (jit_break_name); + bound_minimal_symbol reg_symbol + = lookup_minimal_symbol (jit_break_name, nullptr, the_objfile); if (reg_symbol.minsym == NULL || BMSYMBOL_VALUE_ADDRESS (reg_symbol) == 0) - return false; + continue; - desc_symbol = lookup_minimal_symbol (jit_descriptor_name, NULL, - reg_symbol.objfile); + bound_minimal_symbol desc_symbol + = lookup_minimal_symbol (jit_descriptor_name, NULL, the_objfile); if (desc_symbol.minsym == NULL || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0) - return false; + continue; objf_data = get_jiter_objfile_data (reg_symbol.objfile); objf_data->register_code = reg_symbol.minsym; objf_data->descriptor = desc_symbol.minsym; - ps_data->objfile = reg_symbol.objfile; - } - else - objf_data = get_jiter_objfile_data (ps_data->objfile); - - addr = MSYMBOL_VALUE_ADDRESS (ps_data->objfile, objf_data->register_code); - - if (jit_debug) - fprintf_unfiltered (gdb_stdlog, - "jit_breakpoint_re_set_internal, " - "breakpoint_addr = %s\n", - paddress (gdbarch, addr)); + CORE_ADDR addr = MSYMBOL_VALUE_ADDRESS (the_objfile, + objf_data->register_code); - if (objf_data->cached_code_address == addr) - return true; + if (jit_debug) + fprintf_unfiltered (gdb_stdlog, + "jit_breakpoint_re_set_internal, " + "breakpoint_addr = %s\n", + paddress (gdbarch, addr)); - /* Delete the old breakpoint. */ - if (objf_data->jit_breakpoint != nullptr) - delete_breakpoint (objf_data->jit_breakpoint); + /* Check if we need to re-create the breakpoint. */ + if (objf_data->cached_code_address == addr) + continue; - /* Put a breakpoint in the registration symbol. */ - objf_data->cached_code_address = addr; - objf_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr); + /* Delete the old breakpoint. */ + if (objf_data->jit_breakpoint != nullptr) + delete_breakpoint (objf_data->jit_breakpoint); - return true; + /* Put a breakpoint in the registration symbol. */ + objf_data->cached_code_address = addr; + objf_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr); + } } /* The private data passed around in the frame unwind callback @@ -1219,7 +1175,6 @@ jit_inferior_init (struct gdbarch *gdbarch) { struct jit_descriptor descriptor; struct jit_code_entry cur_entry; - struct jit_program_space_data *ps_data; CORE_ADDR cur_entry_addr; if (jit_debug) @@ -1227,42 +1182,43 @@ jit_inferior_init (struct gdbarch *gdbarch) jit_prepend_unwinder (gdbarch); - ps_data = get_jit_program_space_data (); - if (!jit_breakpoint_re_set_internal (gdbarch, ps_data)) - return; + jit_breakpoint_re_set_internal (gdbarch, current_program_space); - /* There must be a JITer registered, otherwise we would exit early - above. */ - objfile *jiter = ps_data->objfile; + for (objfile *jiter : current_program_space->objfiles ()) + { + if (jiter->jiter_data == nullptr) + continue; - /* Read the descriptor so we can check the version number and load - any already JITed functions. */ - if (!jit_read_descriptor (gdbarch, &descriptor, jiter)) - return; + /* Read the descriptor so we can check the version number and load + any already JITed functions. */ + if (!jit_read_descriptor (gdbarch, &descriptor, jiter)) + continue; - /* Check that the version number agrees with that we support. */ - if (descriptor.version != 1) - { - printf_unfiltered (_("Unsupported JIT protocol version %ld " - "in descriptor (expected 1)\n"), - (long) descriptor.version); - return; - } + /* Check that the version number agrees with that we support. */ + if (descriptor.version != 1) + { + printf_unfiltered (_("Unsupported JIT protocol version %ld " + "in descriptor (expected 1)\n"), + (long) descriptor.version); + continue; + } - /* If we've attached to a running program, we need to check the descriptor - to register any functions that were already generated. */ - for (cur_entry_addr = descriptor.first_entry; - cur_entry_addr != 0; - cur_entry_addr = cur_entry.next_entry) - { - jit_read_code_entry (gdbarch, cur_entry_addr, &cur_entry); + /* If we've attached to a running program, we need to check the + descriptor to register any functions that were already + generated. */ + for (cur_entry_addr = descriptor.first_entry; + cur_entry_addr != 0; + cur_entry_addr = cur_entry.next_entry) + { + jit_read_code_entry (gdbarch, cur_entry_addr, &cur_entry); - /* This hook may be called many times during setup, so make sure we don't - add the same symbol file twice. */ - if (jit_find_objf_with_entry_addr (cur_entry_addr) != NULL) - continue; + /* This hook may be called many times during setup, so make sure + we don't add the same symbol file twice. */ + if (jit_find_objf_with_entry_addr (cur_entry_addr) != NULL) + continue; - jit_register_code (gdbarch, cur_entry_addr, &cur_entry); + jit_register_code (gdbarch, cur_entry_addr, &cur_entry); + } } } @@ -1288,8 +1244,7 @@ jit_inferior_created_hook (void) void jit_breakpoint_re_set (void) { - jit_breakpoint_re_set_internal (target_gdbarch (), - get_jit_program_space_data ()); + jit_breakpoint_re_set_internal (target_gdbarch (), current_program_space); } /* This function cleans up any code entries left over when the diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 1f0861e706a..c83dc40d12e 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-07-22 Tankut Baris Aktemur + + * gdb.base/jit-reader-simple.exp: Add a scenario for a binary that + loads two JITers. + 2020-07-21 Andrew Burgess * gdb.python/py-arch-reg-groups.exp: Additional tests. diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp index 48bd326b533..a8f33c6d7a8 100644 --- a/gdb/testsuite/gdb.base/jit-reader-simple.exp +++ b/gdb/testsuite/gdb.base/jit-reader-simple.exp @@ -34,6 +34,7 @@ standard_testfile set libname $testfile-jit set srcfile_lib $srcdir/$subdir/$libname.c set binfile_lib [standard_output_file $libname.so] +set binfile_lib2 [standard_output_file ${libname}2.so] # Build a standalone JIT binary. @@ -53,12 +54,15 @@ proc build_standalone_jit {{options ""}} { proc build_shared_jit {{options ""}} { global testfile - global srcfile_lib binfile_lib + global srcfile_lib binfile_lib binfile_lib2 lappend options "debug additional_flags=-fPIC" if { [gdb_compile_shlib $srcfile_lib $binfile_lib $options] != "" } { return -1 } + if { [gdb_compile_shlib $srcfile_lib $binfile_lib2 $options] != "" } { + return -1 + } return 0 } @@ -83,6 +87,15 @@ if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl executable \ return -1 } +# Build the program that loads *two* JIT libraries. +set binfile_dl2 $binfile-dl2 +set options [list debug shlib=${binfile_lib} shlib=${binfile_lib2}] +if {[gdb_compile ${srcdir}/${subdir}/${srcfile_dl} $binfile_dl2 executable \ + $options] == -1 } { + untested "failed to compile two-jitter binary" + return -1 +} + # STANDALONE is true when the JIT reader is included directly in the # main program. False when the JIT reader is in a separate shared # library. If CHANGE_ADDR is true, force changing the JIT descriptor @@ -160,3 +173,31 @@ foreach standalone {1 0} { } } } + +# Now start the program that loads two JITer libraries and expect to +# see JIT breakpoints defined for both. + +with_test_prefix "two JITers" { + clean_restart $binfile_dl2 + + if {![runto_main]} { + untested "could not run to main" + return -1 + } + + set num_bps 0 + set ws "\[ \t\]+" + gdb_test_multiple "maint info breakpoints" "have two jit breakpoints" { + -re "jit events${ws}keep y${ws}$hex <__jit_debug_register_code> inf 1\r\n" { + incr num_bps + exp_continue + } + -re "$gdb_prompt $" { + if {$num_bps == 2} { + pass $gdb_test_name + } else { + fail $gdb_test_name + } + } + } +} -- 2.30.2