Modify amd64walker to use table based decode #25958
Conversation
sdmaclea
added
* NO MERGE *
There was a problem hiding this comment.
The odds of me finding a bug here are virtually zero given my very limited knowledge of AMD64 assembly. I think we should find a reviewer with some AMD64 assembly expertise (@CarolEidt @briansull @BruceForstall ?)
I also think it would be handy to look at what you've already done to test this and if there is anything else we should be doing. This feels like the most sizable code change we've taken to a pre-existing debugger feature in a long time and its not clear to me how likely we'd be to spot regressions with review alone.
|
So far this has fairly simplistic testing. I tested this fixed #25132. I also tested I could set a breakpoint on any line in the file without changing results. I will test this against our internal tests when I get instructions from @hoyosjs |
sdmaclea
removed
the
* NO MERGE *
Use tables for NativeWalker::DecodeInstructionForPatchSkip() Also files used to generate the decode tables in doc folder.
sdmaclea
force-pushed
the
amd64InstrDecode
branch
from
ff582c1
to
37a2819
Compare
|
@sdmaclea, is this going to be backported to 3.0 considered it results in corrupted data when debugging? |
|
We thought this was too big a change to meet the 3.0 bar. |
|
What about 3.1 for the LTS? |
* Modify amd64walker to use table based decode Use tables for NativeWalker::DecodeInstructionForPatchSkip() Also files used to generate the decode tables in doc folder.
* Modify amd64walker to use table based decode Use tables for NativeWalker::DecodeInstructionForPatchSkip() Also files used to generate the decode tables in doc folder.
…28033) * Whitespace (#25957) * Modify amd64walker to use table based decode (#25958) * Modify amd64walker to use table based decode Use tables for NativeWalker::DecodeInstructionForPatchSkip() Also files used to generate the decode tables in doc folder. * Review feedback and typos + Typos (#26968) + Feedback from 3.1 amd64InstrDecode review (dotnet/runtime#35670)
* Modify amd64walker to use table based decode Use tables for NativeWalker::DecodeInstructionForPatchSkip() Also files used to generate the decode tables in doc folder. Commit migrated from dotnet/coreclr@f82281f
Use tables for NativeWalker::DecodeInstructionForPatchSkip()
Also files used to generate the decode tables in doc folder.
Fixes #25132