[PATCH] D20073: Make llvm-readobj symbol dumper use the RecordIterator

[Reid Kleckner via llvm-commits]

rnk requested changes to this revision.
rnk added a comment.
This revision now requires changes to proceed.

So, while llvm-readobj may not have error handling, it *does* have very careful bounds checking, and I really want to keep it. This was an important part of our refactor away from DataExtractor to StringRef / consumeObject.


================
Comment at: include/llvm/DebugInfo/CodeView/RecordIterator.h:35
@@ +34,3 @@
+    template <typename T> const T *getData() const {
+      return reinterpret_cast<const T *>(Data.data());
+    }
----------------
This also loses the bounds checking we had before.

================
Comment at: include/llvm/DebugInfo/CodeView/SymbolRecord.h:57
@@ -67,3 +56,3 @@
   uint8_t Flags; // CV_PROCFLAGS
-  // Name: The null-terminated name follows.
+  char Name[1];  // null-terminated string
 };
----------------
Please don't do this, the old code was very careful to avoid buffer overruns when the string was not null terminated. It also throws off consumeObject, which will consume one extra byte, so SymData / LeafData won't hold the full string.

================
Comment at: tools/llvm-readobj/COFFDumper.cpp:1019
@@ -1025,3 +1018,3 @@
       StringRef LinkageName;
-      StringRef DisplayName = SymData.split('\0').first;
+      StringRef DisplayName = Proc->Name;
       W.printHex("PtrParent", Proc->PtrParent);
----------------
If there is no terminating nul, split('\0') would give you all of SymData. I think that's the best behavior we can hope for.


http://reviews.llvm.org/D20073