[flang][CodeGen] Avoid out-of-bounds memory access in SelectCaseOp

[kparzysz]

SelectCaseOp::getCompareOperands may return an empty range for the "default" case. Do not dereference the range until it is expected to be non-empty.

This was detected by address-sanitizer.

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-codegen

Author: Krzysztof Parzyszek (kparzysz)

Changes

SelectCaseOp::getCompareOperands may return an empty range for the "default" case. Do not dereference the range until it is expected to be non-empty.

This was detected by address-sanitizer.

---

Full diff: https://github.com/llvm/llvm-project/pull/92955.diff

1 Files Affected:

• (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+12-11)

diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp index 72172f63888e1..17fbe396d6d53 100644 --- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp +++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp @@ -2966,39 +2966,40 @@ struct SelectCaseOpConversion : public fir::FIROpConversion<fir::SelectCaseOp> { caseOp.getSuccessorOperands(adaptor.getOperands(), t); std::optional<mlir::ValueRange> cmpOps = *caseOp.getCompareOperands(adaptor.getOperands(), t); - mlir::Value caseArg = *(cmpOps.value().begin()); mlir::Attribute attr = cases[t]; + assert(mlir::isa<mlir::UnitAttr>(attr) || cmpOps.has_value()); if (mlir::isa<fir::PointIntervalAttr>(attr)) { auto cmp = rewriter.create<mlir::LLVM::ICmpOp>( - loc, mlir::LLVM::ICmpPredicate::eq, selector, caseArg); + loc, mlir::LLVM::ICmpPredicate::eq, selector, cmpOps->front()); genCaseLadderStep(loc, cmp, dest, destOps, rewriter); continue; } if (mlir::isa<fir::LowerBoundAttr>(attr)) { auto cmp = rewriter.create<mlir::LLVM::ICmpOp>( - loc, mlir::LLVM::ICmpPredicate::sle, caseArg, selector); + loc, mlir::LLVM::ICmpPredicate::sle, cmpOps->front(), selector); genCaseLadderStep(loc, cmp, dest, destOps, rewriter); continue; } if (mlir::isa<fir::UpperBoundAttr>(attr)) { auto cmp = rewriter.create<mlir::LLVM::ICmpOp>( - loc, mlir::LLVM::ICmpPredicate::sle, selector, caseArg); + loc, mlir::LLVM::ICmpPredicate::sle, selector, cmpOps->front()); genCaseLadderStep(loc, cmp, dest, destOps, rewriter); continue; } if (mlir::isa<fir::ClosedIntervalAttr>(attr)) { - auto cmp = rewriter.create<mlir::LLVM::ICmpOp>( - loc, mlir::LLVM::ICmpPredicate::sle, caseArg, selector); + mlir::Value caseArg0 = *cmpOps->begin(); + auto cmp0 = rewriter.create<mlir::LLVM::ICmpOp>( + loc, mlir::LLVM::ICmpPredicate::sle, caseArg0, selector); auto *thisBlock = rewriter.getInsertionBlock(); auto *newBlock1 = createBlock(rewriter, dest); auto *newBlock2 = createBlock(rewriter, dest); rewriter.setInsertionPointToEnd(thisBlock); - rewriter.create<mlir::LLVM::CondBrOp>(loc, cmp, newBlock1, newBlock2); + rewriter.create<mlir::LLVM::CondBrOp>(loc, cmp0, newBlock1, newBlock2); rewriter.setInsertionPointToEnd(newBlock1); - mlir::Value caseArg0 = *(cmpOps.value().begin() + 1); - auto cmp0 = rewriter.create<mlir::LLVM::ICmpOp>( - loc, mlir::LLVM::ICmpPredicate::sle, selector, caseArg0); - genCondBrOp(loc, cmp0, dest, destOps, rewriter, newBlock2); + mlir::Value caseArg1 = *(cmpOps->begin() + 1); + auto cmp1 = rewriter.create<mlir::LLVM::ICmpOp>( + loc, mlir::LLVM::ICmpPredicate::sle, selector, caseArg1); + genCondBrOp(loc, cmp1, dest, destOps, rewriter, newBlock2); rewriter.setInsertionPointToEnd(newBlock2); continue; } 

[jeanPerier]

Thanks for identifying and fixing this.