On Fri, Mar 14, 2014 at 10:44 AM, James Dennett <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
