[PATCH] D89241: [RTDYLD] be more defensive about relocations with empty symbol names
Valentin Churavy via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 12 07:55:59 PDT 2020
vchuravy created this revision.
vchuravy added a reviewer: lhames.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
vchuravy requested review of this revision.
I am unsure if we should add these asserts, so I am putting this up for discussions.
The impetus for this was https://github.com/JuliaLang/julia/pull/37842#issuecomment-706651005
IIUC we map many relocations to the symbol name `""`. (See https://github.com/llvm/llvm-project/blame/176249bd6732a8044d457092ed932768724a6f06/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp#L1199)
and https://github.com/llvm/llvm-project/blob/176249bd6732a8044d457092ed932768724a6f06/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp#L929 looks at `GlobalSymbolTable` to determine whether or not
this is an absolute relocation. If someone happens to insert a symbol that has a `""` as the name, all the absolute relocations will now be treated wrong.
So my question is: We are using `""` as a sentinel value for absolute relocations. Should we ensure that actually is a sentinel value, or is there a platform were `""` is a valid symbol name,
and we should use a different sentinel, like the original `NULL` pointer?
For JuliaLang I am currently filterting out the wrong relocation, after I was unable to find the producer of it.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89241
Files:
llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
Index: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
===================================================================
--- llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
+++ llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
@@ -308,6 +308,7 @@
<< " SID: " << SectionID
<< " Offset: " << format("%p", (uintptr_t)Addr)
<< " flags: " << *FlagsOrErr << "\n");
+ assert(!Name.empty()); // Should not enter an empty symbol
GlobalSymbolTable[Name] = SymbolTableEntry(SectionID, Addr, *JITSymFlags);
} else if (SymType == object::SymbolRef::ST_Function ||
SymType == object::SymbolRef::ST_Data ||
@@ -340,6 +341,7 @@
<< " SID: " << SectionID
<< " Offset: " << format("%p", (uintptr_t)SectOffset)
<< " flags: " << *FlagsOrErr << "\n");
+ assert(!Name.empty()); // Should not enter an empty symbol
GlobalSymbolTable[Name] =
SymbolTableEntry(SectionID, SectOffset, *JITSymFlags);
}
@@ -769,6 +771,7 @@
LLVM_DEBUG(dbgs() << "Allocating common symbol " << Name << " address "
<< format("%p", Addr) << "\n");
+ assert(!Name.empty()); // Should not enter an empty symbol
GlobalSymbolTable[Name] =
SymbolTableEntry(SectionID, Offset, std::move(*JITSymFlags));
Offset += Size;
@@ -930,6 +933,7 @@
if (Loc == GlobalSymbolTable.end()) {
ExternalSymbolRelocations[SymbolName].push_back(RE);
} else {
+ assert(!SymbolName.empty() && "Empty symbol should not be in GlobalSymbolTable");
// Copy the RE since we want to modify its addend.
RelocationEntry RECopy = RE;
const auto &SymInfo = Loc->second;
@@ -1107,6 +1111,7 @@
// New entries may have been added to the relocation list.
i = ExternalSymbolRelocations.find(Name);
} else {
+ assert(!Name.empty() && "Empty symbol should not be in GlobalSymbolTable");
// We found the symbol in our global table. It was probably in a
// Module that we loaded previously.
const auto &SymInfo = Loc->second;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89241.297588.patch
Type: text/x-patch
Size: 2196 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201012/714576bb/attachment.bin>
More information about the llvm-commits
mailing list