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 407FE383E83C for ; Sun, 14 Jun 2020 17:50:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 407FE383E83C 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 C68B41E111; Sun, 14 Jun 2020 13:50:23 -0400 (EDT) Subject: Re: [PATCH 1/3] gdb/jit: use a map to store objfile and jit breakpoint info To: Tankut Baris Aktemur , gdb-patches@sourceware.org References: <618117c0c731e4511927563ed376a6ad84ed138c.1590397723.git.tankut.baris.aktemur@intel.com> From: Simon Marchi Message-ID: Date: Sun, 14 Jun 2020 13:50:23 -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: <618117c0c731e4511927563ed376a6ad84ed138c.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:50:25 -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 1b5ef46469e..fdb1248ed5b 100644 > --- a/gdb/jit.c > +++ b/gdb/jit.c > @@ -42,6 +42,7 @@ > #include "readline/tilde.h" > #include "completer.h" > #include > +#include > > static std::string jit_reader_dir; > > @@ -241,17 +242,11 @@ 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. */ > +/* Per-objfile structure recording JIT breakpoints. */ Just to help disambiguate, maybe precise here that we are talking about the JIT-providing objfiles (those that define the magic interface symbols), not the objfiles that are the result of the JIT. > > -struct jit_program_space_data > +struct jit_objfile_bp > { > - /* The objfile. This is NULL if no objfile holds the JIT > - symbols. */ > - > - struct objfile *objfile = 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 +255,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 informations associated to objfiles. */ > + > + std::map objfile_and_bps; If we don't care about key ordering, I'd use an std::unordered_map. But really, if we expect just to have a handful of items, it would probably be more efficient to have a list or vector. Also, given my comment on the following patch, I think we'll have to do lookups by breakpoint address, so we would have to iterate on the maps items anyway. Unless we use the breakpoint address as the key. > }; > > static program_space_key jit_program_space_key; > @@ -332,9 +337,9 @@ get_jit_program_space_data () > memory. Returns 1 if all went well, 0 otherwise. */ > > static int > -jit_read_descriptor (struct gdbarch *gdbarch, > - struct jit_descriptor *descriptor, > - struct jit_program_space_data *ps_data) > +jit_read_descriptor (gdbarch *gdbarch, > + jit_descriptor *descriptor, > + objfile *objf) > { > int err; > struct type *ptr_type; > @@ -344,17 +349,17 @@ jit_read_descriptor (struct gdbarch *gdbarch, > enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > struct jit_objfile_data *objf_data; > > - if (ps_data->objfile == NULL) > + if (objf == nullptr) > return 0; I would probably change jit_read_descriptor to require a non-NULL objfile. Simon