[PATCH] D77041: [AST] Fix a crash on invalid constexpr Ctorinitializer when building RecoveryExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
hokein edited the summary of this revision.
hokein added inline comments.
hokein added a reviewer: sammccall.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14359
+        !ErrorsInCtorInitializer &&
         !CheckConstexprFunctionDefinition(FD, CheckConstexprKind::Diagnose))
       FD->setInvalidDecl();
----------------
The crash is in `CheckConstexprFunctionDefinition`, I tried different ways to fixing it:

1) mark `FD` invalid when there are any errors in CtorInitailizer -- clang deliberately treats CtorDecl as valid even there are some errors in the initializer to prevent spurious diagnostics (see the cycle delegation in the test as an example), so marking them invalid may affect the quality of diagnostics;

2) Fixing it inside `CheckConstexprFunctionDefinition` or `isPotentialConstantExpr`, but it doesn't seem to be a right layer, these functions are expected to be called on a validDecl (or at least after a few sanity checks), and emit diagnostics.


crash stack:

  lang:  workspace/llvm-project/clang/lib/AST/ExprConstant.cpp:13704: bool EvaluateInPlace(clang::APValue &, (anonymous namespace)::EvalInfo &, const (anonymous namespace)::LValue &, const clang::Expr *, bool): Assertion `!E->isValueDependent()' failed.
   #8  EvaluateInPlace(clang::APValue&, (anonymous namespace)::EvalInfo&, (anonymous namespace)::LValue const&, clang::Expr const*, bool)  workspace/llvm-project/clang/lib/AST/ExprConstant.cpp:0:0
   #9  HandleConstructorCall(clang::Expr const*, (anonymous namespace)::LValue const&, clang::APValue*, clang::CXXConstructorDecl const*, (anonymous namespace)::EvalInfo&, clang::APValue&)  workspace/llvm-project/clang/lib/AST/ExprConstant.cpp:5779:57
  #10  HandleConstructorCall(clang::Expr const*, (anonymous namespace)::LValue const&, llvm::ArrayRef<clang::Expr const*>, clang::CXXConstructorDecl const*, (anonymous namespace)::EvalInfo&, clang::APValue&)  workspace/llvm-project/clang/lib/AST/ExprConstant.cpp:5819:10
  #11  clang::Expr::isPotentialConstantExpr(clang::FunctionDecl const*, llvm::SmallVectorImpl<std::pair<clang::SourceLocation, clang::PartialDiagnostic> >&) workspace/llvm-project/clang/lib/AST/ExprConstant.cpp:14746:5
  #12  CheckConstexprFunctionBody(clang::Sema&, clang::FunctionDecl const*, clang::Stmt*, clang::Sema::CheckConstexprKind)  workspace/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:2306:7
  #13  clang::Sema::CheckConstexprFunctionDefinition(clang::FunctionDecl const*, clang::Sema::CheckConstexprKind)  workspace/llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:1766:0
  #14  clang::Sema::ActOnFinishFunctionBody(clang::Decl*, clang::Stmt*, bool)  workspace/llvm-project/clang/lib/Sema/SemaDecl.cpp:14357:9
  #15  clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&)  workspace/llvm-project/clang/lib/Parse/ParseStmt.cpp:2213:18


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77041

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/invalid-constructor-init.cpp


Index: clang/test/SemaCXX/invalid-constructor-init.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/invalid-constructor-init.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -frecovery-ast -verify %s
+
+struct X {
+  int Y;
+  constexpr X() : Y(foo()) {} // expected-error {{use of undeclared identifier 'foo'}}
+};
+
+struct X2 {
+  int Y = foo(); // expected-error {{use of undeclared identifier 'foo'}} \
+                 // expected-note {{subexpression not valid in a constant expression}}
+  constexpr X2() {} // expected-error {{constexpr constructor never produces a constant expression}}
+};
+
+struct CycleDelegate {
+  int Y;
+  CycleDelegate(int) : Y(foo()) {} // expected-error {{use of undeclared identifier 'foo'}}
+  // no "delegation cycle" diagnostic emitted!
+  CycleDelegate(float) : CycleDelegate(1) {}
+};
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1685,6 +1685,7 @@
 // This implements C++11 [dcl.constexpr]p3,4, as amended by DR1360.
 bool Sema::CheckConstexprFunctionDefinition(const FunctionDecl *NewFD,
                                             CheckConstexprKind Kind) {
+  assert(!NewFD->isInvalidDecl());
   const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewFD);
   if (MD && MD->isInstance()) {
     // C++11 [dcl.constexpr]p4:
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -45,6 +45,7 @@
 #include "clang/Sema/Template.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/Casting.h"
 #include <algorithm>
 #include <cstring>
 #include <functional>
@@ -14345,7 +14346,16 @@
       ActivePolicy = &WP;
     }
 
+    bool ErrorsInCtorInitializer =
+        llvm::isa_and_nonnull<CXXConstructorDecl>(FD)
+            ? llvm::any_of(llvm::dyn_cast<CXXConstructorDecl>(FD)->inits(),
+                           [](const auto *Init) {
+                             return Init->getInit() &&
+                                    Init->getInit()->containsErrors();
+                           })
+            : false;
     if (!IsInstantiation && FD && FD->isConstexpr() && !FD->isInvalidDecl() &&
+        !ErrorsInCtorInitializer &&
         !CheckConstexprFunctionDefinition(FD, CheckConstexprKind::Diagnose))
       FD->setInvalidDecl();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77041.253521.patch
Type: text/x-patch
Size: 2526 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200330/5f7ca835/attachment-0001.bin>