[PATCH] D130357: [MC,llvm-objdump,ARM] Target-dependent disassembly resync policy.

Simon Tatham via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 22 07:17:30 PDT 2022


simon_tatham created this revision.
simon_tatham added reviewers: MaskRay, ostannard, DavidSpickett.
Herald added subscribers: StephenFan, rupprecht, hiraditya, kristof.beyls, emaste.
Herald added a reviewer: jhenderson.
Herald added a project: All.
simon_tatham requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Currently, when llvm-objdump is disassembling a code section and
encounters a point where no instruction can be decoded, it uses the
same policy on all targets: consume one byte of the section, emit it
as "<unknown>", and try disassembling from the next byte position.

On an architecture where instructions are always 4 bytes long and
4-byte aligned, this makes no sense at all. If a 4-byte word cannot be
decoded as an instruction, then the next place that a valid
instruction could //possibly// be found is 4 bytes further on.
Disassembling from a misaligned address can't possibly produce
anything that the code generator intended, or that the CPU would even
attempt to execute.

This patch introduces a new MCDisassembler virtual method called
`suggestBytesToSkip`, which allows each target to choose its own
resynchronization policy. For Arm (as opposed to Thumb) and AArch64,
I've filled in the new method to return a fixed width of 4.

Thumb is a more interesting case, because the criterion for
identifying 2-byte and 4-byte instruction encodings is very simple,
and doesn't require the particular instruction to be recognized. So
`suggestBytesToSkip` is also passed an ArrayRef of the bytes in
question, so that it can take that into account. The new test case
shows Thumb disassembly skipping over two unrecognized instructions,
and identifying one as 2-byte and one as 4-byte.

For targets other than Arm and AArch64, this is NFC: the base class
implementation of `suggestBytesToSkip` still returns 1, so that the
existing behavior is unchanged. Other targets can fill in their own
implementations as they see fit; I haven't attempted to choose a new
behavior for each one myself.

I've updated all the call sites of `MCDisassembler::getInstruction` in
llvm-objdump, and also one in sancov, which was the only other place I
spotted the same idiom of `if (Size == 0) Size = 1` after a call to
`getInstruction`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130357

Files:
  llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h
  llvm/lib/MC/MCDisassembler/MCDisassembler.cpp
  llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
  llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.h
  llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp
  llvm/test/tools/llvm-objdump/ELF/ARM/unknown-instr-resync.test
  llvm/test/tools/llvm-objdump/ELF/ARM/unknown-instr.test
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/sancov/sancov.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130357.446822.patch
Type: text/x-patch
Size: 10582 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220722/8d894fbc/attachment.bin>


More information about the llvm-commits mailing list