[PATCH] D146534: [llvm-nm] Print EC symbol map.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

This code seems like it isn't being careful to ensure calls to read16le/read32le/etc. don't read past the end of a buffer. Maybe it makes sense to add some sort of bounds-checked read helper?  Maybe could leave it for a followup, though, if it's confusing to mix it in with this patch.



================
Comment at: llvm/lib/Object/Archive.cpp:951
+
+  if (Name.equals("/<ECSYMBOLS>/")) {
+    Expected<StringRef> BufOrErr = C->getBuffer();
----------------
This could use a comment explaining what it's doing... I assume it's looking for the EC symbol table, but it's not obvious how the member ordering here works.


================
Comment at: llvm/lib/Object/Archive.cpp:995
 StringRef Archive::Symbol::getName() const {
+  if (Parent->getNumberOfSymbols() <= SymbolIndex)
+    return Parent->ECSymbolTable.begin() + StringIndex;
----------------
The math with the symbol indexes all over is confusing... maybe consider adding a helper to check "does this symbol index refer to an EC symbol"?


================
Comment at: llvm/lib/Object/Archive.cpp:1104
+    // Go to one past next null.
+    t.StringIndex = Parent->ECSymbolTable.find('\0', t.StringIndex) + 1;
   } else {
----------------
What happens if find() fails?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146534/new/

https://reviews.llvm.org/D146534