[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, rsmith.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

`MarkVarDeclODRUsed` indirectly calls `captureInBlock`, which creates a copy expression. The copy expression is insulated in it's own `ExpressionEvaluationContext`, so it saves, mutates, and restores `MaybeODRUseExprs` as `CleanupVarDeclMarking` is iterating through it, leading to a crash. Fix this by iterating through a local copy of `MaybeODRUseExprs`.

rdar://47493525


Repository:
  rC Clang

https://reviews.llvm.org/D59670

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/blocks.cpp


Index: clang/test/SemaCXX/blocks.cpp
===================================================================
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -145,3 +145,11 @@
             A::foo(); });
   }
 }
+
+namespace test7 {
+struct S {};
+void f() {
+  constexpr S s;
+  auto some_block = ^{ (void)s; };
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15686,7 +15686,12 @@
 }
 
 void Sema::CleanupVarDeclMarking() {
-  for (Expr *E : MaybeODRUseExprs) {
+  // Iterate through a local copy in case MarkVarDeclODRUsed makes a recursive
+  // call.
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+
+  for (Expr *E : LocalMaybeODRUseExprs) {
     VarDecl *Var;
     SourceLocation Loc;
     if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
@@ -15702,8 +15707,6 @@
     MarkVarDeclODRUsed(Var, Loc, *this,
                        /*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
-
-  MaybeODRUseExprs.clear();
 }
 
 
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -587,13 +587,13 @@
   /// element type here is ExprWithCleanups::Object.
   SmallVector<BlockDecl*, 8> ExprCleanupObjects;
 
-  /// Store a list of either DeclRefExprs or MemberExprs
-  ///  that contain a reference to a variable (constant) that may or may not
-  ///  be odr-used in this Expr, and we won't know until all lvalue-to-rvalue
-  ///  and discarded value conversions have been applied to all subexpressions
-  ///  of the enclosing full expression.  This is cleared at the end of each
-  ///  full expression.
-  llvm::SmallPtrSet<Expr*, 2> MaybeODRUseExprs;
+  /// Store a set of either DeclRefExprs or MemberExprs that contain a reference
+  /// to a variable (constant) that may or may not be odr-used in this Expr, and
+  /// we won't know until all lvalue-to-rvalue and discarded value conversions
+  /// have been applied to all subexpressions of the enclosing full expression.
+  /// This is cleared at the end of each full expression.
+  using MaybeODRUseExprSet = llvm::SmallPtrSet<Expr *, 2>;
+  MaybeODRUseExprSet MaybeODRUseExprs;
 
   std::unique_ptr<sema::FunctionScopeInfo> PreallocatedFunctionScope;
 
@@ -1029,7 +1029,7 @@
     /// context (i.e. the number of TypoExprs created).
     unsigned NumTypos;
 
-    llvm::SmallPtrSet<Expr*, 2> SavedMaybeODRUseExprs;
+    MaybeODRUseExprSet SavedMaybeODRUseExprs;
 
     /// The lambdas that are present within this context, if it
     /// is indeed an unevaluated context.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59670.191780.patch
Type: text/x-patch
Size: 2753 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190321/2db768fc/attachment-0001.bin>