[PATCH] D22932: Implement llvm-objdump -S and -l

khemant@codeaurora.org via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 10 11:40:20 PDT 2016


khemant added a comment.

In https://reviews.llvm.org/D22932#510698, @compnerd wrote:

> I think it would be better to split the hexagon support into a separate (trivial) patch.  That would make this change only add the source and line number printing and tests for x86.  The hexagon change would then add that target as well.


Got it. I will split this into two patches.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:381
@@ +380,3 @@
+  DILineInfo OldLineInfo;
+  // Storage for the source
+  const ObjectFile *Obj;
----------------
compnerd wrote:
> What exactly do you mean by "storage for the source" here?
The comment should have been for line 385. The source files need to be opened and displayed when a line info is valid. The files may have to live till end of disassembly since using linker scripts the sections may have been laid out such that source from one file may need to be displayed at any point during disassembly.

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:398
@@ +397,3 @@
+  virtual void printSourceLine(raw_ostream &OS, uint64_t Address,
+                               StringRef Delimiter = ";;; ");
+};
----------------
compnerd wrote:
> Why triple semicolon?  IIRC, the binutils interleaving uses '; '.
Sure a single "; " can be added. 

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1304
@@ +1303,3 @@
+        bool Disassembled = DisAsm->getInstruction(
+            Inst, Size, Bytes.slice(Index), SectionAddr + Index, DebugOut,
+            CommentStream);
----------------
compnerd wrote:
> Weird, why did this get reflowed?
Somehow clang format in one of the intermediate commits I squashed for this change may have triggered this.


Repository:
  rL LLVM

https://reviews.llvm.org/D22932





More information about the llvm-commits mailing list