Skip to content

Commit 250c110

Browse files
bruteforceboylanza
authored andcommitted
[CIR][CodeGen] Emit RunCleanupsScope's dtor properly for ExprWithCleanups (llvm#1581)
The following code snippet crashes during CIR CodeGen using `clang tmp.cpp -Xclang -emit-cir -S -o -`: ``` #include <vector> void foo() { std::vector<int> v(1); } ``` The crash is: ``` mlir::Operation* mlir::Block::getTerminator(): Assertion `mightHaveTerminator()' failed. ``` What happens is the scope created [here](https://github.com/llvm/clangir/blob/c7b27ece0971c632f2ebec26bc855ee9b118ffcc/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp#L2370C1-L2381C10) is malformed. The operations inside the scope using `--mlir-print-assume-verified` looks something like: ``` %0 = cir.alloca !cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>, !cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>>, ["ref.tmp0"] {alignment = 1 : i64} %1 = cir.load <<UNKNOWN SSA VALUE>> : !cir.ptr<!cir.int<u, 64>>, !cir.int<u, 64> %2 = cir.load <<UNKNOWN SSA VALUE>> : !cir.ptr<!cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>>>, !cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>> cir.call @_ZNSaIiEC1ERKS_(%0, %2) : (!cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>>, !cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>>) -> () extra(#cir<extra({nothrow = #cir.nothrow})>) %3 = cir.call @_ZNSt6vectorIiSaIiEE11_S_max_sizeERKS0_(%0) : (!cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>>) -> !cir.int<u, 64> extra(#cir<extra({nothrow = #cir.nothrow})>) %4 = cir.cmp(gt, %1, %3) : !cir.int<u, 64>, !cir.bool cir.yield %4 : !cir.bool cir.call @_ZNSaIiED1Ev(%0) : (!cir.ptr<!cir.record<class "std::allocator<int>" padded {!cir.int<u, 8>} #cir.record.decl.ast>>) -> () extra(#cir<extra({nothrow = #cir.nothrow})>) ``` `_ZNSaIiED1Ev` is `std::allocator<int>::~allocator()`. So, the destructor comes after the YieldOp has been created, which is wrong. The destructor comes from the destruction of [`RunCleanupScope`](https://github.com/llvm/clangir/blob/c7b27ece0971c632f2ebec26bc855ee9b118ffcc/clang/lib/CIR/CodeGen/CIRGenFunction.h#L1369) since `LexicalScope` inherits from it. RunCleanupsScope's dtor should come before the yield is created! This PR fixes this by calling `ForceCleanup` before creating the yield, so the YieldOp becomes the last operation in the scope. I found this bug using the code snippet above, but I have added a reduced version, using [creduce](https://github.com/csmith-project/creduce), as a test, because [std-cxx.h](https://github.com/llvm/clangir/blob/main/clang/test/CIR/Inputs/std-cxx.h) doesn't quite replicate `std::vector` correctly.
1 parent 1d2f4f8 commit 250c110

File tree

2 files changed

+49
-3
lines changed

2 files changed

+49
-3
lines changed

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2375,14 +2375,15 @@ mlir::Value ScalarExprEmitter::VisitExprWithCleanups(ExprWithCleanups *E) {
23752375
builder.getInsertionBlock()};
23762376
auto scopeYieldVal = Visit(E->getSubExpr());
23772377
if (scopeYieldVal) {
2378+
// Defend against dominance problems caused by jumps out of expression
2379+
// evaluation through the shared cleanup block. We do not pass {&V} to
2380+
// ForceCleanup, because the scope returns an rvalue.
2381+
lexScope.ForceCleanup();
23782382
builder.create<cir::YieldOp>(loc, scopeYieldVal);
23792383
yieldTy = scopeYieldVal.getType();
23802384
}
23812385
});
23822386

2383-
// Defend against dominance problems caused by jumps out of expression
2384-
// evaluation through the shared cleanup block.
2385-
// TODO(cir): Scope.ForceCleanup({&V});
23862387
return scope.getNumResults() > 0 ? scope->getResult(0) : nullptr;
23872388
}
23882389

clang/test/CIR/CodeGen/dtors.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,48 @@ class B : public A
7878
// CHECK: }
7979

8080
void foo() { B(); }
81+
82+
class A2 {
83+
public:
84+
~A2();
85+
};
86+
87+
struct B2 {
88+
template <typename> using C = A2;
89+
};
90+
91+
struct E {
92+
typedef B2::C<int> D;
93+
};
94+
95+
struct F {
96+
F(long, A2);
97+
};
98+
99+
class G : F {
100+
public:
101+
A2 h;
102+
G(long) : F(i(), h) {}
103+
long i() { k(E::D()); };
104+
long k(E::D);
105+
};
106+
107+
int j;
108+
void m() { G l(j); }
109+
110+
// CHECK: cir.func private @_ZN1G1kE2A2(!cir.ptr<!rec_G>, !rec_A2) -> !s64i
111+
// CHECK: cir.func linkonce_odr @_ZN1G1iEv(%arg0: !cir.ptr<!rec_G>
112+
// CHECK: %[[V0:.*]] = cir.alloca !cir.ptr<!rec_G>, !cir.ptr<!cir.ptr<!rec_G>>, ["this", init] {alignment = 8 : i64}
113+
// CHECK: %[[V1:.*]] = cir.alloca !s64i, !cir.ptr<!s64i>, ["__retval"] {alignment = 8 : i64}
114+
// CHECK: cir.store %arg0, %[[V0]] : !cir.ptr<!rec_G>, !cir.ptr<!cir.ptr<!rec_G>>
115+
// CHECK: %[[V2:.*]] = cir.load %[[V0]] : !cir.ptr<!cir.ptr<!rec_G>>, !cir.ptr<!rec_G>
116+
// CHECK: %[[V3:.*]] = cir.scope {
117+
// CHECK: %[[V4:.*]] = cir.alloca !rec_A2, !cir.ptr<!rec_A2>, ["agg.tmp0"] {alignment = 1 : i64}
118+
// CHECK: cir.call @_ZN2A2C2Ev(%[[V4]]) : (!cir.ptr<!rec_A2>) -> ()
119+
// CHECK: %[[V5:.*]] = cir.load %[[V4]] : !cir.ptr<!rec_A2>, !rec_A2
120+
// CHECK: %[[V6:.*]] = cir.call @_ZN1G1kE2A2(%[[V2]], %[[V5]]) : (!cir.ptr<!rec_G>, !rec_A2) -> !s64i
121+
// CHECK: cir.call @_ZN2A2D1Ev(%[[V4]]) : (!cir.ptr<!rec_A2>) -> ()
122+
// CHECK: cir.yield %[[V6]] : !s64i
123+
// CHECK: } : !s64i
124+
// CHECK: cir.trap
125+
// CHECK: }

0 commit comments

Comments
 (0)