Skip to content

Commit 97026ca

Browse files
committed
Revert "Fix missing dtor in function calls accepting trivial ABI structs (llvm#88751)"
This reverts commit 5a46123.
1 parent 668a58b commit 97026ca

File tree

4 files changed

+21
-48
lines changed

4 files changed

+21
-48
lines changed

clang/lib/CodeGen/CGCall.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4694,11 +4694,11 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
46944694
AggValueSlot Slot = args.isUsingInAlloca()
46954695
? createPlaceholderSlot(*this, type) : CreateAggTemp(type, "agg.tmp");
46964696

4697-
bool DestroyedInCallee = true, NeedsCleanup = true;
4697+
bool DestroyedInCallee = true, NeedsEHCleanup = true;
46984698
if (const auto *RD = type->getAsCXXRecordDecl())
46994699
DestroyedInCallee = RD->hasNonTrivialDestructor();
47004700
else
4701-
NeedsCleanup = type.isDestructedType();
4701+
NeedsEHCleanup = needsEHCleanup(type.isDestructedType());
47024702

47034703
if (DestroyedInCallee)
47044704
Slot.setExternallyDestructed();
@@ -4707,15 +4707,14 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
47074707
RValue RV = Slot.asRValue();
47084708
args.add(RV, type);
47094709

4710-
if (DestroyedInCallee && NeedsCleanup) {
4710+
if (DestroyedInCallee && NeedsEHCleanup) {
47114711
// Create a no-op GEP between the placeholder and the cleanup so we can
47124712
// RAUW it successfully. It also serves as a marker of the first
47134713
// instruction where the cleanup is active.
4714-
pushFullExprCleanup<DestroyUnpassedArg>(NormalAndEHCleanup,
4715-
Slot.getAddress(), type);
4714+
pushFullExprCleanup<DestroyUnpassedArg>(EHCleanup, Slot.getAddress(),
4715+
type);
47164716
// This unreachable is a temporary marker which will be removed later.
4717-
llvm::Instruction *IsActive =
4718-
Builder.CreateFlagLoad(llvm::Constant::getNullValue(Int8PtrTy));
4717+
llvm::Instruction *IsActive = Builder.CreateUnreachable();
47194718
args.addArgCleanupDeactivation(EHStack.stable_begin(), IsActive);
47204719
}
47214720
return;

clang/lib/CodeGen/CGCleanup.cpp

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -634,19 +634,12 @@ static void destroyOptimisticNormalEntry(CodeGenFunction &CGF,
634634
/// Pops a cleanup block. If the block includes a normal cleanup, the
635635
/// current insertion point is threaded through the cleanup, as are
636636
/// any branch fixups on the cleanup.
637-
void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
638-
bool ForDeactivation) {
637+
void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
639638
assert(!EHStack.empty() && "cleanup stack is empty!");
640639
assert(isa<EHCleanupScope>(*EHStack.begin()) && "top not a cleanup!");
641640
EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.begin());
642641
assert(Scope.getFixupDepth() <= EHStack.getNumBranchFixups());
643642

644-
// If we are deactivating a normal cleanup, we need to pretend that the
645-
// fallthrough is unreachable. We restore this IP before returning.
646-
CGBuilderTy::InsertPoint NormalDeactivateOrigIP;
647-
if (ForDeactivation && (Scope.isNormalCleanup() || !getLangOpts().EHAsynch)) {
648-
NormalDeactivateOrigIP = Builder.saveAndClearIP();
649-
}
650643
// Remember activation information.
651644
bool IsActive = Scope.isActive();
652645
Address NormalActiveFlag =
@@ -736,8 +729,6 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
736729
EHStack.popCleanup(); // safe because there are no fixups
737730
assert(EHStack.getNumBranchFixups() == 0 ||
738731
EHStack.hasNormalCleanups());
739-
if (NormalDeactivateOrigIP.isSet())
740-
Builder.restoreIP(NormalDeactivateOrigIP);
741732
return;
742733
}
743734

