From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75929 invoked by alias); 16 Mar 2016 16:58:57 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 75703 invoked by uid 89); 16 Mar 2016 16:58:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=preferable, fist, surrounding, assumption X-HELO: e06smtp09.uk.ibm.com Received: from e06smtp09.uk.ibm.com (HELO e06smtp09.uk.ibm.com) (195.75.94.105) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Wed, 16 Mar 2016 16:58:48 +0000 Received: from localhost by e06smtp09.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 16 Mar 2016 16:58:45 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp09.uk.ibm.com (192.168.101.139) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 16 Mar 2016 16:58:43 -0000 X-IBM-Helo: d06dlp02.portsmouth.uk.ibm.com X-IBM-MailFrom: uweigand@de.ibm.com X-IBM-RcptTo: gdb-patches@sourceware.org Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 04E23219005E for ; Wed, 16 Mar 2016 16:58:25 +0000 (GMT) Received: from d06av06.portsmouth.uk.ibm.com (d06av06.portsmouth.uk.ibm.com [9.149.37.217]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u2GGwhpN1114408 for ; Wed, 16 Mar 2016 16:58:43 GMT Received: from d06av06.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u2GGwg19017166 for ; Wed, 16 Mar 2016 12:58:42 -0400 Received: from oc7340732750.ibm.com (icon-9-164-189-191.megacenter.de.ibm.com [9.164.189.191]) by d06av06.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u2GGwgA0017144; Wed, 16 Mar 2016 12:58:42 -0400 Received: by oc7340732750.ibm.com (Postfix, from userid 500) id 59F891537; Wed, 16 Mar 2016 17:58:41 +0100 (CET) Subject: Re: [PATCH 3/4 v2] gdbserver: Add powerpc fast tracepoint support. To: koriakin@0x04.net (=?UTF-8?q?Marcin=20Ko=C5=9Bcielnicki?=) Date: Wed, 16 Mar 2016 16:58:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, koriakin@0x04.net (=?UTF-8?q?Marcin=20Ko=C5=9Bcielnicki?=) In-Reply-To: <1457993420-3934-1-git-send-email-koriakin@0x04.net> from "=?UTF-8?q?Marcin=20Ko=C5=9Bcielnicki?=" at Mar 14, 2016 11:10:20 PM MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Message-Id: <20160316165841.59F891537@oc7340732750.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16031616-0037-0000-0000-0000063E3532 X-SW-Source: 2016-03/txt/msg00257.txt.bz2 Marcin Kościelnicki wrote: > I'm not sure how to properly determine whether ELFv1 or ELFv2 is in use > by the target, so I assume it's the same ABI as in gdbserver itself. That's a generally sensible assumption. In principle, you can check which ABI the target is using by checking the e_flags field in the ELF header. The problem for gdbserver is that it usually doesn't access the ELF file. However, the ELF header is mapped into target memory, so could be checked there. Yet again, the *address* of the ELF header is not very easily detectable without making assumptions on how memory is layed out. The best bet would probably be to determine the AT_PHDR auxillary vector, which gives you the address of the *program headers* for the main executable in the target, and then round that address down to the page size boundary. This should give you the address of the ELF header, when making the assumption that the linker places program headers in the fist page, following the ELF header. This is not necessarily guaranteed by the ELF format, but is what all linkers happen to do. > Allocating the jump pad buffer needs a mention. On powerpc64, IPA simply > scans from 0x10000000 (the executable load address) downwards until it > find a free address. However, on 32-bit, ld.so loads dynamic libraries > there, and they can easily cause us to exceed the 32MiB branch range - > so, instead I aim right after the executable, at sbrk(0). This will > of course cause further memory allocation via brk to fail, but glibc > transparently falls back to mmap in this case, so that's not a problem. > Of course, given a program with large data/bss section, this might > exceed the branch range as well, so perhaps some better heuristic is > in order here. These heuristics seem reasonable to me. > This fixes get_raw_reg for 32-bit (ULONGEST was used to read the > registers dumped by jump pad instead of unsigned long). Also slightly > optimizes gen_limm and fixes a few comments. > +alloc_jump_pad_buffer (size_t size) > +{ > +#ifdef __powerpc64__ > + uintptr_t addr; > + int pagesize; > + void *res; > + > + pagesize = sysconf (_SC_PAGE_SIZE); > + if (pagesize == -1) > + perror_with_name ("sysconf"); > + > + addr = 0x10000000 - size; It also would be preferable to dynamically determine the actual load address, since 0x10000000 is just the default, which can be changed e.g. via linker script or due to PIE address randomization. You might again use the AT_PHDR auxillary vector (which is what Wei-cheng's patches did). > +/* Get the thread area address. This is used to recognize which > + thread is which when tracing with the in-process agent library. We > + don't read anything from the address, and treat it as opaque; it's > + the address itself that we assume is unique per-thread. */ > + > +static int > +ppc_get_thread_area (int lwpid, CORE_ADDR *addr) > +{ > +#ifdef __powerpc64__ > + struct lwp_info *lwp = find_lwp_pid (pid_to_ptid (lwpid)); > + struct thread_info *thr = get_lwp_thread (lwp); > + struct regcache *regcache = get_thread_regcache (thr, 1); > + int is_64 = register_size (regcache->tdesc, 0) == 8; > +#endif > + long res; > + > +#ifdef __powerpc64__ > + if (is_64) > + res = ptrace (PTRACE_PEEKUSER, lwpid, (long) PT_R13 * sizeof (long), > + (long) 0); > + else > +#endif > + res = ptrace (PTRACE_PEEKUSER, lwpid, (long) PT_R2 * sizeof (long), > + (long) 0); Was there a particular reason why you changed to using direct ptrace calls instead of collecting the value from the regcache as in the original patch? > +/* GEN_LOAD and GEN_STORE generate 64- or 32-bit load/store for ppc64 or ppc32 > + respectively. They are primary used for save/restore GPRs in jump-pad, > + not used for bytecode compiling. */ > + > +#if defined __powerpc64__ > +#define GEN_LOAD(buf, rt, ra, si) (is_64 ? GEN_LD (buf, rt, ra, si) : \ > + GEN_LWZ (buf, rt, ra, si)) > +#define GEN_STORE(buf, rt, ra, si) (is_64 ? GEN_STD (buf, rt, ra, si) : \ > + GEN_STW (buf, rt, ra, si)) Huh, using the is_64 variable from the surrounding routine without passing it in as macro argument is probably not the best idea :-) > +#if _CALL_ELF == 2 > + /* XXX is there a way to get this dynamically from the inferior? */ > + int is_opd = 0; See above. Otherwise, this looks good to me. Thanks, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com