[PATCH] D146534: [llvm-nm] Print EC symbol map.
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 21 12:43:46 PDT 2023
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
More information about the llvm-commits
mailing list