[PATCH] D21503: [llvm-dwarfdump] - Teach dwarfdump to dump gdb-index section.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 11 06:49:24 PDT 2016


grimar added inline comments.

================
Comment at: lib/DebugInfo/DWARF/DWARFGdbIndex.cpp:70
@@ +69,3 @@
+  uint32_t I = 0;
+  for (const std::pair<uint32_t, std::vector<uint32_t>> &V :
+       ConstantPoolVectors) {
----------------
dblaikie wrote:
> consider 'auto' here?
Done.

================
Comment at: lib/DebugInfo/DWARF/DWARFGdbIndex.cpp:80
@@ +79,3 @@
+void DWARFGdbIndex::dump(raw_ostream &OS) const {
+  if (!ErrMessage.empty()) {
+    OS << ErrMessage;
----------------
dblaikie wrote:
> Rather than using this ErrMessage string, could the parse function return an llvm::Error? (I can't remember if I've plumbed through any of that in such places yet)
Looks it a bit more common to use report_fatal_error in dwarfdump tool. So I used that instead.

================
Comment at: lib/DebugInfo/DWARF/DWARFGdbIndex.cpp:147
@@ +146,3 @@
+  uint32_t CuVectorsTotal = 0;
+  while (SymTableSize--) {
+    uint32_t NameOffset = Data.getU32(&Offset);
----------------
dblaikie wrote:
> I'd suggest writing this (& other similar) loop as a normal for loop - more obvious what it's doing, and that there's no intended/needed side effect of changing the value of SymTableSize.
Done.


http://reviews.llvm.org/D21503





More information about the llvm-commits mailing list