From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49791 invoked by alias); 8 Feb 2016 13:05:00 -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 49775 invoked by uid 89); 8 Feb 2016 13:05:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 spammy=he's, advised, his X-HELO: usplmg21.ericsson.net Received: from usplmg21.ericsson.net (HELO usplmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 08 Feb 2016 13:04:59 +0000 Received: from EUSAAHC005.ericsson.se (Unknown_Domain [147.117.188.87]) by usplmg21.ericsson.net (Symantec Mail Security) with SMTP id 83.27.32102.26298B65; Mon, 8 Feb 2016 14:04:35 +0100 (CET) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.89) with Microsoft SMTP Server id 14.3.248.2; Mon, 8 Feb 2016 08:04:56 -0500 Subject: Re: [PATCH] gdb.trace: Remove struct tracepoint_action_ops. To: , =?UTF-8?Q?Marcin_Ko=c5=9bcielnicki?= , Pedro Alves References: <1453577516-19252-1-git-send-email-koriakin@0x04.net> <56A60CD3.6060704@redhat.com> <56AB3B6B.3050509@0x04.net> <56B546EB.5060201@0x04.net> From: Antoine Tremblay Message-ID: <56B89277.9000903@ericsson.com> Date: Mon, 08 Feb 2016 13: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: <56B546EB.5060201@0x04.net> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00192.txt.bz2 On 02/05/2016 08:05 PM, Marcin Ko=C5=9Bcielnicki wrote: > On 29/01/16 11:14, Marcin Ko=C5=9Bcielnicki wrote: >> On 25/01/16 12:53, Pedro Alves wrote: >>> On 01/23/2016 07:31 PM, Marcin Ko=C5=9Bcielnicki 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) >>>> =3D=3D 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 =3D=3D 32 >>>> >>>> and its layout in IPA: >>>> >>>> 0: base.type >>>> 8-15: addr >>>> 16-23: len >>>> 24-27: basereg >>>> sizeof =3D=3D 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=C5=9Bcielnicki >> > > > Ping. I have a similar situation and was advised that I could commit someone=20 else patch if he's in the Commit After Approval list, which Wei-cheng is. You can keep the author of the patch to be the original author in the=20 commit and have his name in the ChangeLog. (git commit -a=20 --author=3D if you need too) If the patch needs some changes and you're willing to do them, you can=20 do the same procedure as above and add your name to the ChangeLog. Providing you've submitted the changes to the list and that they are=20 approved of course... Regards, Antoine