From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by sourceware.org (Postfix) with ESMTPS id A24D9388B014 for ; Tue, 16 Jun 2020 09:51:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A24D9388B014 IronPort-SDR: ZwKI4Uj4vEAXitW/aUJO8Tv9QHWifHJ8s6Yf/bbRPDGn3hwLT55/OHaf/FfGum9SyHSbd52E6E xXHbnPfwj3qw== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jun 2020 02:51:22 -0700 IronPort-SDR: zgpXG9VXar6EzBIwlZQxPPdf8LULm8r5Vl9pgGSMLmXM0qD0sUp6QU/7+KrZ0UZF/8x2xOUo1p RtN6+mp4HTEg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,518,1583222400"; d="scan'208";a="382824053" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by fmsmga001.fm.intel.com with ESMTP; 16 Jun 2020 02:51:21 -0700 Received: from ulvlx001.iul.intel.com (ulvlx001.iul.intel.com [172.28.207.17]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id 05G9pKQX020548; Tue, 16 Jun 2020 10:51:20 +0100 Received: from ulvlx001.iul.intel.com (localhost [127.0.0.1]) by ulvlx001.iul.intel.com with ESMTP id 05G9pKHM004980; Tue, 16 Jun 2020 11:51:20 +0200 Received: (from taktemur@localhost) by ulvlx001.iul.intel.com with LOCAL id 05G9pKYp004976; Tue, 16 Jun 2020 11:51:20 +0200 From: Tankut Baris Aktemur To: gdb-patches@sourceware.org Cc: simark@simark.ca Subject: [PATCH v2 3/3] gdb/jit: enable tracking multiple jitter objfiles Date: Tue, 16 Jun 2020 11:49:45 +0200 Message-Id: <4f7d8cfd9635484c5148e72f0941a0d9f369b7f3.1592299502.git.tankut.baris.aktemur@intel.com> X-Mailer: git-send-email 1.7.0.7 In-Reply-To: References: In-Reply-To: References: X-Spam-Status: No, score=-22.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_STOCKGEN, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jun 2020 09:51:27 -0000 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 replaces the single objfile pointer with a list so that multiple objfiles can be tracked per program space. This patch is viewed better with 'git diff -w'. Regression-tested on X86_64 Linux. gdb/ChangeLog: 2020-06-15 Tankut Baris Aktemur * jit.c: (struct jiter_and_bp): New struct. (struct jit_program_space_data): Enable storing multiple objfiles. (jit_breakpoint_re_set_internal): Iterate over all objfiles in the program space to look for JIT symbols. (jit_breakpoint_deleted): Update to iterate over multiple jiters. (free_objfile_data): Ditto. (jit_inferior_init): Ditto. gdb/testsuite/ChangeLog: 2020-06-15 Tankut Baris Aktemur * gdb.base/jit-reader-simple.exp: Add a scenario for a binary that loads two JITers. --- gdb/jit.c | 172 +++++++++++-------- gdb/testsuite/gdb.base/jit-reader-simple.exp | 43 ++++- 2 files changed, 144 insertions(+), 71 deletions(-) diff --git a/gdb/jit.c b/gdb/jit.c index 5da0601b024..2a15aac9e70 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -241,17 +241,15 @@ jit_reader_unload_command (const char *args, int from_tty) loaded_jit_reader = NULL; } -/* Per-program space structure recording which objfile has the JIT - symbols. */ +/* Structure recording JIT breakpoints per JITer objfile. */ -struct jit_program_space_data +struct jiter_and_bp { - /* The objfile. This is NULL if no objfile holds the JIT - symbols. */ + /* The JITer objfile. */ - struct objfile *objfile = nullptr; + objfile *jiter = nullptr; - /* If this program space has __jit_debug_register_code, this is the + /* If this objfile has __jit_debug_register_code, this is the cached address from the minimal symbol. This is used to detect relocations requiring the breakpoint to be re-created. */ @@ -260,7 +258,17 @@ struct jit_program_space_data /* This is the JIT event breakpoint, or NULL if it has not been set. */ - struct breakpoint *jit_breakpoint = nullptr; + breakpoint *jit_breakpoint = nullptr; +}; + +/* Per-program space structure recording the objfiles and their JIT + symbols. */ + +struct jit_program_space_data +{ + /* The JIT breakpoint information associated with JITer objfiles. */ + + std::forward_list jiter_and_bps; }; static program_space_key jit_program_space_key; @@ -941,16 +949,18 @@ jit_breakpoint_deleted (struct breakpoint *b) struct jit_program_space_data *ps_data; ps_data = jit_program_space_key.get (iter->pspace); - if (ps_data != NULL && ps_data->jit_breakpoint == iter->owner) - { - ps_data->cached_code_address = 0; - ps_data->jit_breakpoint = NULL; - } + if (ps_data != nullptr) + for (auto &info : ps_data->jiter_and_bps) + if (info.jit_breakpoint == iter->owner) + { + info.cached_code_address = 0; + info.jit_breakpoint = nullptr; + } } } -/* (Re-)Initialize the jit breakpoint if necessary. - Return true if the jit breakpoint has been successfully initialized. */ +/* (Re-)Initialize the jit breakpoint if necessary. Return true if + one or more jit breakpoints have been successfully initialized. */ static bool jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, @@ -961,50 +971,62 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, struct jit_objfile_data *objf_data; CORE_ADDR addr; - if (ps_data->objfile == NULL) + for (objfile *the_objfile : current_program_space->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); + 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); if (desc_symbol.minsym == NULL || BMSYMBOL_VALUE_ADDRESS (desc_symbol) == 0) - return false; + continue; objf_data = get_jit_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_jit_objfile_data (ps_data->objfile); + the_objfile = reg_symbol.objfile; - addr = MSYMBOL_VALUE_ADDRESS (ps_data->objfile, objf_data->register_code); + addr = MSYMBOL_VALUE_ADDRESS (the_objfile, objf_data->register_code); - if (jit_debug) - fprintf_unfiltered (gdb_stdlog, - "jit_breakpoint_re_set_internal, " - "breakpoint_addr = %s\n", - paddress (gdbarch, addr)); + if (jit_debug) + fprintf_unfiltered (gdb_stdlog, + "jit_breakpoint_re_set_internal, " + "breakpoint_addr = %s\n", + paddress (gdbarch, addr)); - if (ps_data->cached_code_address == addr) - return true; + /* Search for an existing entry, or insert new if necessary. */ + auto iter = ps_data->jiter_and_bps.begin (); + for (; iter != ps_data->jiter_and_bps.end (); iter++) + if (iter->jiter == the_objfile) + break; - /* Delete the old breakpoint. */ - if (ps_data->jit_breakpoint != NULL) - delete_breakpoint (ps_data->jit_breakpoint); + if (iter == ps_data->jiter_and_bps.end ()) + { + ps_data->jiter_and_bps.emplace_front (); + iter = ps_data->jiter_and_bps.begin (); + iter->jiter = the_objfile; + } - /* Put a breakpoint in the registration symbol. */ - ps_data->cached_code_address = addr; - ps_data->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr); + if (iter->cached_code_address == addr) + continue; - return true; + /* Delete the old breakpoint. */ + if (iter->jit_breakpoint != nullptr) + delete_breakpoint (iter->jit_breakpoint); + + /* Put a breakpoint in the registration symbol. */ + iter->cached_code_address = addr; + iter->jit_breakpoint = create_jit_event_breakpoint (gdbarch, addr); + } + + return !ps_data->jiter_and_bps.empty (); } /* The private data passed around in the frame unwind callback @@ -1255,38 +1277,40 @@ jit_inferior_init (struct gdbarch *gdbarch) if (!jit_breakpoint_re_set_internal (gdbarch, ps_data)) return; - /* There must be a JITer registered, otherwise we would exit early - above. */ - objfile *jiter = ps_data->objfile; + for (auto &info : ps_data->jiter_and_bps) + { + objfile *jiter = info.jiter; - /* 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); + } } } @@ -1383,12 +1407,20 @@ free_objfile_data (struct objfile *objfile, void *data) struct jit_program_space_data *ps_data; ps_data = jit_program_space_key.get (objfile->pspace); - if (ps_data != NULL && ps_data->objfile == objfile) + if (ps_data != nullptr) { - ps_data->objfile = NULL; - if (ps_data->jit_breakpoint != NULL) - delete_breakpoint (ps_data->jit_breakpoint); - ps_data->cached_code_address = 0; + /* Remove the data kept for objfile, but delete the + breakpoint, too. */ + ps_data->jiter_and_bps.remove_if ([=] (jiter_and_bp &info) + { + if (info.jiter == objfile) + { + if (info.jit_breakpoint != nullptr) + delete_breakpoint (info.jit_breakpoint); + return true; + } + return false; + }); } } diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp index 48bd326b533..d8a54a37ce7 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 for the two-JITer test" + 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.17.1