[PATCH] D23621: llvm-objdump: ELF: Handle code and data mix in all scenarios

[Saleem Abdulrasool via llvm-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

LGTM with at least the cast on the use of format to avoid cross-endian issues.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1315
@@ +1314,3 @@
+        // function, it is denoted as a word/short etc
+        if (isArmElf(Obj) && std::get<2>(Symbols[si]) != ELF::STT_OBJECT &&
+            !DisassembleAll) {
----------------
Is there a real reason to limit this to ARM ELF?  Shouldn't all ELF have this behavior?

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1332
@@ +1331,3 @@
+                if (Obj->isLittleEndian()) {
+                  const support::ulittle32_t *Word =
+                      reinterpret_cast<const support::ulittle32_t *>(
----------------
auto would be nicer since the cast spells out the type.

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1342
@@ -1314,1 +1341,3 @@
+                }
+                outs() << "0x" << format("%08" PRIx32, Data);
               } else if (Index + 2 <= End) {
----------------
Add an explicit cast to unsigned before passing this through format, otherwise you will be breaking cross-endianness.

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1349
@@ +1348,3 @@
+                if (Obj->isLittleEndian()) {
+                  const support::ulittle16_t *Word =
+                      reinterpret_cast<const support::ulittle16_t *>(
----------------
auto would be nicer since the cast spells out the large type.


Repository:
  rL LLVM

https://reviews.llvm.org/D23621