[cfe-commits] [PATCH] Handle evaluation and ICE-checking of self-referencing constant integers without crashing

[Eli Friedman]

Examples of issues:

(C or C++):
int a() { const int t=t; switch(1) { case t:; } }

(C++)
extern const int a,b;
const int a=b,b=a;
int a() { if (a) return 1; return 0; }

Doug, since you were the last person to touch this code recently,
would you mind taking a look?  I somehow get the feeling I'm missing
something, but I'm not sure what.

-Eli
-------------- next part --------------
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h	(revision 89989)
+++ include/clang/AST/Decl.h	(working copy)
@@ -351,10 +351,17 @@
   /// \brief Whether this statement was already evaluated.
   bool WasEvaluated : 1;
 
+  /// \brief Whether this statement is being evaluated.
+  bool IsEvaluating : 1;
+
   /// \brief Whether we already checked whether this statement was an
   /// integral constant expression.
   bool CheckedICE : 1;
 
+  /// \brief Whether we are checking whether this statement is an
+  /// integral constant expression.
+  bool CheckingICE : 1;
+
   /// \brief Whether this statement is an integral constant
   /// expression. Only valid if CheckedICE is true.
   bool IsICE : 1;
@@ -502,9 +509,7 @@
 
   void setInit(ASTContext &C, Expr *I);
 
-  /// \brief Note that constant evaluation has computed the given
-  /// value for this variable's initializer.
-  void setEvaluatedValue(ASTContext &C, const APValue &Value) const {
+  EvaluatedStmt *EnsureEvaluatedStmt(ASTContext &C) const {
     EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>();
     if (!Eval) {
       Stmt *S = Init.get<Stmt *>();
@@ -512,7 +517,29 @@
       Eval->Value = S;
       Init = Eval;
     }
+    return Eval;
+  }
 
+  /// \brief Check whether we are in the process of checking whether the
+  /// initializer can be evaluated.
+  bool isEvaluatingValue() const {
+    if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
+      return Eval->IsEvaluating;
+
+    return false;
+  }
+
+  /// \brief Note that we now are checking whether the initializer can be
+  /// evaluated.
+  void setEvaluatingValue(ASTContext &C) const {
+    EvaluatedStmt *Eval = EnsureEvaluatedStmt(C);
+    Eval->IsEvaluating = true;
+  }
+
+  /// \brief Note that constant evaluation has computed the given
+  /// value for this variable's initializer.
+  void setEvaluatedValue(ASTContext &C, const APValue &Value) const {
+    EvaluatedStmt *Eval = EnsureEvaluatedStmt(C);
     Eval->WasEvaluated = true;
     Eval->Evaluated = Value;
   }
@@ -546,17 +573,27 @@
     return Init.get<EvaluatedStmt *>()->IsICE;
   }
 
+  /// \brief Check whether we are in the process of checking the initializer
+  /// is an integral constant expression.
+  bool isCheckingICE() const {
+    if (EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>())
+      return Eval->CheckingICE;
+
+    return false;
+  }
+
+  /// \brief Note that we now are checking whether the initializer is an
+  /// integral constant expression.
+  void setCheckingICE(ASTContext &C) const {
+    EvaluatedStmt *Eval = EnsureEvaluatedStmt(C);
+    Eval->CheckingICE = true;
+  }
+
   /// \brief Note that we now know whether the initializer is an
   /// integral constant expression.
   void setInitKnownICE(ASTContext &C, bool IsICE) const {
-    EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>();
-    if (!Eval) {
-      Stmt *S = Init.get<Stmt *>();
-      Eval = new (C) EvaluatedStmt;
-      Eval->Value = S;
-      Init = Eval;
-    }
-
+    EvaluatedStmt *Eval = EnsureEvaluatedStmt(C);
+    Eval->CheckingICE = false;
     Eval->CheckedICE = true;
     Eval->IsICE = IsICE;
   }
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp	(revision 89989)
+++ lib/AST/ExprConstant.cpp	(working copy)
@@ -867,13 +867,17 @@
     if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
       const VarDecl *Def = 0;
       if (const Expr *Init = VD->getDefinition(Def)) {
-        if (APValue *V = VD->getEvaluatedValue())
+        if (APValue *V = VD->getEvaluatedValue()) {
           return Success(V->getInt(), E);
-          
-        if (Visit(const_cast<Expr*>(Init))) {
-          // Cache the evaluated value in the variable declaration.
-          VD->setEvaluatedValue(Info.Ctx, Result);
-          return true;
+        } else if (VD->isEvaluatingValue()) {
+          // Recursive loop; don't try to visit.
+        } else {
+          VD->setEvaluatingValue(Info.Ctx);
+          if (Visit(const_cast<Expr*>(Init))) {
+            // Cache the evaluated value in the variable declaration.
+            VD->setEvaluatedValue(Info.Ctx, Result);
+            return true;
+          }
         }
 
         return false;
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp	(revision 89989)
+++ lib/AST/Expr.cpp	(working copy)
@@ -1600,7 +1600,12 @@
             Dcl->setInitKnownICE(Ctx, false);
             return ICEDiag(2, cast<DeclRefExpr>(E)->getLocation());
           }
-          
+
+          if (Dcl->isCheckingICE()) {
+            return ICEDiag(2, cast<DeclRefExpr>(E)->getLocation());
+          }
+
+          Dcl->setCheckingICE(Ctx);
           ICEDiag Result = CheckICE(Init, Ctx);
           // Cache the result of the ICE test.
           Dcl->setInitKnownICE(Ctx, Result.Val == 0);