[PATCH] D79284: [llvm-objdump][ARM] Print inline relocations when dumping ARM data

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Thanks for the patch! This will allow me to use `llvm-objdump` again for aarch32!



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1117
+  outs() << "\t\t.byte\t" << format_hex(Bytes[0], 4);
+  return 1;
 }
----------------
It looks like this code previously advanced the `Index`, but this version doesn't.  Intentional?


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1462-1467
+          DumpARMELFData = Kind == 'd';
+          if (SecondarySTI) {
+            if (Kind == 'a') {
+              STI = PrimaryIsThumb ? SecondarySTI : PrimarySTI;
+              DisAsm = PrimaryIsThumb ? SecondaryDisAsm : PrimaryDisAsm;
+            } else if (Kind == 't') {
----------------
Would this be simpler as a `switch (Kind)` or even `switch (getMappingSymbolKind(MappingSymbols, Index)`? Then the two cases for `'t'` and '`a'` could have the same body, and you could have another case for `'d'`.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1475-1476
+        if (DumpARMELFData) {
+          Size = dumpARMELFData(SectionAddr, Index, End, Obj, Bytes,
+                                MappingSymbols);
+        } else {
----------------
Can we not `dumpARMELFData` as soon as we see a `SectionKind` of `'d'`?  I'm curious if we could eliminate `DumpARMELFData`, or if there's some complicated case where `CheckARMELFData` could be `false`, yet `DumpARMELFData` is `true`?


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1480
+          // them. Otherwise we might want to skip zero bytes we see.
+          if (!DisassembleZeroes) {
+            uint64_t MaxOffset = End - Index;
----------------
This looks like its on the wrong side of the `else`. What happens if I disassemble with `-z` for mixed aarch32/aarch64 code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79284