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

[Saleem Abdulrasool via llvm-commits]

compnerd added inline comments.

================
Comment at: test/tools/llvm-objdump/X86/disassemble-code-data-mix.test:15
@@ +14,3 @@
+        .globl bar
+        .type bar, at object
+bar:
----------------
Please uniformly choose a style (space or no space between the comma and type specifier).

================
Comment at: test/tools/llvm-objdump/X86/disassemble-code-data-mix.test:18
@@ +17,3 @@
+        .string "test string"
+        .size bar, .-bar
+// CHECK: b:        74 65 73 74 20 73 74 72         test str
----------------
Are the sizes really needed?

================
Comment at: test/tools/llvm-objdump/X86/disassemble-code-data-mix.test:19
@@ +18,3 @@
+        .size bar, .-bar
+// CHECK: b:        74 65 73 74 20 73 74 72         test str
+// CHECK-NEXT:  13:        69 6e 67 00                     ing.
----------------
Whitespace is ignored, so it makes it easier to read if you align the address labels.

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1105
@@ +1104,3 @@
+  // data.
+  // This is used to pretty print the symbols while disassembling.
+  typedef std::vector<std::tuple<uint64_t, StringRef, SymbolRef::Type>>
----------------
Join this and the previous line?

================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:1108
@@ -1100,2 +1107,3 @@
+      SectionSymbolsTy;
   std::map<SectionRef, SectionSymbolsTy> AllSymbols;
   for (const SymbolRef &Symbol : Obj->symbols()) {
----------------
I wonder how much more expensive it is to keep a `std::vector<SymbolRef>` instead and query the Address/Name/Type from the symbol.


Repository:
  rL LLVM

https://reviews.llvm.org/D23621