chore: Add unit tests for arm64 disassembler - #3245
Conversation
AI review
Accumulator bugs
Tests that can't fail
Minor
It looks like it found some possible bugs in the accumulator (I did not verify myself). Fine if you want to fix them here, or defer for out-of-scope. |
7aa8d2b to
52dc08c
Compare
This PR intended to add tests to verify existing arm64 disassembler behaviors.
|
|
Findings are all in the new test code; the production changes look good (the formatter padding is a real fix — a mnemonic reaching
Reviewed with Claude Code. |
84d7a22 to
a01d20d
Compare
a01d20d to
2fdfa41
Compare
2fdfa41 to
025c00a
Compare
timcassell
left a comment
There was a problem hiding this comment.
Follow-up on the updated head. The fixes from the last round all look right — the macOS-arm64 address, WithStrictOrdering, the duplicate MOVZ row, DmbIshLdInstr, and the Decode.cs comments. A few items are still open, plus one new one and one follow-up note.
Reviewed with Claude Code.
…rMethod is called
timcassell
left a comment
There was a problem hiding this comment.
Another pass on 7124b6c3a. AddFalsePointer and the absolute-address assertion in TryGetReferencedAddress_With_BranchRelative are both good additions. Remaining items below — the UnreachableException visibility one is the only one that affects shipped code.
Reviewed with Claude Code.
timcassell
left a comment
There was a problem hiding this comment.
Pass on edc10f573 — all eight items from the last review are addressed, and the UnreachableException visibility/guard fix is exactly right. Seven remaining, severity-tagged; the two mediums are both tests that pass for the wrong reason.
Reviewed with Claude Code.
This PR contains following changes.
1. Cleanup arm64 disassembler related code to preparing to add unit tests
See following PR comment for details.
2. Add AsmArm64 package reference
AsmArm64 package to unit test project.
Currently it's used for test purpose.
It's expected existing arm64 disassembler is replaced to
AsmArm64based implementation. (#3246)3. Add arm64 disassembler related unit tests.
To ensure existing arm64 disassembler behavior.
Unit test codes are added for major code paths. (It can confirm code coverage results with
Analyze Code Coverageon VS)Note:
Almost of unit tests on .NET Framework are excluded by
#if NETdirective.UnsafeAccessor(It requires .NET 8 or later)Libraryproject)