From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80487 invoked by alias); 4 Mar 2015 17:13:50 -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 80455 invoked by uid 89); 4 Mar 2015 17:13:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 04 Mar 2015 17:13:47 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t24HDd1b021350 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 4 Mar 2015 12:13:40 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t24HDbiw029393; Wed, 4 Mar 2015 12:13:38 -0500 Message-ID: <54F73D41.9020109@redhat.com> Date: Wed, 04 Mar 2015 17:13:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Ulrich Weigand , Wei-cheng Wang CC: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Fast tracepoint for powerpc64le References: <201502271952.t1RJqq7X018591@d03av02.boulder.ibm.com> In-Reply-To: <201502271952.t1RJqq7X018591@d03av02.boulder.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-03/txt/msg00130.txt.bz2 On 02/27/2015 07:52 PM, Ulrich Weigand wrote: >> The main reason why PowerPC64 big-endian doesn't work is >> calling convention (function descriptors) issue. >> When installing a tracepoint in inferior memory, gdbserver >> asks the address of "gdb_collect" (and etc.) using qSymbol packet, >> and it generate a sequence of instructions to calling that address. >> However, gdb-client "return the start of code instead of >> any data function descriptor." >> See commenting in remote_check_symbols/remote.c, >> https://sourceware.org/ml/gdb-patches/2007-06/msg00389.html >> and gen_call() in this patch. >> In order for powerpc64be to work, qSymbol packet should be >> extend for function descriptors. > So I guess there's two ways to fix this. One would be to change > gdbserver to work more like GDB here. This would involve removing > the descriptor->code address conversion in remote.c, and instead > performing the conversion in gdbserver's thread_db_enable_reporting. > Now, there is no gdbarch_convert_from_func_ptr_addr in gdbserver, > so a similar mechanism would have to be invented there. (I guess > this would mean a new target hook.) Fortunately, the only platform > that uses function descriptors *and* supports libthread_db debugging > in gdbserver is ppc64-linux, so we'd only have to add that new > mechanim on this platform. Note sure about this one, ppc64_convert_from_func_ptr_addr wants to get at the bfd/binary's unrelocated sections. We'd have to teach gdbserver to read the binary. > > This has the advantage that qSymbol could now be used to lookup > function symbols and get the descriptor address as expected. > On the other hand, this would mean an incompatible change in the > remote protocol: if you used a new GDB together with an old > gdbserver (or vice versa), thread debugging would stop working. > However, I guess that could be fixed by having gdbserver request > the new behavior from GDB by specifying a feature code. With old > GDBs gdbserver would have to skip the descriptor->code conversion. > > > The second alternative would be to extend qSymbol to support > returning two different types of addresses for function symbols: > the symbol value (i.e. function pointer value, i.e. descriptor > on PPC64), and a code address suitable to set a breakpoint on > function entry. This could be either by having gdbserver > request one or the other via an additional flag on the qSymbol > request, or else by GDB simply always returning both values > in two fields. Again, this would be an incompatible protocol > change that would need to be guarded by a qFeature check. > > In this case, gdbserver would use the "normal" symbol values > for most purposes (e.g. tracepoint routine lookup), but would > use the "code address" values to return from ps_pglobal_lookup. > With old GDBs, gdbserver would disable fast tracepoint support > on powerpc64. > > If we do that, then for consistency it might also be useful > to pass code addresses to ps_pglobal_lookup in GDB itself. > In addition, the "code address" could be changed to skip > the local entry point prolog on powerpc64le to ensure that > the breakpoint is set correctly. (This does not matter in > practice since __nptl_create_event has no local entry point, > but it would seem more fully correct in general.) > > > Overall, the second alternative seems a bit better to me, > but I'd certainly appreciate feedback on this ... I inclined for the second alternative as well. (Note for testing: __nptl_create_event will only be used on old kernels without PTRACE_EVENT_CLONE, unless you hack the code to force usage.) > > >> For powerpc32 to work, some data structure/function in tracepoint.c >> need to be fixed. For example, >> >> * write_inferior_data_ptr should be fixed for big-endian. >> If sizeof (CORE_ADDR) is larger than sizeof (void*), zeros are written. >> BTW, I thnink write_inferior_data_pointer provides the same functionality >> without this issue. I'm not sure why write_inferior_data_ptr is needed? > > This is odd, I don't see the point of this either. Yeah, probably I just missed merging them fully while cleaning up the initial contribution... I agree we should just use write_inferior_data_pointer. >> * Data structure layout between gdbserver and IPA is not consistent. >> >> There are two versions of tracepoint_action one for gdbserver, >> and antoher for inferior (IPA side). >> ... > Ugh. That's a strange construct, and extremely dependent on alignment > rules (as you noticed). I'm not really sure what the best way to fix > this would be. My preference right now would be get rid of "ops" on > the gdbserver side too, and just switch on "type" in the two places > where the ops->send and ops->download routines are called right now. > > This makes the data structures the same on gdbserver and IPA, which > simplifies downloading quite a bit. (Also, it keeps the data structure > identical in IPA, which should avoid compatibility issues between > versions.) That sounds fine. Thanks, Pedro Alves