[PATCH] D81717: [llvm/Object] Reimplment basic_symbol_iterator in TapiFile
Jonas Devlieghere via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 11 23:57:44 PDT 2020
JDevlieghere created this revision.
JDevlieghere added a reviewer: cishida.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.
cishida accepted this revision.
cishida added a comment.
This revision is now accepted and ready to land.
Thanks for fixing this! one nit but LGTM
================
Comment at: llvm/lib/Object/TapiFile.cpp:81
Error TapiFile::printSymbolName(raw_ostream &OS, DataRefImpl DRI) const {
- const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
- OS << Sym->Prefix << Sym->Name;
+ assert(DRI.d.a < Symbols.size());
+ const Symbol &Sym = Symbols[DRI.d.a];
----------------
nit: maybe add a message to notify an out of bounds access?
================
Comment at: llvm/lib/Object/TapiFile.cpp:88
Expected<uint32_t> TapiFile::getSymbolFlags(DataRefImpl DRI) const {
- const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
- return Sym->Flags;
+ assert(DRI.d.a < Symbols.size());
+ return Symbols[DRI.d.a].Flags;
----------------
same here
Use indices into the Symbols vector instead of dereferencing `std::vector::end()`.
NFC modulo the Windows failure: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/24024
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81717
Files:
llvm/lib/Object/TapiFile.cpp
Index: llvm/lib/Object/TapiFile.cpp
===================================================================
--- llvm/lib/Object/TapiFile.cpp
+++ llvm/lib/Object/TapiFile.cpp
@@ -75,30 +75,28 @@
TapiFile::~TapiFile() = default;
-void TapiFile::moveSymbolNext(DataRefImpl &DRI) const {
- const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
- DRI.p = reinterpret_cast<uintptr_t>(++Sym);
-}
+void TapiFile::moveSymbolNext(DataRefImpl &DRI) const { DRI.d.a++; }
Error TapiFile::printSymbolName(raw_ostream &OS, DataRefImpl DRI) const {
- const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
- OS << Sym->Prefix << Sym->Name;
+ assert(DRI.d.a < Symbols.size());
+ const Symbol &Sym = Symbols[DRI.d.a];
+ OS << Sym.Prefix << Sym.Name;
return Error::success();
}
Expected<uint32_t> TapiFile::getSymbolFlags(DataRefImpl DRI) const {
- const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
- return Sym->Flags;
+ assert(DRI.d.a < Symbols.size());
+ return Symbols[DRI.d.a].Flags;
}
basic_symbol_iterator TapiFile::symbol_begin() const {
DataRefImpl DRI;
- DRI.p = reinterpret_cast<uintptr_t>(&*Symbols.begin());
+ DRI.d.a = 0;
return BasicSymbolRef{DRI, this};
}
basic_symbol_iterator TapiFile::symbol_end() const {
DataRefImpl DRI;
- DRI.p = reinterpret_cast<uintptr_t>(&*Symbols.end());
+ DRI.d.a = Symbols.size();
return BasicSymbolRef{DRI, this};
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81717.270310.patch
Type: text/x-patch
Size: 1408 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200612/f9c43223/attachment.bin>
More information about the llvm-commits
mailing list