From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115288 invoked by alias); 29 Jan 2016 10:14:09 -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 115279 invoked by uid 89); 29 Jan 2016 10:14:08 -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=2427, 815, Hx-languages-length:2482, 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; Fri, 29 Jan 2016 10:14:07 +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 21F2A3FE5B; Fri, 29 Jan 2016 11:14:51 +0100 (CET) Received: from [192.168.43.80] (public-gprs357731.centertel.pl [37.47.28.164]) by hogfather.0x04.net (Postfix) with ESMTPSA id B793358008A; Fri, 29 Jan 2016 11:14:04 +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> From: =?UTF-8?Q?Marcin_Ko=c5=9bcielnicki?= Message-ID: <56AB3B6B.3050509@0x04.net> Date: Fri, 29 Jan 2016 10:14: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: <56A60CD3.6060704@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg00717.txt.bz2 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