diff --git a/llvm/lib/CodeGen/StackColoring.cpp b/llvm/lib/CodeGen/StackColoring.cpp index 4d69b6104f683..d90c3e95f4779 100644 --- a/llvm/lib/CodeGen/StackColoring.cpp +++ b/llvm/lib/CodeGen/StackColoring.cpp @@ -370,37 +370,6 @@ STATISTIC(EscapedAllocas, "Number of allocas that escaped the lifetime region"); // If in RPO ordering chosen to walk the CFG we happen to visit the b[k] // before visiting the memcpy block (which will contain the lifetime start // for "b" then it will appear that 'b' has a degenerate lifetime. -// -// Handle Windows Exception with LifetimeStartOnFirstUse: -// ----------------- -// -// There was a bug for using LifetimeStartOnFirstUse in win32. -// class Type1 { -// ... -// ~Type1(){ write memory;} -// } -// ... -// try{ -// Type1 V -// ... -// } catch (Type2 X){ -// ... -// } -// For variable X in catch(X), we put point pX=&(&X) into ConservativeSlots -// to prevent using LifetimeStartOnFirstUse. Because pX may merged with -// object V which may call destructor after implicitly writing pX. All these -// are done in C++ EH runtime libs (through CxxThrowException), and can't -// obviously check it in IR level. -// -// The loader of pX, without obvious writing IR, is usually the first LOAD MI -// in EHPad, Some like: -// bb.x.catch.i (landing-pad, ehfunclet-entry): -// ; predecessors: %bb... -// successors: %bb... -// %n:gr32 = MOV32rm %stack.pX ... -// ... -// The Type2** %stack.pX will only be written in EH runtime libs, so we -// check the StoreSlots to screen it out. namespace { @@ -462,9 +431,6 @@ class StackColoring : public MachineFunctionPass { /// slots lifetime-start-on-first-use is disabled). BitVector ConservativeSlots; - /// Record the FI slots referenced by a 'may write to memory'. - BitVector StoreSlots; - /// Number of iterations taken during data flow analysis. unsigned NumIterations; @@ -660,13 +626,10 @@ unsigned StackColoring::collectMarkers(unsigned NumSlot) { InterestingSlots.resize(NumSlot); ConservativeSlots.clear(); ConservativeSlots.resize(NumSlot); - StoreSlots.clear(); - StoreSlots.resize(NumSlot); // number of start and end lifetime ops for each slot SmallVector NumStartLifetimes(NumSlot, 0); SmallVector NumEndLifetimes(NumSlot, 0); - SmallVector NumLoadInCatchPad(NumSlot, 0); // Step 1: collect markers and populate the "InterestingSlots" // and "ConservativeSlots" sets. @@ -722,13 +685,6 @@ unsigned StackColoring::collectMarkers(unsigned NumSlot) { if (! BetweenStartEnd.test(Slot)) { ConservativeSlots.set(Slot); } - // Here we check the StoreSlots to screen catch point out. For more - // information, please refer "Handle Windows Exception with - // LifetimeStartOnFirstUse" at the head of this file. - if (MI.mayStore()) - StoreSlots.set(Slot); - if (MF->getWinEHFuncInfo() && MBB->isEHPad() && MI.mayLoad()) - NumLoadInCatchPad[Slot] += 1; } } } @@ -739,14 +695,23 @@ unsigned StackColoring::collectMarkers(unsigned NumSlot) { return 0; } - // 1) PR27903: slots with multiple start or end lifetime ops are not + // PR27903: slots with multiple start or end lifetime ops are not // safe to enable for "lifetime-start-on-first-use". - // 2) And also not safe for variable X in catch(X) in windows. for (unsigned slot = 0; slot < NumSlot; ++slot) { - if (NumStartLifetimes[slot] > 1 || NumEndLifetimes[slot] > 1 || - (NumLoadInCatchPad[slot] > 1 && !StoreSlots.test(slot))) + if (NumStartLifetimes[slot] > 1 || NumEndLifetimes[slot] > 1) ConservativeSlots.set(slot); } + + // The write to the catch object by the personality function is not propely + // modeled in IR: It happens before any cleanuppads are executed, even if the + // first mention of the catch object is in a catchpad. As such, mark catch + // object slots as conservative, so they are excluded from first-use analysis. + if (WinEHFuncInfo *EHInfo = MF->getWinEHFuncInfo()) + for (WinEHTryBlockMapEntry &TBME : EHInfo->TryBlockMap) + for (WinEHHandlerType &H : TBME.HandlerArray) + if (H.CatchObj.FrameIndex != std::numeric_limits::max()) + ConservativeSlots.set(H.CatchObj.FrameIndex); + LLVM_DEBUG(dumpBV("Conservative slots", ConservativeSlots)); // Step 2: compute begin/end sets for each block diff --git a/llvm/test/CodeGen/X86/stack-coloring-seh.ll b/llvm/test/CodeGen/X86/stack-coloring-wineh.ll similarity index 95% rename from llvm/test/CodeGen/X86/stack-coloring-seh.ll rename to llvm/test/CodeGen/X86/stack-coloring-wineh.ll index 3995bfa8101d6..892c81a12dc1a 100644 --- a/llvm/test/CodeGen/X86/stack-coloring-seh.ll +++ b/llvm/test/CodeGen/X86/stack-coloring-wineh.ll @@ -3,7 +3,7 @@ @type_info = external global ptr -; FIXME: This is a miscompile. +; Make sure %a1 and %a2 don't share the same stack offset. define void @pr66984(ptr %arg) personality ptr @__CxxFrameHandler3 { ; CHECK-LABEL: pr66984: ; CHECK: # %bb.0: # %bb @@ -12,7 +12,7 @@ define void @pr66984(ptr %arg) personality ptr @__CxxFrameHandler3 { ; CHECK-NEXT: pushl %ebx ; CHECK-NEXT: pushl %edi ; CHECK-NEXT: pushl %esi -; CHECK-NEXT: subl $20, %esp +; CHECK-NEXT: subl $24, %esp ; CHECK-NEXT: movl %esp, -28(%ebp) ; CHECK-NEXT: movl $-1, -16(%ebp) ; CHECK-NEXT: leal -24(%ebp), %eax @@ -31,7 +31,7 @@ define void @pr66984(ptr %arg) personality ptr @__CxxFrameHandler3 { ; CHECK-NEXT: $ehgcr_0_4: ; CHECK-NEXT: movl -24(%ebp), %eax ; CHECK-NEXT: movl %eax, %fs:0 -; CHECK-NEXT: addl $20, %esp +; CHECK-NEXT: addl $24, %esp ; CHECK-NEXT: popl %esi ; CHECK-NEXT: popl %edi ; CHECK-NEXT: popl %ebx @@ -47,7 +47,7 @@ define void @pr66984(ptr %arg) personality ptr @__CxxFrameHandler3 { ; CHECK-NEXT: pushl %ebp ; CHECK-NEXT: addl $12, %ebp ; CHECK-NEXT: movl %esp, -28(%ebp) -; CHECK-NEXT: movl -32(%ebp), %ecx +; CHECK-NEXT: movl -36(%ebp), %ecx ; CHECK-NEXT: movl $2, -16(%ebp) ; CHECK-NEXT: calll _cleanup ; CHECK-NEXT: movl $LBB0_3, %eax