From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22910 invoked by alias); 6 Feb 2016 01:05:52 -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 22899 invoked by uid 89); 6 Feb 2016 01:05:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=HContent-Transfer-Encoding:8bit X-HELO: xyzzy.0x04.net Received: from xyzzy.0x04.net (HELO xyzzy.0x04.net) (109.74.193.254) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 06 Feb 2016 01:05:50 +0000 Received: from hogfather.0x04.net (89-65-66-135.dynamic.chello.pl [89.65.66.135]) by xyzzy.0x04.net (Postfix) with ESMTPS id 208AA3FE5C; Sat, 6 Feb 2016 02:06:37 +0100 (CET) Received: from [192.168.0.17] (87-206-68-155.dynamic.chello.pl [87.206.68.155]) by hogfather.0x04.net (Postfix) with ESMTPSA id F1D6758008C; Sat, 6 Feb 2016 02:05:47 +0100 (CET) Subject: Re: [PATCH] gdb.trace: Remove struct tracepoint_action_ops. To: Pedro Alves , gdb-patches@sourceware.org References: <1453577516-19252-1-git-send-email-koriakin@0x04.net> <56A60CD3.6060704@redhat.com> <56AB3B6B.3050509@0x04.net> From: =?UTF-8?Q?Marcin_Ko=c5=9bcielnicki?= Message-ID: <56B546EB.5060201@0x04.net> Date: Sat, 06 Feb 2016 01:05:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56AB3B6B.3050509@0x04.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00159.txt.bz2 On 29/01/16 11:14, Marcin Kościelnicki wrote: > On 25/01/16 12:53, Pedro Alves wrote: >> On 01/23/2016 07:31 PM, Marcin Kościelnicki wrote: >>> The struct tracepoint_action has an ops field, pointing to >>> a tracepoint_action_ops structure, containing send and download ops. >>> However, this field is only present when compiled in gdbserver, and not >>> when compiled in IPA. When gdbserver is downloading tracepoint actions >>> to IPA, it skips offsetof(struct tracepoint_action, type) bytes from >>> its struct tracepoint_action, to get to the part that corresponds to >>> IPA's struct tracepoint_action. >>> >>> Unfortunately, this fails badly on ILP32 platforms where alignof(long >>> long) >>> == 8. Consider struct collect_memory_action layout in gdbserver: >>> >>> 0-3: base.ops >>> 4: base.type >>> 8-15: addr >>> 16-23: len >>> 24-27: basereg >>> sizeof == 32 >>> >>> and its layout in IPA: >>> >>> 0: base.type >>> 8-15: addr >>> 16-23: len >>> 24-27: basereg >>> sizeof == 32 >>> >>> When gdbserver tries to download it to IPA, it skips the first 4 bytes >>> (base.ops), figuring the rest will match what IPA expects - which is >>> not true, since addr is aligned to 8 bytes and will be at a different >>> relative position to base.type. >>> >>> The problem went unnoticed on the currently supported platforms, since >>> aarch64 and x86_64 have ops aligned to 8 bytes, and i386 has only 4-byte >>> alignment for long long. >>> >>> There are a few possible ways around this problem. I decided on >>> removing >>> ops altogether, since they can be easily inlined in their (only) places >>> of use - in fact allowing us share the code between 'L' and 'R'. Any >>> approach where struct tracepoint_action is different between IPA and >>> gdbserver is just asking for trouble. >>> >>> Found on s390. Tested on x86_64, s390, s390x. >> >> Hmm, this is essentially the same as: >> >> https://sourceware.org/ml/gdb-patches/2015-03/msg00995.html >> >> Right? >> >> Seems that other patch inlines things a bit less though, which offhand >> looks preferable. WDYT? >> >> Not sure what happened to that series. I thought most of it (if not all) >> had been approved already. >> >> Thanks, >> Pedro Alves >> > > So - what should I do to get these patches pushed? The original > developer seems not to be responding, but the work is mostly good, and > some patches can be pushed unchanged. What's the procedure for that? Do > I commit as myself, but with the author's name in ChangeLog? What if > the patch needs some fixes? > > Marcin Kościelnicki > Ping.