[PATCH] D108482: [Clang] Fix instantiation of OpaqueValueExprs (Bug #45964)

[Jason Rice via Phabricator via cfe-commits]

ricejasonf updated this revision to Diff 368485.
ricejasonf added a comment.

I added a simple lit test. The case is the reduced case mentioned in the bug. I spent some time trying to reproduce this outside decomposition. The only other way (I know of) to produce an ArrayInitLoopExpr is an implicit copy constructor, but the AST for that is generated lazily so it never gets transformed. Other entities containing OpaqueValueExpr do not appear to have this problem either. Let me know if there are any more deficiencies or suggested changes. Thank you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108482/new/

https://reviews.llvm.org/D108482

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/pr45964-nested-ove.cpp


Index: clang/test/SemaCXX/pr45964-nested-ove.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/pr45964-nested-ove.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++17 -emit-codegen-only -verify %s
+// Don't crash (Bugzilla - Bug 45964).
+
+// non-dependent ArrayInitLoop should not, upon instantiation,
+// contain an OpaqueValueExpr with a nested OpaqueValueExpr or an
+// uninstantiated source expr. (triggers asserts in CodeGen)
+
+// expected-no-diagnostics
+int a[1];
+template <int> void b() {
+  auto [c] = a;
+}
+void (*d)(){b<0>};
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -10511,7 +10511,19 @@
 TreeTransform<Derived>::TransformOpaqueValueExpr(OpaqueValueExpr *E) {
   assert((!E->getSourceExpr() || getDerived().AlreadyTransformed(E->getType())) &&
          "opaque value expression requires transformation");
-  return E;
+
+  // Note that SourceExpr can be nullptr
+  ExprResult SourceExpr = TransformExpr(E->getSourceExpr());
+  if (SourceExpr.isInvalid())
+    return ExprError();
+  if (SourceExpr.get() == E->getSourceExpr() && !getDerived().AlwaysRebuild()) {
+    return E;
+  }
+
+  OpaqueValueExpr *New = new (SemaRef.Context)
+      OpaqueValueExpr(E->getExprLoc(), E->getType(), E->getValueKind(),
+                      E->getObjectKind(), SourceExpr.get());
+  return New;
 }
 
 template<typename Derived>
Index: clang/lib/Sema/SemaInit.cpp
===================================================================
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8643,9 +8643,13 @@
 
     case SK_ArrayLoopIndex: {
       Expr *Cur = CurInit.get();
-      Expr *BaseExpr = new (S.Context)
-          OpaqueValueExpr(Cur->getExprLoc(), Cur->getType(),
-                          Cur->getValueKind(), Cur->getObjectKind(), Cur);
+      // prevent nested OpaqueValueExprs
+      Expr *BaseExpr = dyn_cast<OpaqueValueExpr>(Cur);
+      if (!BaseExpr) {
+        BaseExpr = new (S.Context)
+            OpaqueValueExpr(Cur->getExprLoc(), Cur->getType(),
+                            Cur->getValueKind(), Cur->getObjectKind(), Cur);
+      }
       Expr *IndexExpr =
           new (S.Context) ArrayInitIndexExpr(S.Context.getSizeType());
       CurInit = S.CreateBuiltinArraySubscriptExpr(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108482.368485.patch
Type: text/x-patch
Size: 2420 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210824/3c66f9bb/attachment-0001.bin>