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 D36DB385B831 for ; Mon, 23 Mar 2020 03:03:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D36DB385B831 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] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 421D71E5F8; Sun, 22 Mar 2020 23:03:48 -0400 (EDT) Subject: Re: [PATCH 6/7] [gdb/testsuite] use -Ttext-segment for jit-elf tests To: Mihails Strasuns , gdb-patches@sourceware.org References: <20200218124339.11270-1-mihails.strasuns@intel.com> <20200218124339.11270-7-mihails.strasuns@intel.com> From: Simon Marchi Message-ID: <80652212-6dcf-79b6-c2f0-e4cc26810c87@simark.ca> Date: Sun, 22 Mar 2020 23:03:47 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200218124339.11270-7-mihails.strasuns@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-30.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, KAM_SHORT, 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: Mon, 23 Mar 2020 03:03:51 -0000 Just some formatting nits: > @@ -96,6 +89,15 @@ update_locations (const void *const addr, int idx) > #define MAIN main > #endif > > +/* Must be defined by .exp file when compiling to know > + what address to map the ELF binary to. */ > +#ifndef LOAD_ADDRESS > +#error "Must define LOAD_ADDRESS" > +#endif > +#ifndef LOAD_INCREMENT > +#error "Must define LOAD_INCREMENT" > +#endif > + > /* Used to spin waiting for GDB. */ > volatile int wait_for_gdb = ATTACH; > #define WAIT_FOR_GDB while (wait_for_gdb) > @@ -137,7 +139,8 @@ MAIN (int argc, char *argv[]) > exit (1); > } > > - const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE, > + void* load_addr = (void*) (size_t) (LOAD_ADDRESS + (i-1) * LOAD_INCREMENT); > + const void *const addr = mmap (load_addr, st.st_size, PROT_READ|PROT_WRITE, > MAP_PRIVATE, fd, 0); We try to use the same style in code for tests as code for GDB itself. So void* load_addr = (void*) (size_t) (LOAD_ADDRESS + (i-1) * LOAD_INCREMENT); should be void *load_addr = (void *) (size_t) (LOAD_ADDRESS + (i - 1) * LOAD_INCREMENT); I think we should be using MAP_FIXED in this mmap call, to make sure the mapping ends up exactly at `load_addr`, and check the result to make sure it's not MAP_FAILED. > struct jit_code_entry *const entry = calloc (1, sizeof (*entry)); > > @@ -147,7 +150,7 @@ MAIN (int argc, char *argv[]) > exit (1); > } > > - update_locations (addr, i); > + update_name (addr, i); > > /* Link entry at the end of the list. */ > entry->symfile_addr = (const char *)addr; > diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp > index f63694b8f5..cff47ec73c 100644 > --- a/gdb/testsuite/lib/jit-elf-helpers.exp > +++ b/gdb/testsuite/lib/jit-elf-helpers.exp > @@ -13,15 +13,25 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see . > > +# Magic constants used to calculate a starting address when linking > +# "jit" shared libraries. When loaded, will be mapped by jit-elf-main Our standard is to use two spaces after periods in comments and other "prose". Simon