https://github.com/bcardosolopes updated https://github.com/llvm/llvm-project/pull/229191
>From 778507659ea987a8b0711b19f5eb7ca8c1db0d71 Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes <[email protected]> Date: Mon, 5 Oct 2026 12:20:50 -0700 Subject: [PATCH 1/2] [CIR] Fix use-after-free in BrOp::canonicalize BrOp::canonicalize took the branch's destination operands as an OperandRange, erased the branch, and then passed the range to mergeBlocks. The range points into the branch's operand storage, which eraseOp frees, so mergeBlocks read freed memory whenever the merged block had arguments. Valgrind reports it on the new test: Invalid read of size 8 at mlir::ValueRange::dereference_iterator by mlir::RewriterBase::inlineBlockBefore by cir::BrOp::canonicalize Address ... is 136 bytes inside a block of size 144 free'd by mlir::RewriterBase::eraseOp by cir::BrOp::canonicalize It goes unnoticed with glibc malloc, which usually leaves the freed operands intact. Copy the operands before erasing the branch. --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 3 ++- clang/test/CIR/Transforms/canonicalize.cir | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 15cebd358040e..2576ca6d040de 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -2067,7 +2067,8 @@ LogicalResult cir::BrOp::canonicalize(BrOp op, PatternRewriter &rewriter) { if (isa<cir::LabelOp, cir::IndirectBrOp>(dst->front())) return failure(); - auto operands = op.getDestOperands(); + // Copy the operands out: erasing the branch frees its operand storage. + SmallVector<Value> operands(op.getDestOperands()); rewriter.eraseOp(op); rewriter.mergeBlocks(dst, src, operands); return success(); diff --git a/clang/test/CIR/Transforms/canonicalize.cir b/clang/test/CIR/Transforms/canonicalize.cir index 29b1d880a3748..c5b72f424aa64 100644 --- a/clang/test/CIR/Transforms/canonicalize.cir +++ b/clang/test/CIR/Transforms/canonicalize.cir @@ -28,6 +28,20 @@ module { // CHECK-NEXT: cir.return // CHECK-NEXT: } + // The branch operands replace the arguments of the merged blocks. + cir.func @redundant_br_block_args(%arg0: !s32i, %arg1: !s32i) -> !s32i { + cir.br ^bb1(%arg0, %arg1 : !s32i, !s32i) + ^bb1(%0: !s32i, %1: !s32i): // pred: ^bb0 + %2 = cir.add %0, %1 : !s32i + cir.br ^bb2(%2 : !s32i) + ^bb2(%3: !s32i): // pred: ^bb1 + cir.return %3 : !s32i + } + // CHECK: cir.func{{.*}} @redundant_br_block_args(%[[ARG0:.*]]: !s32i, %[[ARG1:.*]]: !s32i) -> !s32i { + // CHECK-NEXT: %[[SUM:.*]] = cir.add %[[ARG0]], %[[ARG1]] : !s32i + // CHECK-NEXT: cir.return %[[SUM]] : !s32i + // CHECK-NEXT: } + cir.func @scope_yield_value_fold() -> !u32i { %0 = cir.const #cir.int<7> : !u32i %1 = cir.scope { >From 3ec61dc3669423308bcb3574510b3657dabe6dfb Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes <[email protected]> Date: Mon, 5 Oct 2026 13:32:15 -0700 Subject: [PATCH 2/2] [CIR] Skip BrOp block merging when an operand is the destination's own argument In an unreachable cycle, a block's only predecessor can branch back to it with one of the block's own arguments as the operand. BrOp::canonicalize then merged the block into that predecessor, replacing the argument with itself, so the argument's other uses outlived the erased block: Assertion `use_empty() && "Cannot destroy a value that still has uses!"' ... mlir::Block::erase() cir::BrOp::canonicalize(cir::BrOp, mlir::PatternRewriter&) Skip the merge in that case, as the cf.br canonicalization does since #183797. --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 9 +++++++++ clang/test/CIR/Transforms/canonicalize.cir | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 2576ca6d040de..be76e27bec2e5 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -2067,6 +2067,15 @@ LogicalResult cir::BrOp::canonicalize(BrOp op, PatternRewriter &rewriter) { if (isa<cir::LabelOp, cir::IndirectBrOp>(dst->front())) return failure(); + // An operand that is an argument of the destination itself (possible in an + // unreachable cycle) would be replaced by itself, leaving its other uses + // dangling once the destination is erased. + if (llvm::any_of(op.getDestOperands(), [&](Value operand) { + auto arg = dyn_cast<BlockArgument>(operand); + return arg && arg.getOwner() == dst; + })) + return failure(); + // Copy the operands out: erasing the branch frees its operand storage. SmallVector<Value> operands(op.getDestOperands()); rewriter.eraseOp(op); diff --git a/clang/test/CIR/Transforms/canonicalize.cir b/clang/test/CIR/Transforms/canonicalize.cir index c5b72f424aa64..a786b252e91ad 100644 --- a/clang/test/CIR/Transforms/canonicalize.cir +++ b/clang/test/CIR/Transforms/canonicalize.cir @@ -42,6 +42,26 @@ module { // CHECK-NEXT: cir.return %[[SUM]] : !s32i // CHECK-NEXT: } + // In this unreachable cycle, ^header is the only predecessor of ^body and + // ^body the only predecessor of ^header, and ^body passes ^header's own + // argument back to it. ^header must not be merged into ^body: %keep would be + // replaced by itself, and its use in the call would outlive ^header. + cir.func private @use(!s32i) + cir.func @no_merge_self_arg_loop(%arg0: !s32i) -> !s32i { + cir.return %arg0 : !s32i + ^header(%keep: !s32i): + cir.call @use(%keep) : (!s32i) -> () + cir.br ^body + ^body: + cir.br ^header(%keep : !s32i) + } + // CHECK: cir.func{{.*}} @no_merge_self_arg_loop(%[[ARG0:.*]]: !s32i) -> !s32i { + // CHECK-NEXT: cir.return %[[ARG0]] : !s32i + // CHECK-NEXT: ^[[HEADER:bb[0-9]+]](%[[KEEP:.*]]: !s32i): + // CHECK-NEXT: cir.call @use(%[[KEEP]]) : (!s32i) -> () + // CHECK-NEXT: cir.br ^[[HEADER]](%[[KEEP]] : !s32i) + // CHECK-NEXT: } + cir.func @scope_yield_value_fold() -> !u32i { %0 = cir.const #cir.int<7> : !u32i %1 = cir.scope { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
