For 3.1 Backport #25958 Modify amd64walker to use table based decode

[sdmaclea]

Fixes dotnet/runtime#34109

This has already been fixed in .NET 5 (see #25958). This PR ports that fix to release/3.1.

Customer Impact

Setting a debugger breakpoint on Amd64 (x64) can result in incorrect calculation of floating point values.

Regression?

This was a known and difficult to fix issue. The hand written Amd64 instruction parser was flaky and nearly impossible to maintain.

The problem arose because the debugger must determine the length and form of the instruction where the breakpoint is inserted. The original code was a minimal implementation which only covered the instruction form historically used by the JIT. As new forms and especially intrinsics were added, the code was not updated because the complexity was far too high.

We completely missed decode of all x64 simd instruction forms.

The fix required was this substantial change. At the time we missed the 3.0 branch and we did not have sufficient feedback to consider backport to 3.x.

Testing

• This code was manually tested to fix the originally reported issue.
  • Additionally the same code was tested for regressions in other areas.
    • Trying a breakpoint on every line of code
    • Stepping through every line of code
• The code has been in the 5.0 branch for more than 6 months with no new regressions
  • Shipped in all .NET Core 5.0 previews to date
  • Over the last 90 days, there have been
    • 184K debugging sessions launched on .NET Core 5 previews
    • from 3.3K unique MAC addresses
  • We have no reports of issues linked to this fix
• The 5.0 code has been tested against recently reported similar issues
  • We have several confirmation that this issue observed in 3.0/3.1 has been fixed by 5.0
• The internal diagnostic test suite was amended to test for this issue and prevent further identical regressions.
• @tommcdon & @hoyosjs have each used the x64 debugger since this change for debugging various reported/discovered issues. They saw no issues suggesting a regression.

Risk

Low.

• Modify amd64walker to use table based decode #25958 has been in 5.0 since 9/2019
• Only affects x64 debugger operation.
• Removes a flaky instruction parser and replaces it with a machine generated one.
  • Rewrites one function NativeWalker::DecodeInstructionForPatchSkip(). Used only for debugging x64 code.
  • Adds 1 new machine generated instruction decode table. (10k lines)
• Adds tools to regenerate the parser tables
  • Add 3 files to archive the tool used to generate the decode table.to machine generate. (Only used in development)
• Adds documentation (1 file)
• Sufficient testing coverage

Code Reviewers

@hoyosjs @tannergooding @noahfalk @jkotas

[tannergooding]

LGTM the additional +43/-43 changes (as compared to #25958) are just stripping additional whitespace from the end of lines in src/debug/ee/amd64/amd64walker.cpp

[hoyosjs]

Also verified the customer scenario reported gets fixed.

[jeffschwMSFT]

Will consider again in a few months

[tommcdon]

Adding @noahfalk @jkotas for additional review

[noahfalk]

I'm taking a look at the change but one question that rises to the top - do we have telemetry telling us how much usage we've gotten on this code path in .Net 5? For example can we determine the number of VS debugging sessions that have occurred with a .Net 5 x64 runtime? My fear is that we'd make an assumption that .Net 5 has given the scenario lots of coverage and the lack of feedback implies the change has no issues when in practice the code had been used relatively little and there are still bugs lurking. It seems unlikely that we'd catch an issue in this change via code review.

[noahfalk]

A basic pass through revealed no obvious issues, but again I wouldn't expect to be able to spot anything without spending considerable time studying the X64 instruction encoding reference documentation and then doing a detailed comparison between the spec and this code. If we can demonstrate considerable usage has already occurred in .Net 5 that would bring down the risk.

[sdmaclea]

Over the last 90 days, there have been

• 184K debugging sessions launched on .NET Core 5
• from 3.3K unique MAC addresses
• All of these build had the 5.0 version of this PR.

[Anipik]

@tannergooding @hoyosjs @jkotas can you review and approve the change after the latest commits ?