Attempt to fix issue #1829 (dangling pointers after replacing globals)

Postpone the destruction of replaced global variables, register them in
`IRState::globalsToReplace` and perform a second replacement pass when
finalizing the IR module. The globals will be destroyed in that 2nd pass,
so there may still be dangling pointers in `IrGlobal::value`, but now only
after writing the module.

The nicer thing would be replacing IrGlobal::value by IrGlobal::getValue()
which could look it up in IRState, but that's not feasible due to the
field being inherited from IrVar and IrGlobal apparently quite often being
used as IrVar (via pointers union), so the cached field is currently
required.

So the feasible options to me were either registering each IrGlobal with
its mangled name and replacing their values when swapping out the global
variable, or performing this second replacement pass right before
finalizing the IR module to make sure we really replace all occurrences.
I went for the latter as it's both easier and safer (from the LL
perspective as the old global might be cached somewhere else too).
This commit is contained in:
Martin 2017-06-18 00:57:43 +02:00
parent 6eb62236d4
commit ea0167d652
4 changed files with 32 additions and 7 deletions

View file

@ -216,6 +216,10 @@ void CodeGenerator::finishLLModule(Module *m) {
}
void CodeGenerator::writeAndFreeLLModule(const char *filename) {
// Issue #1829: make sure all replaced global variables are replaced
// everywhere.
ir_->replaceGlobals();
ir_->DBuilder.Finalize();
emitLLVMUsedArray(*ir_);

View file

@ -282,15 +282,15 @@ public:
LLConstant *initVal =
DtoConstInitializer(decl->loc, decl->type, decl->_init);
setLinkage(lwc, gvar);
// In case of type mismatch, swap out the variable.
irGlobal->value = irs->setGlobalVarInitializer(gvar, initVal);
// Now, set the initializer.
// Cache it.
assert(!irGlobal->constInit);
irGlobal->constInit = initVal;
// Set the initializer, swapping out the variable if the types do not
// match.
setLinkage(lwc, gvar);
irGlobal->value = irs->setGlobalVarInitializer(gvar, initVal);
// Also set up the debug info.
irs->DBuilder.EmitGlobalVariable(gvar, decl);
}

View file

@ -167,7 +167,9 @@ LLConstant *IRState::setGlobalVarInitializer(LLGlobalVariable *&globalVar,
// Replace all existing uses of globalVar by the bitcast pointer.
auto castHelperVar = DtoBitCast(globalHelperVar, globalVar->getType());
globalVar->replaceAllUsesWith(castHelperVar);
globalVar->eraseFromParent();
// Register replacement for later occurrences of the original globalVar.
globalsToReplace.emplace_back(globalVar, castHelperVar);
// Reset globalVar to the helper variable.
globalVar = globalHelperVar;
@ -175,6 +177,15 @@ LLConstant *IRState::setGlobalVarInitializer(LLGlobalVariable *&globalVar,
return castHelperVar;
}
void IRState::replaceGlobals() {
for (const auto &pair : globalsToReplace) {
pair.first->replaceAllUsesWith(pair.second);
pair.first->eraseFromParent();
}
globalsToReplace.resize(0);
}
////////////////////////////////////////////////////////////////////////////////
IRBuilder<> *IRBuilderHelper::operator->() {

View file

@ -107,6 +107,11 @@ struct IRAsmBlock {
// represents the module
struct IRState {
private:
std::vector<std::pair<llvm::GlobalVariable *, llvm::Constant *>>
globalsToReplace;
public:
IRState(const char *name, llvm::LLVMContext &context);
~IRState();
@ -209,6 +214,11 @@ struct IRState {
llvm::Constant *setGlobalVarInitializer(llvm::GlobalVariable *&globalVar,
llvm::Constant *initializer);
// To be called when finalizing the IR module in order to perform a second
// replacement pass for global variables replaced (and registered) by
// setGlobalVarInitializer().
void replaceGlobals();
/// Vector of options passed to the linker as metadata in object file.
#if LDC_LLVM_VER >= 306
llvm::SmallVector<llvm::Metadata *, 5> LinkerMetadataArgs;