[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:1107
@@ -1100,2 +1106,3 @@
+      SectionSymbolsTy;
   std::map<SectionRef, SectionSymbolsTy> AllSymbols;
   for (const SymbolRef &Symbol : Obj->symbols()) {
----------------
compnerd wrote:
> IIRC the Expected wrappers are not as intrusive as the Error wrappers.  So, it may end up being reasonable still.  I would agree that this would go beyond what is needed here, and is possibly a good follow up cleanup.
> 
> However, would you mind using a typedef (or the C++11 equivalent with using) for the tuple type?  Even better would be to have a private enumeration so we can semi-symbolically expand the tuple.
Looks like this is a moot point now. Please check this commit: https://reviews.llvm.org/rL278921

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1313
@@ +1312,3 @@
+                         Bytes.slice(Index, 4)[2] << 8 |
+                         Bytes.slice(Index, 4)[3];
+                outs() << "0x" << format("%08" PRIx32, Data);
----------------
compnerd wrote:
> I wonder if we can use `llvm::ulittle32_t` to translate the value.  It would require a specific cast going through format, but would be easier to understand.
Good point. Will do this.


Repository:
  rL LLVM

https://reviews.llvm.org/D23621