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

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1073
     uint64_t Size;
-    DisAsm->getInstruction(Inst, Size, Bytes.slice(Index), SectionAddr + Index,
-                           nulls());
+    ArrayRef<uint8_t> ThisBytes = Bytes.slice(Index - SectionAddr);
+    DisAsm->getInstruction(Inst, Size, ThisBytes, Index, nulls());
----------------
scott.linder wrote:
> @simon_tatham was this change intended? There is a fix at https://reviews.llvm.org/D135430 and I wanted to ping you in case we missed something.
I see what you mean – it surely can't be sensible to call `Bytes.slice(Index - SectionAddr)` when `Index` is iterating from 0 up to `Bytes.size()`.

This code looks suspiciously like the code in the next hunk up, around line 1027 in `collectLocalBranchTargets`. It's correct //there//, because the bounds on `Index` are set differently. So I suspect I pasted the same code in both places without noticing the difference. Sorry!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130357/new/

https://reviews.llvm.org/D130357