From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id B57873858D39 for ; Tue, 28 Mar 2023 12:33:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B57873858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.170] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 27F8D1E0D2; Tue, 28 Mar 2023 08:33:54 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1680006834; bh=lB4LK0BZF2Czqgc+y/DLZQPnNbaTGDynVUrDR4CdyLc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=u59EHFm7fl5O+iO1xsfwTZspa08DlNtn04llQL4KKvrShpCuIBzhLunUC//WucGZQ zi+dJ2BMBOhkTW0n9WAPQcU4ofOGtetrdfvljhkEuim03CvldBk5aIubIoXT7OW2DT RAjWTQQt/eNkkWVrnhqdcnHnGKihLwFs5TieAjrI= Message-ID: <999a9165-8cb1-4a9a-0b3f-49f4e960f557@simark.ca> Date: Tue, 28 Mar 2023 08:33:53 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCHv3 0/3] AMD64 Displaced Stepping Fix Content-Language: fr To: Andrew Burgess , gdb-patches@sourceware.org Cc: Pedro Alves References: From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 3/27/23 08:32, Andrew Burgess via Gdb-patches wrote: > In V3: > > - Change of direction since v2. As Pedro pointed out, using the $pc > value to decide if the displaced step succeeded is not good > enough. This new version switches back to using the > target_waitstatus value. > > - The target_waitstatus is examined, and the result (a bool) is > passed to each architecture's fixup routine. > > - Just like in v2, each fixup routine is updated, with amd64 being > "fixed", while aarch64, ppc, and s390 (which aren't broken) just > have an early return case added to handle the unsuccessful > displaced step case. > > - As with v2, ARM is left broken, though this is no more broken than > it was before this patch series. > > - As Simon suggested, the debug has now moved to > displaced_step_prepare_throw, so the (not yet upstreamed) > displaced stepping for the AMD GPU target should still see the > debug output, > > - The debug code now handles a failure to disassemble better: > there's nothing worse than enabling debug to try and solve a > problem, and having GDB crash in a different way. If an > instruction fails to disassemble GDB will now print a basic debug > message and skip the rest of the debug output, > > - As suggested, I've moved the displaced_step_dump_bytes helper > function into gdbsupport/ and given it a better name, > > - I have NOT tried to implement the improvement Simon suggested > where the architecture backend tells GDB core how many bytes the > replacement instruction(s) occupied. This still means that in > some cases we will disassemble the entire displaced step buffer > unnecessarily, but I don't see that as a huge problem. Fixing > this just to reduce some debug output a little seems excessive. > Let me know if you feel this is a blocker for this work being > merged and I can take another look at it. > > Thanks, > Andrew > > --- > > Andrew Burgess (3): > gdb: more debug output for displaced stepping > gdb: move displaced_step_dump_bytes into gdbsupport (and rename) > gdb: fix reg corruption from displaced stepping on amd64 When applying the last one: Applying: gdb: fix reg corruption from displaced stepping on amd64 .git/rebase-apply/patch:591: new blank line at EOF. + .git/rebase-apply/patch:995: new blank line at EOF. + Simon