From a8234196c58396c0505ac93983dafee743a67b11 Mon Sep 17 00:00:00 2001 From: Peter Klausler Date: Mon, 19 Dec 2022 12:41:25 -0800 Subject: [PATCH] [flang] Restore checking for some optional values before use Recent commits (2098ad7f00324ee0f2a6538f418a6f81dfdd2edb and 15a9a72ee68166c0cff3f036cacd3c82be66c729) replaced usage of "o.value()" on optionals with "*o". Those optional values are expected to be present -- but now, if it ever turns out that they're not, compilation will proceed with garbage data rather than crashing immediately (and more debuggably) with an uncaught exception. Add asserts for presence to restore the previous level of safety. (I could have revert these patches so as to resume used of .value() but I didn't want to just have them get broken again.) Differential Revision: https://reviews.llvm.org/D140340 --- flang/include/flang/Evaluate/tools.h | 6 ++++-- flang/lib/Lower/Bridge.cpp | 4 +++- flang/lib/Lower/ConvertCall.cpp | 4 +++- flang/lib/Lower/ConvertExpr.cpp | 11 +++++++++-- flang/lib/Lower/ConvertExprToHLFIR.cpp | 4 +++- flang/lib/Lower/CustomIntrinsicCall.cpp | 4 +++- 6 files changed, 25 insertions(+), 8 deletions(-) diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h index 3bed3e2..35e7858 100644 --- a/flang/include/flang/Evaluate/tools.h +++ b/flang/include/flang/Evaluate/tools.h @@ -583,8 +583,10 @@ template struct ConvertToKindHelper { template common::IfNoLvalue>, VALUE> ConvertToKind( int kind, VALUE &&x) { - return *common::SearchTypes( - ConvertToKindHelper{kind, std::move(x)}); + auto result{common::SearchTypes( + ConvertToKindHelper{kind, std::move(x)})}; + CHECK(result.has_value()); + return *result; } // Given a type category CAT, SameKindExprs is a variant that diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index 6e34742..76f1696 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -2769,11 +2769,13 @@ private: } else { llvm_unreachable("unknown category"); } - if (lhsIsWholeAllocatable) + if (lhsIsWholeAllocatable) { + assert(lhsRealloc.has_value()); fir::factory::finalizeRealloc(*builder, loc, *lhsMutableBox, /*lbounds=*/std::nullopt, /*takeLboundsIfRealloc=*/false, *lhsRealloc); + } }, // [2] User defined assignment. If the context is a scalar diff --git a/flang/lib/Lower/ConvertCall.cpp b/flang/lib/Lower/ConvertCall.cpp index 5ccb8a9..4b69d97 100644 --- a/flang/lib/Lower/ConvertCall.cpp +++ b/flang/lib/Lower/ConvertCall.cpp @@ -366,10 +366,12 @@ fir::ExtendedValue Fortran::lower::genCallOpAndResult( callNumResults = call.getNumResults(); } - if (caller.mustSaveResult()) + if (caller.mustSaveResult()) { + assert(allocatedResult.has_value()); builder.create(loc, callResult, fir::getBase(*allocatedResult), arrayResultShape, resultLengths); + } if (allocatedResult) { allocatedResult->match( diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp index a5ff7dc..b525c56 100644 --- a/flang/lib/Lower/ConvertExpr.cpp +++ b/flang/lib/Lower/ConvertExpr.cpp @@ -3723,8 +3723,10 @@ public: mlir::Value oldInnerArg = modifyOp.getSequence(); std::size_t offset = explicitSpace->argPosition(oldInnerArg); explicitSpace->setInnerArg(offset, fir::getBase(lexv)); - fir::ExtendedValue exv = arrayModifyToExv( - builder, loc, *explicitSpace->getLhsLoad(0), modifyOp.getResult(0)); + auto lhsLoad = explicitSpace->getLhsLoad(0); + assert(lhsLoad.has_value()); + fir::ExtendedValue exv = + arrayModifyToExv(builder, loc, *lhsLoad, modifyOp.getResult(0)); genScalarUserDefinedAssignmentCall(builder, loc, userAssignment, exv, elementalExv); } else { @@ -3899,6 +3901,7 @@ private: } mlir::Value val = builder.createConvert(loc, eleTy, origVal); if (isBoundsSpec()) { + assert(lbounds.has_value()); auto lbs = *lbounds; if (lbs.size() > 0) { // Rebox the value with user-specified shift. @@ -3908,9 +3911,11 @@ private: mlir::Value{}); } } else if (isBoundsRemap()) { + assert(lbounds.has_value()); auto lbs = *lbounds; if (lbs.size() > 0) { // Rebox the value with user-specified shift and shape. + assert(ubounds.has_value()); auto shapeShiftArgs = flatZip(lbs, *ubounds); auto shapeTy = fir::ShapeShiftType::get(eleTy.getContext(), lbs.size()); mlir::Value shapeShift = @@ -6298,6 +6303,7 @@ private: charLen = builder.createTemporary(loc, builder.getI64Type()); mlir::Value castLen = builder.createConvert(loc, builder.getI64Type(), fir::getLen(exv)); + assert(charLen.has_value()); builder.create(loc, castLen, *charLen); } } @@ -6312,6 +6318,7 @@ private: // Convert to extended value. if (fir::isa_char(seqTy.getEleTy())) { + assert(charLen.has_value()); auto len = builder.create(loc, *charLen); return {fir::CharArrayBoxValue{mem, len, extents}, /*needCopy=*/false}; } diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp index 87c1eca..2f68b29 100644 --- a/flang/lib/Lower/ConvertExprToHLFIR.cpp +++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp @@ -820,8 +820,10 @@ private: gen(const Fortran::evaluate::FunctionRef &expr) { mlir::Type resType = Fortran::lower::TypeBuilder::genType(getConverter(), expr); - return *Fortran::lower::convertCallToHLFIR( + auto result = Fortran::lower::convertCallToHLFIR( getLoc(), getConverter(), expr, resType, getSymMap(), getStmtCtx()); + assert(result.has_value()); + return *result; } template diff --git a/flang/lib/Lower/CustomIntrinsicCall.cpp b/flang/lib/Lower/CustomIntrinsicCall.cpp index 51dcbde..7772be4 100644 --- a/flang/lib/Lower/CustomIntrinsicCall.cpp +++ b/flang/lib/Lower/CustomIntrinsicCall.cpp @@ -179,8 +179,10 @@ lowerIshftc(fir::FirOpBuilder &builder, mlir::Location loc, llvm::SmallVector args; args.push_back(getOperand(0)); args.push_back(getOperand(1)); + auto iPC = isPresentCheck(2); + assert(iPC.has_value()); args.push_back(builder - .genIfOp(loc, {resultType}, *isPresentCheck(2), + .genIfOp(loc, {resultType}, *iPC, /*withElseRegion=*/true) .genThen([&]() { fir::ExtendedValue sizeExv = getOperand(2); -- 2.7.4