[flang-commits] [flang] a823419 - [flang] Restore checking for some optional values before use

Peter Klausler via flang-commits flang-commits at lists.llvm.org
Thu Dec 29 09:37:46 PST 2022


Author: Peter Klausler
Date: 2022-12-29T09:37:34-08:00
New Revision: a8234196c58396c0505ac93983dafee743a67b11

URL: https://github.com/llvm/llvm-project/commit/a8234196c58396c0505ac93983dafee743a67b11
DIFF: https://github.com/llvm/llvm-project/commit/a8234196c58396c0505ac93983dafee743a67b11.diff

LOG: [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

Added: 
    

Modified: 
    flang/include/flang/Evaluate/tools.h
    flang/lib/Lower/Bridge.cpp
    flang/lib/Lower/ConvertCall.cpp
    flang/lib/Lower/ConvertExpr.cpp
    flang/lib/Lower/ConvertExprToHLFIR.cpp
    flang/lib/Lower/CustomIntrinsicCall.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 3bed3e2408705..35e785843d9f4 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -583,8 +583,10 @@ template <TypeCategory TOCAT, typename VALUE> struct ConvertToKindHelper {
 template <TypeCategory TOCAT, typename VALUE>
 common::IfNoLvalue<Expr<SomeKind<TOCAT>>, VALUE> ConvertToKind(
     int kind, VALUE &&x) {
-  return *common::SearchTypes(
-      ConvertToKindHelper<TOCAT, VALUE>{kind, std::move(x)});
+  auto result{common::SearchTypes(
+      ConvertToKindHelper<TOCAT, VALUE>{kind, std::move(x)})};
+  CHECK(result.has_value());
+  return *result;
 }
 
 // Given a type category CAT, SameKindExprs<CAT, N> is a variant that

diff  --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6e34742eb22c8..76f1696bdee94 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2769,11 +2769,13 @@ class FirConverter : public Fortran::lower::AbstractConverter {
               } 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 5ccb8a9e65615..4b69d972bc709 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<fir::SaveResultOp>(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 a5ff7dcd12a17..b525c56aa76ee 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -3723,8 +3723,10 @@ class ArrayExprLowering {
       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 @@ class ArrayExprLowering {
     }
     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 @@ class ArrayExprLowering {
                                            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 @@ class ArrayExprLowering {
         charLen = builder.createTemporary(loc, builder.getI64Type());
         mlir::Value castLen =
             builder.createConvert(loc, builder.getI64Type(), fir::getLen(exv));
+        assert(charLen.has_value());
         builder.create<fir::StoreOp>(loc, castLen, *charLen);
       }
     }
@@ -6312,6 +6318,7 @@ class ArrayExprLowering {
 
     // Convert to extended value.
     if (fir::isa_char(seqTy.getEleTy())) {
+      assert(charLen.has_value());
       auto len = builder.create<fir::LoadOp>(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 87c1ecab11d6e..2f68b29a885fc 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -820,8 +820,10 @@ class HlfirBuilder {
   gen(const Fortran::evaluate::FunctionRef<T> &expr) {
     mlir::Type resType =
         Fortran::lower::TypeBuilder<T>::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 <typename T>

diff  --git a/flang/lib/Lower/CustomIntrinsicCall.cpp b/flang/lib/Lower/CustomIntrinsicCall.cpp
index 51dcbdeec3c02..7772be4071e57 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<fir::ExtendedValue> 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);


        


More information about the flang-commits mailing list