From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id CFD83383E83C for ; Sun, 14 Jun 2020 17:09:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CFD83383E83C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 296B21E111; Sun, 14 Jun 2020 13:09:08 -0400 (EDT) Subject: Re: [PATCH 2/3] gdb/jit: enable tracking multiple jitter objfiles To: Tankut Baris Aktemur , gdb-patches@sourceware.org References: <886ee7b960b7da9ee20ae3c4b889d2b9c3246b33.1590397723.git.tankut.baris.aktemur@intel.com> From: Simon Marchi Message-ID: <298ca402-f06a-4901-84c3-c8ca58a00cee@simark.ca> Date: Sun, 14 Jun 2020 13:09:07 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <886ee7b960b7da9ee20ae3c4b889d2b9c3246b33.1590397723.git.tankut.baris.aktemur@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, 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: Sun, 14 Jun 2020 17:09:10 -0000 On 2020-05-25 5:38 a.m., Tankut Baris Aktemur via Gdb-patches wrote: > diff --git a/gdb/jit.c b/gdb/jit.c > index fdb1248ed5b..c689ac3f392 100644 > --- a/gdb/jit.c > +++ b/gdb/jit.c > @@ -966,58 +966,53 @@ jit_breakpoint_re_set_internal (struct gdbarch *gdbarch, The comment above jit_breakpoint_re_set_internal says: Return 0 if the jit breakpoint has been successfully initialized. Can you adjust it to the new reality? Something like "Return 0 if one or more JIT breakpoints were successfully initialized". I think it would also be good (as a separate cleanup) to make this function return a bool, returning true when one or more JIT breakpoints were inserted. > @@ -1357,38 +1352,38 @@ jit_event_handler (struct gdbarch *gdbarch) > > jit_program_space_data *ps_data = get_jit_program_space_data (); > > - /* Fetch the saved objfile. */ > - objfile *objf = nullptr; > - if (!ps_data->objfile_and_bps.empty ()) > - objf = (ps_data->objfile_and_bps.begin ())->first; > - > - /* Read the descriptor from remote memory. */ > - if (!jit_read_descriptor (gdbarch, &descriptor, objf)) > - return; > - entry_addr = descriptor.relevant_entry; > - > - /* Do the corresponding action. */ > - switch (descriptor.action_flag) > + for (auto &objf_and_bp : ps_data->objfile_and_bps) > { > - case JIT_NOACTION: > - break; > - case JIT_REGISTER: > - jit_read_code_entry (gdbarch, entry_addr, &code_entry); > - jit_register_code (gdbarch, entry_addr, &code_entry); > - break; > - case JIT_UNREGISTER: > - objf = jit_find_objf_with_entry_addr (entry_addr); > - if (objf == NULL) > - printf_unfiltered (_("Unable to find JITed code " > - "entry at address: %s\n"), > - paddress (gdbarch, entry_addr)); > - else > - objf->unlink (); > + objfile *objf = objf_and_bp.first; > > - break; > - default: > - error (_("Unknown action_flag value in JIT descriptor!")); > - break; > + /* Read the descriptor from remote memory. */ > + if (!jit_read_descriptor (gdbarch, &descriptor, objf)) > + continue; > + entry_addr = descriptor.relevant_entry; > + > + /* Do the corresponding action. */ > + switch (descriptor.action_flag) > + { > + case JIT_NOACTION: > + break; > + case JIT_REGISTER: > + jit_read_code_entry (gdbarch, entry_addr, &code_entry); > + jit_register_code (gdbarch, entry_addr, &code_entry); > + break; > + case JIT_UNREGISTER: > + objf = jit_find_objf_with_entry_addr (entry_addr); > + if (objf == NULL) > + printf_unfiltered (_("Unable to find JITed code " > + "entry at address: %s\n"), > + paddress (gdbarch, entry_addr)); > + else > + objf->unlink (); > + > + break; > + default: > + error (_("Unknown action_flag value in JIT descriptor!")); > + break; > + } I don't think that iterating on all JIT-providing objfiles is the correct thing to do here. After all, we have hit the __jit_debug_register_code breakpoint of exactly one of them, so we should only process the jit descriptor of that one. At the same moment, the other JIT-providing objfiles could have prepared anything in their descriptor (or there could be junk data), but they have not asked us to process it. I haven't tried, but I bet that you could build a test to trigger a bug here: 1. Have two jit-providing objfiles register one jit objfile each 2. Have the two jit-providing objfiles prepare their descriptor with JIT_UNREGISTER and the address of their jit objfile 3. Have one of them call __jit_debug_register_code. We would forget about both jit objfiles, when in reality we should forget only about one. I think we'll need to know which jit event breakpoint was hit, so we can know which jit-providing objfile asked us to process its descriptor. Simon