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

[khemant@codeaurora.org via llvm-commits]

khemant added inline comments.

================
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) {
----------------
compnerd wrote:
> Is there a real reason to limit this to ARM ELF?  Shouldn't all ELF have this behavior?
ARM is the only one that has these mapping symbols that mix inside the function (from beginning of a at function type and within the size of that symbol). All other architectures there needs to be an explicit data symbol (local or global) in which case the disassembler uses the second kind - the general way of interpreting that bytes are data.

================
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 *>(
----------------
compnerd wrote:
> auto would be nicer since the cast spells out the type.
Yep.

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

================
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 *>(
----------------
compnerd wrote:
> auto would be nicer since the cast spells out the large type.
Got it.


Repository:
  rL LLVM

https://reviews.llvm.org/D23621