From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id 61E523858D28 for ; Wed, 1 Dec 2021 09:24:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 61E523858D28 Received: by mail-wm1-x335.google.com with SMTP id g191-20020a1c9dc8000000b0032fbf912885so625997wme.4 for ; Wed, 01 Dec 2021 01:24:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=1OOd3t01rQPuomqM2oY6u2Dri7RAoNuMssHO7fHgMQ8=; b=foxwrm/rZGvKKKvo3fYrhe27eyuSTlNnX/XOZp/9hbFN4rxwdgHs93P/WxolodsHCl suz/8V5jkMNKRXhfhZa9e5l75CPL94/oLPMlrj94LcGEOy3cucQq1uZZ93OCG3gsv7Fz fofbTwatASZReaQ1vhY6/8jSQlVesjgTp7dqxalPv4LcNr4JSodphb8DFYCqVaelysez Ya1AEmjOkLgcr45MP143fkwwkQOSoe++BPC9Od6zRDdNfV7lYu+qywqoTNhK/mOeTaHS o3M+RdCk+x5VqtDoexYzFZqZv3WW3/Rc3NRfR9SwajqFxtoFPAqyAAldr8t16yL849g2 AQ1g== X-Gm-Message-State: AOAM532eVMkgwOcDhz3ZhLpwLOsFBYQn5xun4jwsbiuyDBSz0qXOQ7Iu nX4bm6LstgHT5cR8KAqOL2HPym3OLD9viyU= X-Google-Smtp-Source: ABdhPJxj93sat1HO0saC6ppWv06D3rWGXTCOKZ4AyUlk0ZvF5/G1fLOPx9qPA7qTnyxh9FR5oS0e2g== X-Received: by 2002:a7b:c30e:: with SMTP id k14mr5440440wmj.156.1638350640374; Wed, 01 Dec 2021 01:24:00 -0800 (PST) Received: from takamaka.home ([2a01:cb22:1d5:1100:fe27:8dcf:c00b:474c]) by smtp.gmail.com with ESMTPSA id r83sm402826wma.22.2021.12.01.01.23.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Dec 2021 01:24:00 -0800 (PST) Received: by takamaka.home (Postfix, from userid 1000) id 8624EA5A31; Wed, 1 Dec 2021 13:23:57 +0400 (+04) Date: Wed, 1 Dec 2021 13:23:57 +0400 From: Joel Brobecker To: Jan Vrany Cc: "gdb-patches@sourceware.org" , Joel Brobecker Subject: Re: [PATCH v2 1/2] ppc: use 'trap' ('tw, 31, 0, 0', 0x7fe00008) as breakpoint instruction Message-ID: References: <20211123154237.2335848-1-jan.vrany@labware.com> <20211124130926.2412617-1-jan.vrany@labware.com> <20211124130926.2412617-2-jan.vrany@labware.com> <08be85df9760d969375f31febb1df71c0936384e.camel@labware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <08be85df9760d969375f31febb1df71c0936384e.camel@labware.com> X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Dec 2021 09:24:02 -0000 Hi Jan, > > >  typedef BP_MANIPULATION_ENDIAN (little_breakpoint, big_breakpoint) > > >    rs6000_breakpoint; > > > > I'm going to say OK, but I'd like to know what your source is > > for this, so we can document that change. Part of me wonders > > why we're making a change like this one on some code which > > works, which is always a risk, no matter how small, unless we have > > enough evidence that this is going to be helpful in practice. > > According to Power ISA 3.0 B spec [1], section 3.3.11 Fixed-Point Trap > Instructions (also section C.6 Trap Mnemonics), unconditional trap > is encoded as `tw 31, 0, 0`. While debugging a problem on POWER > (gdb not stopping on a `trap` instruction) I found that GDB is using > `tdnl r2, r2` (trap if not less than) which works but it is different > than unconditional trap as specified. I could not find out why > `tdnl` was used instead of unconditional `trap`. I thought it is  > more "natural" to use canonical unconditional trap. So this is merely > a "cosmetic" change. > > I agree that I should have included reference to the spec in the > commit message and maybe also add comment to the code. Will do in > next version (should we decide to keep this patch). > > Now, I do understand the risk of changing something that works. I'm > happy to drop this patch since the next one in this miniseries  > fixes the problem we were experiencing properly (that is, GDB not > stopping on software-planted breakpoint instructions). > > What do you think? With the addition of the reference in the commit message, I'd say let's make the change. I don't think the risk is very high, and I don't want an issue that's most likely non-existant to cause enough FUD that we'd ignore a patch that looks correct. Worse case scenario, we revert! -- Joel