From e57898ce5596a65579b6259e7d2e65c42a7e364e Mon Sep 17 00:00:00 2001 From: Jeffrey Yasskin Date: Thu, 4 Mar 2010 06:50:01 +0000 Subject: [PATCH] Fix PR5291, in which a SmallPtrSet iterator was held across an insertion into the set. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@97720 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/ExecutionEngine/JIT/JITEmitter.cpp | 60 ++++++++++++++--------- unittests/ExecutionEngine/JIT/JITTest.cpp | 27 ++++++++++ 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/lib/ExecutionEngine/JIT/JITEmitter.cpp b/lib/ExecutionEngine/JIT/JITEmitter.cpp index 5faf0eea403..0323a83c428 100644 --- a/lib/ExecutionEngine/JIT/JITEmitter.cpp +++ b/lib/ExecutionEngine/JIT/JITEmitter.cpp @@ -348,9 +348,6 @@ namespace { /// MMI - Machine module info for exception informations MachineModuleInfo* MMI; - // GVSet - a set to keep track of which globals have been seen - SmallPtrSet GVSet; - // CurFn - The llvm function being emitted. Only valid during // finishFunction(). const Function *CurFn; @@ -507,8 +504,14 @@ namespace { bool MayNeedFarStub); void *getPointerToGVIndirectSym(GlobalValue *V, void *Reference); unsigned addSizeOfGlobal(const GlobalVariable *GV, unsigned Size); - unsigned addSizeOfGlobalsInConstantVal(const Constant *C, unsigned Size); - unsigned addSizeOfGlobalsInInitializer(const Constant *Init, unsigned Size); + unsigned addSizeOfGlobalsInConstantVal( + const Constant *C, unsigned Size, + SmallPtrSet &SeenGlobals, + SmallVectorImpl &Worklist); + unsigned addSizeOfGlobalsInInitializer( + const Constant *Init, unsigned Size, + SmallPtrSet &SeenGlobals, + SmallVectorImpl &Worklist); unsigned GetSizeOfGlobalsInBytes(MachineFunction &MF); }; } @@ -968,11 +971,14 @@ unsigned JITEmitter::addSizeOfGlobal(const GlobalVariable *GV, unsigned Size) { } /// addSizeOfGlobalsInConstantVal - find any globals that we haven't seen yet -/// but are referenced from the constant; put them in GVSet and add their -/// size into the running total Size. +/// but are referenced from the constant; put them in SeenGlobals and the +/// Worklist, and add their size into the running total Size. -unsigned JITEmitter::addSizeOfGlobalsInConstantVal(const Constant *C, - unsigned Size) { +unsigned JITEmitter::addSizeOfGlobalsInConstantVal( + const Constant *C, + unsigned Size, + SmallPtrSet &SeenGlobals, + SmallVectorImpl &Worklist) { // If its undefined, return the garbage. if (isa(C)) return Size; @@ -994,7 +1000,7 @@ unsigned JITEmitter::addSizeOfGlobalsInConstantVal(const Constant *C, case Instruction::PtrToInt: case Instruction::IntToPtr: case Instruction::BitCast: { - Size = addSizeOfGlobalsInConstantVal(Op0, Size); + Size = addSizeOfGlobalsInConstantVal(Op0, Size, SeenGlobals, Worklist); break; } case Instruction::Add: @@ -1010,8 +1016,9 @@ unsigned JITEmitter::addSizeOfGlobalsInConstantVal(const Constant *C, case Instruction::And: case Instruction::Or: case Instruction::Xor: { - Size = addSizeOfGlobalsInConstantVal(Op0, Size); - Size = addSizeOfGlobalsInConstantVal(CE->getOperand(1), Size); + Size = addSizeOfGlobalsInConstantVal(Op0, Size, SeenGlobals, Worklist); + Size = addSizeOfGlobalsInConstantVal(CE->getOperand(1), Size, + SeenGlobals, Worklist); break; } default: { @@ -1025,8 +1032,10 @@ unsigned JITEmitter::addSizeOfGlobalsInConstantVal(const Constant *C, if (C->getType()->getTypeID() == Type::PointerTyID) if (const GlobalVariable* GV = dyn_cast(C)) - if (GVSet.insert(GV)) + if (SeenGlobals.insert(GV)) { + Worklist.push_back(GV); Size = addSizeOfGlobal(GV, Size); + } return Size; } @@ -1034,15 +1043,18 @@ unsigned JITEmitter::addSizeOfGlobalsInConstantVal(const Constant *C, /// addSizeOfGLobalsInInitializer - handle any globals that we haven't seen yet /// but are referenced from the given initializer. -unsigned JITEmitter::addSizeOfGlobalsInInitializer(const Constant *Init, - unsigned Size) { +unsigned JITEmitter::addSizeOfGlobalsInInitializer( + const Constant *Init, + unsigned Size, + SmallPtrSet &SeenGlobals, + SmallVectorImpl &Worklist) { if (!isa(Init) && !isa(Init) && !isa(Init) && !isa(Init) && !isa(Init) && Init->getType()->isFirstClassType()) - Size = addSizeOfGlobalsInConstantVal(Init, Size); + Size = addSizeOfGlobalsInConstantVal(Init, Size, SeenGlobals, Worklist); return Size; } @@ -1053,7 +1065,7 @@ unsigned JITEmitter::addSizeOfGlobalsInInitializer(const Constant *Init, unsigned JITEmitter::GetSizeOfGlobalsInBytes(MachineFunction &MF) { unsigned Size = 0; - GVSet.clear(); + SmallPtrSet SeenGlobals; for (MachineFunction::iterator MBB = MF.begin(), E = MF.end(); MBB != E; ++MBB) { @@ -1077,7 +1089,7 @@ unsigned JITEmitter::GetSizeOfGlobalsInBytes(MachineFunction &MF) { // assuming the addresses of the new globals in this module // start at 0 (or something) and adjusting them after codegen // complete. Another possibility is to grab a marker bit in GV. - if (GVSet.insert(GV)) + if (SeenGlobals.insert(GV)) // A variable as yet unseen. Add in its size. Size = addSizeOfGlobal(GV, Size); } @@ -1086,12 +1098,14 @@ unsigned JITEmitter::GetSizeOfGlobalsInBytes(MachineFunction &MF) { } DEBUG(dbgs() << "JIT: About to look through initializers\n"); // Look for more globals that are referenced only from initializers. - // GVSet.end is computed each time because the set can grow as we go. - for (SmallPtrSet::iterator I = GVSet.begin(); - I != GVSet.end(); I++) { - const GlobalVariable* GV = *I; + SmallVector Worklist( + SeenGlobals.begin(), SeenGlobals.end()); + while (!Worklist.empty()) { + const GlobalVariable* GV = Worklist.back(); + Worklist.pop_back(); if (GV->hasInitializer()) - Size = addSizeOfGlobalsInInitializer(GV->getInitializer(), Size); + Size = addSizeOfGlobalsInInitializer(GV->getInitializer(), Size, + SeenGlobals, Worklist); } return Size; diff --git a/unittests/ExecutionEngine/JIT/JITTest.cpp b/unittests/ExecutionEngine/JIT/JITTest.cpp index 84ee0e3075d..168bb3cb98d 100644 --- a/unittests/ExecutionEngine/JIT/JITTest.cpp +++ b/unittests/ExecutionEngine/JIT/JITTest.cpp @@ -65,6 +65,8 @@ public: stubsAllocated = 0; } + void setSizeRequired(bool Required) { SizeRequired = Required; } + virtual void setMemoryWritable() { Base->setMemoryWritable(); } virtual void setMemoryExecutable() { Base->setMemoryExecutable(); } virtual void setPoisonMemory(bool poison) { Base->setPoisonMemory(poison); } @@ -628,6 +630,31 @@ TEST_F(JITTest, AvailableExternallyFunctionIsntCompiled) { << " not 7 from the IR version."; } +TEST_F(JITTest, NeedsExactSizeWithManyGlobals) { + // PR5291: When the JMM needed the exact size of function bodies before + // starting to emit them, the JITEmitter would modify a set while iterating + // over it. + TheJIT->DisableLazyCompilation(true); + RJMM->setSizeRequired(true); + + LoadAssembly("@A = global i32 42 " + "@B = global i32* @A " + "@C = global i32** @B " + "@D = global i32*** @C " + "@E = global i32**** @D " + "@F = global i32***** @E " + "@G = global i32****** @F " + "@H = global i32******* @G " + "@I = global i32******** @H " + "define i32********* @test() { " + " ret i32********* @I " + "}"); + Function *testIR = M->getFunction("test"); + int32_t********* (*test)() = reinterpret_cast( + (intptr_t)TheJIT->getPointerToFunction(testIR)); + EXPECT_EQ(42, *********test()); +} + // Converts the LLVM assembly to bitcode and returns it in a std::string. An // empty string indicates an error. std::string AssembleToBitcode(LLVMContext &Context, const char *Assembly) {