Re: [PATCHv3 0/3] AMD64 Displaced Stepping Fix

[Simon Marchi <simark@simark.ca>]

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