Skip to content

Conversation

@bruteforceboy
Copy link
Contributor

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 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 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, as a test, because std-cxx.h doesn't quite replicate std::vector correctly.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one minor nit

@bcardosolopes
Copy link
Member

Looks like tests are failing

@bruteforceboy
Copy link
Contributor Author

Looks like tests are failing

Oh, I didn't notice ( It seems this PR#1580 made quite a number of changes. I will update the test in a few hours.

@bcardosolopes bcardosolopes merged commit d071f2f into llvm:main Apr 24, 2025
9 checks passed
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
…nups (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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants