[PATCH] Fix a crash (assertion failure) in EvaluateAsRValue.

[James Dennett]

jdennett added you to the CC list for the revision "Fix a crash (assertion failure) in EvaluateAsRValue.".

Hi klimek,

Gracefully fail to evaluate a constant expression if its type is
unknown, rather than failing an assertion trying to access the type.

http://llvm-reviews.chandlerc.com/D3075

Files:
  lib/AST/ExprConstant.cpp
  unittests/AST/CMakeLists.txt
  unittests/AST/EvaluateAsRValueTest.cpp

Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -8035,6 +8035,9 @@
 /// EvaluateAsRValue - Try to evaluate this expression, performing an implicit
 /// lvalue-to-rvalue cast if it is an lvalue.
 static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {
+  if (E->getType().isNull())
+    return false;
+
   if (!CheckLiteralType(Info, E))
     return false;
 
@@ -8062,6 +8065,13 @@
     IsConst = true;
     return true;
   }
+
+  // This case should be rare, but we need to check it before we check on
+  // the type below.
+  if (Exp->getType().isNull()) {
+    IsConst = false;
+    return true;
+  }
   
   // FIXME: Evaluating values of large array and record types can cause
   // performance problems. Only do so in C++11 for now.
Index: unittests/AST/CMakeLists.txt
===================================================================
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -10,6 +10,7 @@
   CommentParser.cpp
   DeclPrinterTest.cpp
   DeclTest.cpp
+  EvaluateAsRValueTest.cpp
   ExternalASTSourceTest.cpp
   SourceLocationTest.cpp
   StmtPrinterTest.cpp
Index: unittests/AST/EvaluateAsRValueTest.cpp
===================================================================
--- /dev/null
+++ unittests/AST/EvaluateAsRValueTest.cpp
@@ -0,0 +1,111 @@
+//===- unittests/AST/EvaluateAsRValueTest.cpp -----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// \file
+// \brief Unit tests for evaluation of constant initializers.
+//
+//===----------------------------------------------------------------------===//
+
+#include <map>
+#include <string>
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+#include "clang/AST/ASTConsumer.h"
+
+using namespace clang::tooling;
+
+namespace {
+// For each variable name encountered, whether its initializer was a
+// constant.
+typedef std::map<std::string, bool> VarInfoMap;
+
+/// \brief Records information on variable initializers to a map.
+class EvaluateConstantInitializersVisitor
+    : public clang::RecursiveASTVisitor<EvaluateConstantInitializersVisitor> {
+ public:
+  explicit EvaluateConstantInitializersVisitor(VarInfoMap &VarInfo)
+      : VarInfo(VarInfo) {}
+  /// \brief Checks that isConstantInitializer and EvaluateAsRValue agree
+  /// and don't crash.
+  ///
+  /// For each VarDecl with an initializer this also records in VarInfo
+  /// whether the initializer could be evaluated as a constant.
+  bool VisitVarDecl(const clang::VarDecl *VD) {
+    if (const clang::Expr *Init = VD->getInit()) {
+      clang::Expr::EvalResult Result;
+      bool WasEvaluated = Init->EvaluateAsRValue(Result, VD->getASTContext());
+      VarInfo[VD->getNameAsString()] = WasEvaluated;
+      EXPECT_EQ(WasEvaluated, Init->isConstantInitializer(VD->getASTContext(),
+                                                          false /*ForRef*/));
+    }
+    return true;
+  }
+
+ private:
+  VarInfoMap &VarInfo;
+};
+
+class EvaluateConstantInitializersAction : public clang::ASTFrontendAction {
+ public:
+  clang::ASTConsumer *CreateASTConsumer(clang::CompilerInstance &Compiler,
+                                        llvm::StringRef FilePath) override {
+    return new Consumer;
+  }
+
+ private:
+  class Consumer : public clang::ASTConsumer {
+   public:
+    ~Consumer() override {}
+
+    void HandleTranslationUnit(clang::ASTContext &Ctx) override {
+      VarInfoMap VarInfo;
+      EvaluateConstantInitializersVisitor Evaluator(VarInfo);
+      Evaluator.TraverseDecl(Ctx.getTranslationUnitDecl());
+      EXPECT_EQ(2u, VarInfo.size());
+      EXPECT_FALSE(VarInfo["Dependent"]);
+      EXPECT_TRUE(VarInfo["Constant"]);
+      EXPECT_EQ(2u, VarInfo.size());
+    }
+  };
+};
+}
+
+TEST(EvaluateAsRValue, FailsGracefullyForUnknownTypes) {
+  // This is a regression test; the AST library used to trigger assertion
+  // failures because it assumed that the type of initializers was always
+  // known (which is true only after template instantiation).
+  std::string ModesToTest[] = {"-std=c++03", "-std=c++11", "-std=c++1y"};
+  for (std::string const &Mode : ModesToTest) {
+    std::vector<std::string> Args(1, Mode);
+    ASSERT_TRUE(runToolOnCodeWithArgs(
+      new EvaluateConstantInitializersAction(),
+      R"(template <typename T>
+         struct vector {
+           explicit vector(int size);
+         };
+         template <typename R>
+         struct S {
+           vector<R> intervals() const {
+             vector<R> Dependent(2);
+             return Dependent;
+           }
+         };
+         void doSomething() {
+           int Constant = 2 + 2;
+           (void) Constant;
+         }
+      )",
+      Args));
+  }
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3075.3.patch
Type: text/x-patch
Size: 5163 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140313/ff1e8b70/attachment.bin>