@@ -774,16 +765,9 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
774765
if (!RequiresNormalCleanup) {
775766
// Mark CPP scope end for passed-by-value Arg temp
776767
// per Windows ABI which is "normally" Cleanup in callee
777-
if (IsEHa && getInvokeDest()) {
778-
// If we are deactivating a normal cleanup then we don't have a
779-
// fallthrough. Restore original IP to emit CPP scope ends in the correct
780-
// block.
781-
if (NormalDeactivateOrigIP.isSet())
782-
Builder.restoreIP(NormalDeactivateOrigIP);
783-
if (Personality.isMSVCXXPersonality() && Builder.GetInsertBlock())
768+
if (IsEHa && getInvokeDest() && Builder.GetInsertBlock()) {
769+
if (Personality.isMSVCXXPersonality())
784770
EmitSehCppScopeEnd();
785-
if (NormalDeactivateOrigIP.isSet())
786-
NormalDeactivateOrigIP = Builder.saveAndClearIP();
787771
}
788772
destroyOptimisticNormalEntry(*this, Scope);
789773
Scope.MarkEmitted();
@@ -1008,8 +992,6 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough,
1008992
}
1009993
}
1010994

1011-
if (NormalDeactivateOrigIP.isSet())
1012-
Builder.restoreIP(NormalDeactivateOrigIP);
1013995
assert(EHStack.hasNormalCleanups() || EHStack.getNumBranchFixups() == 0);
1014996

1015997
// Emit the EH cleanup if required.
@@ -1299,8 +1281,17 @@ void CodeGenFunction::DeactivateCleanupBlock(EHScopeStack::stable_iterator C,
12991281
// to the current RunCleanupsScope.
13001282
if (C == EHStack.stable_begin() &&
13011283
CurrentCleanupScopeDepth.strictlyEncloses(C)) {
1302-
PopCleanupBlock(/*FallthroughIsBranchThrough=*/false,
1303-
/*ForDeactivation=*/true);
1284+
// Per comment below, checking EHAsynch is not really necessary
1285+
// it's there to assure zero-impact w/o EHAsynch option
1286+
if (!Scope.isNormalCleanup() && getLangOpts().EHAsynch) {
1287+
PopCleanupBlock();
1288+
} else {
1289+
// If it's a normal cleanup, we need to pretend that the
1290+
// fallthrough is unreachable.
1291+
CGBuilderTy::InsertPoint SavedIP = Builder.saveAndClearIP();
1292+
PopCleanupBlock();
1293+
Builder.restoreIP(SavedIP);
1294+
}
13041295
return;
13051296
}
13061297

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -957,8 +957,7 @@ class CodeGenFunction : public CodeGenTypeCache {
957957

958958
/// PopCleanupBlock - Will pop the cleanup entry on the stack and
959959
/// process all branch fixups.
960-
void PopCleanupBlock(bool FallThroughIsBranchThrough = false,
961-
bool ForDeactivation = false);
960+
void PopCleanupBlock(bool FallThroughIsBranchThrough = false);
962961

963962
/// DeactivateCleanupBlock - Deactivates the given cleanup block.
964963
/// The block cannot be reactivated. Pops it if it's the top of the

clang/test/CodeGenCXX/control-flow-in-stmt-expr.cpp

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -391,19 +391,3 @@ void ArrayInitWithContinue() {
391391
})};
392392
}
393393
}
394-
395-
struct [[clang::trivial_abi]] HasTrivialABI {
396-
HasTrivialABI();
397-
~HasTrivialABI();
398-
};
399-
void AcceptTrivialABI(HasTrivialABI, int);
400-
void TrivialABI() {
401-
// CHECK-LABEL: define dso_local void @_Z10TrivialABIv()
402-
AcceptTrivialABI(HasTrivialABI(), ({
403-
if (foo()) return;
404-
// CHECK: if.then:
405-
// CHECK-NEXT: call void @_ZN13HasTrivialABID1Ev
406-
// CHECK-NEXT: br label %return
407-
0;
408-
}));
409-
}

0 commit comments

Comments
 (0)