r203950 - Fix a crash (assertion failure) in EvaluateAsRValue.

[Hans Wennborg]

On Fri, Mar 14, 2014 at 10:44 AM, James Dennett <jdennett at google.com> wrote:
> Author: jdennett
> Date: Fri Mar 14 12:44:10 2014
> New Revision: 203950
>
> URL: http://llvm.org/viewvc/llvm-project?rev=203950&view=rev
> Log:
> Fix a crash (assertion failure) in EvaluateAsRValue.
>
> Summary:
> Gracefully fail to evaluate a constant expression if its type is
> unknown, rather than failing an assertion trying to access the type.
>
> Reviewers: klimek
>
> Reviewed By: klimek
>
> CC: chandlerc, cfe-commits
>
> Differential Revision: http://llvm-reviews.chandlerc.com/D3075
>
> Added:
>     cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp
> Modified:
>     cfe/trunk/lib/AST/ExprConstant.cpp
>     cfe/trunk/unittests/AST/CMakeLists.txt
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=203950&r1=203949&r2=203950&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Mar 14 12:44:10 2014
> @@ -8034,6 +8034,9 @@ static bool EvaluateInPlace(APValue &Res
>  /// 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;
>
> @@ -8061,6 +8064,13 @@ static bool FastEvaluateAsRValue(const E
>      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.
>
> Modified: cfe/trunk/unittests/AST/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/CMakeLists.txt?rev=203950&r1=203949&r2=203950&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/AST/CMakeLists.txt (original)
> +++ cfe/trunk/unittests/AST/CMakeLists.txt Fri Mar 14 12:44:10 2014
> @@ -10,6 +10,7 @@ add_clang_unittest(ASTTests
>    CommentParser.cpp
>    DeclPrinterTest.cpp
>    DeclTest.cpp
> +  EvaluateAsRValueTest.cpp
>    ExternalASTSourceTest.cpp
>    SourceLocationTest.cpp
>    StmtPrinterTest.cpp
>
> Added: cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp?rev=203950&view=auto
> ==============================================================================
> --- cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp (added)
> +++ cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp Fri Mar 14 12:44:10 2014
> @@ -0,0 +1,112 @@
> +//===- 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;
> +         }
> +      )",

Looks like this broke at least one buildbot:
http://bb.pgr.jp/builders/cmake-clang-x86_64-linux/builds/21170

/home/bb/cmake-clang-x86_64-linux/llvm-project/clang/unittests/AST/EvaluateAsRValueTest.cpp:94:7:
error: unterminated raw string
/home/bb/cmake-clang-x86_64-linux/llvm-project/clang/unittests/AST/EvaluateAsRValueTest.cpp:109:8:
warning: missing terminating " character [enabled by default]
/home/bb/cmake-clang-x86_64-linux/llvm-project/clang/unittests/AST/EvaluateAsRValueTest.cpp:92:5:
error: stray ‘R’ in program
/home/bb/cmake-clang-x86_64-linux/llvm-project/clang/unittests/AST/EvaluateAsRValueTest.cpp:92:5:
error: missing terminating " character