[flang-commits] [flang] 4d8c00c - [flang] Fix crash on erroneous expressions

[Pete Steinfeld via flang-commits]

Author: Pete Steinfeld
Date: 2020-06-05T08:46:43-07:00
New Revision: 4d8c00c0595b6998e41373afadaaca3f51ae2cf8

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

LOG: [flang] Fix crash on erroneous expressions

Summary:
If you create an expression with parse errors, the `parser::Expr.typedExpr`
will be empty, which causes a compiler crash.  The crash is caused by the
check in check-do-forall.cpp that scans all expresssions to see if `DO`
variables are being modified.

It turned out that the problem was that I was fetching subexpressions of type
`parser::Expr`, which are not guaranteed to have a non-null `typedExpr`.  I
fixed this by only grabbing the top-level expression from which to gather
arguments as part of the DO loop analysis.  This, in turn, exposed a problem
where I wasn't collecting all of the actual arguments in an expression.  This
was caused by the fact that I wasn't recursing through the rest of the
expression after finding an argument.  I fixed this by recursing through the
argument in the member function in `CollectActualArgumentsHelper`.

Reviewers: klausler, tskeith, DavidTruby

Subscribers: llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D81101

Added: 
    

Modified: 
    flang/lib/Semantics/check-do-forall.cpp
    flang/lib/Semantics/check-do-forall.h
    flang/test/Semantics/resolve91.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Semantics/check-do-forall.cpp b/flang/lib/Semantics/check-do-forall.cpp
index 503a28412102..a53df602c0d7 100644
--- a/flang/lib/Semantics/check-do-forall.cpp
+++ b/flang/lib/Semantics/check-do-forall.cpp
@@ -1034,7 +1034,8 @@ struct CollectActualArgumentsHelper
   CollectActualArgumentsHelper() : Base{*this} {}
   using Base::operator();
   ActualArgumentSet operator()(const evaluate::ActualArgument &arg) const {
-    return ActualArgumentSet{arg};
+    return Combine(ActualArgumentSet{arg},
+        CollectActualArgumentsHelper{}(arg.UnwrapExpr()));
   }
 };
 
@@ -1044,11 +1045,16 @@ template <typename A> ActualArgumentSet CollectActualArguments(const A &x) {
 
 template ActualArgumentSet CollectActualArguments(const SomeExpr &);
 
+void DoForallChecker::Enter(const parser::Expr &parsedExpr) { ++exprDepth_; }
+
 void DoForallChecker::Leave(const parser::Expr &parsedExpr) {
-  if (const SomeExpr * expr{GetExpr(parsedExpr)}) {
-    ActualArgumentSet argSet{CollectActualArguments(*expr)};
-    for (const evaluate::ActualArgumentRef &argRef : argSet) {
-      CheckIfArgIsDoVar(*argRef, parsedExpr.source, context_);
+  CHECK(exprDepth_ > 0);
+  if (--exprDepth_ == 0) { // Only check top level expressions
+    if (const SomeExpr * expr{GetExpr(parsedExpr)}) {
+      ActualArgumentSet argSet{CollectActualArguments(*expr)};
+      for (const evaluate::ActualArgumentRef &argRef : argSet) {
+        CheckIfArgIsDoVar(*argRef, parsedExpr.source, context_);
+      }
     }
   }
 }

diff  --git a/flang/lib/Semantics/check-do-forall.h b/flang/lib/Semantics/check-do-forall.h
index 8f0cdc75ac97..3b2ae59f5f3f 100644
--- a/flang/lib/Semantics/check-do-forall.h
+++ b/flang/lib/Semantics/check-do-forall.h
@@ -50,6 +50,7 @@ class DoForallChecker : public virtual BaseChecker {
   void Leave(const parser::ForallStmt &);
   void Leave(const parser::ForallAssignmentStmt &s);
   void Enter(const parser::ExitStmt &);
+  void Enter(const parser::Expr &);
   void Leave(const parser::Expr &);
   void Leave(const parser::InquireSpec &);
   void Leave(const parser::IoControlSpec &);
@@ -58,6 +59,7 @@ class DoForallChecker : public virtual BaseChecker {
 
 private:
   SemanticsContext &context_;
+  int exprDepth_{0};
 
   void SayBadLeave(
       StmtType, const char *enclosingStmt, const ConstructNode &) const;

diff  --git a/flang/test/Semantics/resolve91.f90 b/flang/test/Semantics/resolve91.f90
index 0a375b873aa4..2fbcaa384dfc 100644
--- a/flang/test/Semantics/resolve91.f90
+++ b/flang/test/Semantics/resolve91.f90
@@ -51,3 +51,15 @@ module m5
   !ERROR: The type of 'a' has already been implicitly declared
   character(len=len(b)) :: a
 end module m5
+
+module m6
+  integer, dimension(3) :: iarray
+  !ERROR: Derived type 'ubound' not found
+  character(len=ubound(iarray)(1)) :: first
+end module m6
+
+module m7
+  integer, dimension(2) :: iarray
+  !ERROR: Derived type 'ubound' not found
+  integer :: ivar = ubound(iarray)(1)
+end module m7