From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32147 invoked by alias); 12 Aug 2015 19:39:08 -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 32135 invoked by uid 89); 12 Aug 2015 19:39:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Aug 2015 19:39:06 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZPbra-0004yO-4O from Luis_Gustavo@mentor.com ; Wed, 12 Aug 2015 12:39:02 -0700 Received: from [172.30.12.5] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Wed, 12 Aug 2015 12:39:01 -0700 Message-ID: <55CBA0D1.5000203@codesourcery.com> Date: Wed, 12 Aug 2015 19:39:00 -0000 From: Luis Machado Reply-To: Luis Machado User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Joel Brobecker , Pedro Alves CC: Subject: Re: [PATCH v4 00/18] All-stop on top of non-stop References: <1432250354-2721-1-git-send-email-palves@redhat.com> <55C4E3BD.8040801@redhat.com> <20150812183208.GA24901@adacore.com> In-Reply-To: <20150812183208.GA24901@adacore.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00316.txt.bz2 Hi Joel, On 08/12/2015 03:32 PM, Joel Brobecker wrote: > Hi Pedro, > > On Fri, Aug 07, 2015 at 05:58:37PM +0100, Pedro Alves wrote: >> On 05/22/2015 12:18 AM, Pedro Alves wrote: >>> v3: https://sourceware.org/ml/gdb-patches/2015-04/msg00662.html >>> v2: https://sourceware.org/ml/gdb-patches/2015-04/msg00198.html >>> v1: https://sourceware.org/ml/gdb-patches/2015-04/msg00073.html >>> >>> Compared to v3, a set of minor changes accumulated, to address review >>> comments and discussions (i.e., no major functional/design changes), >>> plus fixes to a patch that touched all targets (that I noticed >>> converted a couple wrong). >>> >>> I believe I addressed all review comments thus far. Barring comments, >>> I'd like to push this in, and expose it to the build bots. >> >> FYI, I pushed this in. I'm keeping an eye on the build bots. >> >> If you spot any related problem, please confirm whether >> "maint set target-non-stop off" fixes it. > > This uncovered what I think is a latent bug in the "how-did-we-never- > see-this-before" category. Does this patch look OK to you? > > gdb/ChangeLog: > > * amd64-tdep.c (amd64_displaced_step_fixup): Fix the mask used to > compute RETADDR. > > gdb/testsuite/ChangeLog: > > * gdb.base/dso2dso-impl.c, gdb.base/dso2dso-impl.h, > gdb.base/dso2dso-pck.c, gdb.base/dso2dso-pck.h, gdb.base/dso2dso.c, > gdb.base/dso2dso.exp: New files. > > Tested on x86_64-linux, no regression. > > Thanks! > Two things about the patch. I see that the change to GDB's code is almost trivial, but the testcase looks quite involved. The first thing is that one wouldn't be able to tell what the testcase does without looking at the commit log. dso2dso doesn't particularly translate to "displaced stepping instruction masking problem on amd64". Should we change the testcase name to something a bit more meaningful? Maybe document it a bit? The second point is, should we restrict this testcase to be executed only for amd64? Maybe move it to gdb.arch? Unless you actually tried this testcase with different architectures and confirmed the testcase is sane there, it feels a bit iffy to add/execute this for non-amd64 targets. In the worst case, we could have a failure due to different instruction scheduling schemes (or maybe even a displaced stepping bug) on non-amd64 targets. At a minimum, we'd have a spurious PASS that wouldn't be meaningful for the overall test results since things never failed on said architecture in the first place. Alternatively, for such a trivial fix, shouldn't we go without a testcase? Luis