Skip to content

Commit aa1692a

Browse files
bruteforceboybruteforceboy
authored
[CIR][CodeGen] Fix catch-all dispatch and multiple destructor calls (#1740)
This PR has two parts: 1. Mimicking the OG [special case](https://github.com/llvm/clangir/blob/d030c9bff74f4f9504a61abe9b2c04a8777028a5/clang/lib/CodeGen/CGException.cpp#L690) for a single catch-all when getting dispatch blocks. The huge testcase I added, gotten by using [creduce](https://github.com/csmith-project/creduce) on a c++ file, crashed at this point [in our version](https://github.com/llvm/clangir/blob/d030c9bff74f4f9504a61abe9b2c04a8777028a5/clang/lib/CIR/CodeGen/CIRGenException.cpp#L789). 2. Fixing multiple destructor calls for the same object. For example, there were tests like [#1](https://github.com/llvm/clangir/blob/d030c9bff74f4f9504a61abe9b2c04a8777028a5/clang/test/CIR/CodeGen/try-catch-dtors.cpp#L370C1-L372C80) and [#2](https://github.com/llvm/clangir/blob/d030c9bff74f4f9504a61abe9b2c04a8777028a5/clang/test/CIR/CodeGen/conditional-cleanup.cpp#L217C1-L224C25), having a second destructor call to an already destroyed object. This PR fixes these and I have updated the tests. Also, I added `"CIR-NEXT"` at some points, to confirm the destructors are indeed called once. As usual, please let me know if you have any concerns.
1 parent 504714a commit aa1692a

File tree

4 files changed

+126
-37
lines changed

4 files changed

+126
-37
lines changed

‎clang/lib/CIR/CodeGen/CIRGenCleanup.cpp‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -643,12 +643,17 @@ void CIRGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
643643
// We only actually emit the cleanup code if the cleanup is either
644644
// active or was used before it was deactivated.
645645
if (EHActiveFlag.isValid() || IsActive) {
646-
cleanupFlags.setIsForEHCleanup();
647-
mlir::OpBuilder::InsertionGuard guard(builder);
648-
649646
auto yield = cast<YieldOp>(ehEntry->getTerminator());
650-
builder.setInsertionPoint(yield);
651-
emitCleanup(*this, Fn, cleanupFlags, EHActiveFlag);
647+
648+
// We skip the cleanups at the end of CIR scopes as they will be handled
649+
// later. This prevents cases like multiple destructor calls for the same
650+
// object.
651+
if (!isa<ScopeOp>(yield->getParentOp())) {
652+
cleanupFlags.setIsForEHCleanup();
653+
mlir::OpBuilder::InsertionGuard guard(builder);
654+
builder.setInsertionPoint(yield);
655+
emitCleanup(*this, Fn, cleanupFlags, EHActiveFlag);
656+
}
652657
}
653658

654659
if (CPI)

‎clang/lib/CIR/CodeGen/CIRGenException.cpp‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ static void emitCatchDispatchBlock(CIRGenFunction &CGF,
418418
// that catch-all as the dispatch block.
419419
if (catchScope.getNumHandlers() == 1 &&
420420
catchScope.getHandler(0).isCatchAll()) {
421-
// assert(dispatchBlock == catchScope.getHandler(0).Block);
421+
assert(dispatchBlock == catchScope.getHandler(0).Block);
422422
return;
423423
}
424424

@@ -786,13 +786,21 @@ CIRGenFunction::getEHDispatchBlock(EHScopeStack::stable_iterator si,
786786
case EHScope::Catch: {
787787
// LLVM does some optimization with branches here, CIR just keep track of
788788
// the corresponding calls.
789-
assert(callWithExceptionCtx && "expected call information");
790-
{
791-
mlir::OpBuilder::InsertionGuard guard(getBuilder());
792-
assert(callWithExceptionCtx.getCleanup().empty() &&
793-
"one per call: expected empty region at this point");
794-
dispatchBlock = builder.createBlock(&callWithExceptionCtx.getCleanup());
795-
builder.createYield(callWithExceptionCtx.getLoc());
789+
EHCatchScope &catchScope = cast<EHCatchScope>(scope);
790+
if (catchScope.getNumHandlers() == 1 &&
791+
catchScope.getHandler(0).isCatchAll()) {
792+
dispatchBlock = catchScope.getHandler(0).Block;
793+
assert(dispatchBlock);
794+
} else {
795+
assert(callWithExceptionCtx && "expected call information");
796+
{
797+
mlir::OpBuilder::InsertionGuard guard(getBuilder());
798+
assert(callWithExceptionCtx.getCleanup().empty() &&
799+
"one per call: expected empty region at this point");
800+
dispatchBlock =
801+
builder.createBlock(&callWithExceptionCtx.getCleanup());
802+
builder.createYield(callWithExceptionCtx.getLoc());
803+
}
796804
}
797805
break;
798806
}

‎clang/test/CIR/CodeGen/conditional-cleanup.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,10 +218,6 @@ namespace test7 {
218218
// CIR_EH: cir.if %[[VAL_41]] {
219219
// CIR_EH: cir.call @_ZN5test71AD1Ev(%[[VAL_2]]) : (!cir.ptr<!rec_test73A3AA>) -> ()
220220
// CIR_EH: }
221-
// CIR_EH: %[[VAL_42:.*]] = cir.load{{.*}} %[[VAL_3]] : !cir.ptr<!cir.bool>, !cir.bool
222-
// CIR_EH: cir.if %[[VAL_42]] {
223-
// CIR_EH: cir.call @_ZN5test71AD1Ev(%[[VAL_2]]) : (!cir.ptr<!rec_test73A3AA>) -> ()
224-
// CIR_EH: }
225221
// CIR_EH: %[[VAL_43:.*]] = cir.load{{.*}} %[[VAL_1]] : !cir.ptr<!cir.bool>, !cir.bool
226222
// CIR_EH: cir.if %[[VAL_43]] {
227223
// CIR_EH: cir.call @_ZdlPvm(%[[VAL_16]], %[[VAL_15]]) : (!cir.ptr<!void>, !u64i) -> ()

‎clang/test/CIR/CodeGen/try-catch-dtors.cpp‎

Lines changed: 100 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -351,24 +351,104 @@ void d() {
351351
}
352352

353353
// CIR: %[[V0:.*]] = cir.alloca !rec_C, !cir.ptr<!rec_C>, ["a"] {alignment = 1 : i64}
354-
// CIR: %[[V1:.*]] = cir.alloca !rec_C, !cir.ptr<!rec_C>, ["b"] {alignment = 1 : i64}
355-
// CIR: cir.scope {
356-
// CIR: %[[V2:.*]] = cir.alloca !rec_C, !cir.ptr<!rec_C>, ["agg.tmp0"] {alignment = 1 : i64}
357-
// CIR: cir.copy %[[V1]] to %[[V2]] : !cir.ptr<!rec_C>
358-
// CIR: %[[V3:.*]] = cir.load{{.*}} %[[V2]] : !cir.ptr<!rec_C>, !rec_C
359-
// CIR: cir.try synthetic cleanup {
360-
// CIR: cir.call exception @_ZN1CaSES_(%[[V0]], %[[V3]]) : (!cir.ptr<!rec_C>, !rec_C) -> () cleanup {
361-
// CIR: cir.call @_ZN1CD1Ev(%[[V2]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
362-
// CIR: cir.call @_ZN1CD1Ev(%[[V1]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
354+
// CIR-NEXT: %[[V1:.*]] = cir.alloca !rec_C, !cir.ptr<!rec_C>, ["b"] {alignment = 1 : i64}
355+
// CIR-NEXT: cir.scope {
356+
// CIR-NEXT: %[[V2:.*]] = cir.alloca !rec_C, !cir.ptr<!rec_C>, ["agg.tmp0"] {alignment = 1 : i64}
357+
// CIR-NEXT: cir.copy %[[V1]] to %[[V2]] : !cir.ptr<!rec_C>
358+
// CIR-NEXT: %[[V3:.*]] = cir.load{{.*}} %[[V2]] : !cir.ptr<!rec_C>, !rec_C
359+
// CIR-NEXT: cir.try synthetic cleanup {
360+
// CIR-NEXT: cir.call exception @_ZN1CaSES_(%[[V0]], %[[V3]]) : (!cir.ptr<!rec_C>, !rec_C) -> () cleanup {
361+
// CIR-NEXT: cir.call @_ZN1CD1Ev(%[[V2]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
362+
// CIR-NEXT: cir.call @_ZN1CD1Ev(%[[V1]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
363+
// CIR-NEXT: cir.yield
364+
// CIR-NEXT: }
365+
// CIR-NEXT: cir.yield
366+
// CIR-NEXT: } catch [#cir.unwind {
367+
// CIR-NEXT: cir.resume
368+
// CIR-NEXT: }]
369+
// CIR-NEXT: cir.call @_ZN1CD1Ev(%[[V2]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
370+
// CIR-NEXT: }
371+
// CIR-NEXT: cir.call @_ZN1CD1Ev(%[[V1]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
372+
// CIR-NEXT: cir.call @_ZN1CD1Ev(%[[V0]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
373+
// CIR-NEXT: cir.return
374+
375+
template <typename> class a;
376+
377+
template <> class a<void> {
378+
public:
379+
struct b {
380+
typedef a<int> c;
381+
};
382+
};
383+
384+
template <typename> class a {
385+
public:
386+
template <typename d> a(d) noexcept;
387+
~a();
388+
};
389+
390+
struct e {
391+
using f = a<void>::b::c;
392+
};
393+
394+
template <typename, typename> using g = e::f;
395+
396+
template <typename h> void i(h);
397+
398+
class j {
399+
400+
public:
401+
using k = g<int, j>;
402+
};
403+
404+
class l {
405+
public:
406+
template <typename m, typename n> l(m p1, n) : l(p1, 0, a<void>()) {}
407+
template <typename m, typename n, typename h> l(m, n, h o) {
408+
try {
409+
j::k p(o);
410+
i(p);
411+
} catch (...) {
412+
}
413+
}
414+
};
415+
416+
class G {
417+
public:
418+
template <typename q, typename n> G(q p1, n) : r(p1, 0) {}
419+
l r;
420+
};
421+
422+
class s : G {
423+
public:
424+
int t;
425+
s() : G(t, 0) {}
426+
};
427+
428+
void fn3() { s(); }
429+
430+
// CIR: cir.func linkonce_odr @_ZN1lC2Iii1aIvEEET_T0_T1_
431+
// CIR: cir.scope
432+
// CIR: %[[V5:.*]] = cir.alloca !rec_a3Cint3E, !cir.ptr<!rec_a3Cint3E>
433+
// CIR: %[[V6:.*]] = cir.alloca !rec_a3Cvoid3E, !cir.ptr<!rec_a3Cvoid3E>
434+
// CIR: cir.try {
435+
// CIR: cir.copy {{.*}} to %[[V6]] : !cir.ptr<!rec_a3Cvoid3E>
436+
// CIR: %[[V7:.*]] = cir.load align(1) %[[V6]] : !cir.ptr<!rec_a3Cvoid3E>, !rec_a3Cvoid3E
437+
// CIR: cir.call @_ZN1aIiEC1IS_IvEEET_(%[[V5]], %[[V7]]) : (!cir.ptr<!rec_a3Cint3E>, !rec_a3Cvoid3E) -> ()
438+
// CIR: cir.scope {
439+
// CIR: %[[V8:.*]] = cir.alloca !rec_a3Cint3E, !cir.ptr<!rec_a3Cint3E>
440+
// CIR: cir.copy %[[V5]] to %[[V8]] : !cir.ptr<!rec_a3Cint3E>
441+
// CIR: %[[V9:.*]] = cir.load align(1) %[[V8]] : !cir.ptr<!rec_a3Cint3E>, !rec_a3Cint3E
442+
// CIR-NEXT: cir.call exception @_Z1iI1aIiEEvT_(%[[V9]]) : (!rec_a3Cint3E) -> () cleanup {
443+
// CIR-NEXT: cir.call @_ZN1aIiED1Ev(%[[V8]]) : (!cir.ptr<!rec_a3Cint3E>) -> ()
444+
// CIR-NEXT: cir.call @_ZN1aIiED1Ev(%[[V5]]) : (!cir.ptr<!rec_a3Cint3E>) -> ()
445+
// CIR-NEXT: cir.yield
446+
// CIR-NEXT: }
447+
// CIR-NEXT: cir.call @_ZN1aIiED1Ev(%[[V8]]) : (!cir.ptr<!rec_a3Cint3E>) -> ()
448+
// CIR-NEXT: }
449+
// CIR-NEXT: cir.call @_ZN1aIiED1Ev(%[[V5]]) : (!cir.ptr<!rec_a3Cint3E>) -> ()
450+
// CIR-NEXT: cir.yield
451+
// CIR: } catch [type #cir.all {
452+
// CIR: %[[V7:.*]] = cir.catch_param -> !cir.ptr<!void>
363453
// CIR: cir.yield
364-
// CIR: }
365-
// CIR: cir.yield
366-
// CIR: } catch [#cir.unwind {
367-
// CIR: cir.resume
368-
// CIR: }]
369-
// CIR: cir.call @_ZN1CD1Ev(%[[V2]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
370-
// CIR: cir.call @_ZN1CD1Ev(%[[V1]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
371-
// CIR: }
372-
// CIR: cir.call @_ZN1CD1Ev(%[[V1]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
373-
// CIR: cir.call @_ZN1CD1Ev(%[[V0]]) : (!cir.ptr<!rec_C>) -> () extra(#fn_attr)
374-
// CIR: cir.return
454+
// CIR: }]

0 commit comments

Comments
 (0)