From 5f1fcc43e592871b3860760a4bed56cf1be5772f Mon Sep 17 00:00:00 2001
From: Antonio Frighetto <me@antoniofrighetto.com>
Date: Wed, 13 Sep 2023 18:44:19 +0200
Subject: [PATCH] [clang] Bail out when handling union access with virtual
 inheritance

An assertion issue that arose when handling union member access with
virtual base class has been addressed. As pointed out by @zygoloid,
there is no need for further derived-to-base analysis in this instance,
so we can bail out upon encountering a virtual base class. Minor
refinement on the function name as we might not be handling a union.

Reported-By: ormris

Fixes: https://github.com/llvm/llvm-project/issues/65982
(cherry picked from commit 660876a4019b81b5a7427a3dcec5ce8c39cd1ee0)
---
 clang/lib/AST/ExprConstant.cpp                 | 17 ++++++++++++-----
 clang/test/SemaCXX/cxx2a-virtual-base-used.cpp | 11 +++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 clang/test/SemaCXX/cxx2a-virtual-base-used.cpp

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f1bad0c..c62044f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -6013,8 +6013,9 @@ const AccessKinds StartLifetimeOfUnionMemberHandler::AccessKind;
 /// operator whose left-hand side might involve a union member access. If it
 /// does, implicitly start the lifetime of any accessed union elements per
 /// C++20 [class.union]5.
-static bool HandleUnionActiveMemberChange(EvalInfo &Info, const Expr *LHSExpr,
-                                          const LValue &LHS) {
+static bool MaybeHandleUnionActiveMemberChange(EvalInfo &Info,
+                                               const Expr *LHSExpr,
+                                               const LValue &LHS) {
   if (LHS.InvalidBase || LHS.Designator.Invalid)
     return false;
 
@@ -6069,8 +6070,14 @@ static bool HandleUnionActiveMemberChange(EvalInfo &Info, const Expr *LHSExpr,
         break;
       // Walk path backwards as we walk up from the base to the derived class.
       for (const CXXBaseSpecifier *Elt : llvm::reverse(ICE->path())) {
+        if (Elt->isVirtual()) {
+          // A class with virtual base classes never has a trivial default
+          // constructor, so S(E) is empty in this case.
+          E = nullptr;
+          break;
+        }
+
         --PathLength;
-        (void)Elt;
         assert(declaresSameEntity(Elt->getType()->getAsCXXRecordDecl(),
                                   LHS.Designator.Entries[PathLength]
                                       .getAsBaseOrMember().getPointer()));
@@ -7748,7 +7755,7 @@ public:
         // per C++20 [class.union]5.
         if (Info.getLangOpts().CPlusPlus20 && OCE &&
             OCE->getOperator() == OO_Equal && MD->isTrivial() &&
-            !HandleUnionActiveMemberChange(Info, Args[0], ThisVal))
+            !MaybeHandleUnionActiveMemberChange(Info, Args[0], ThisVal))
           return false;
 
         Args = Args.slice(1);
@@ -8621,7 +8628,7 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
     return false;
 
   if (Info.getLangOpts().CPlusPlus20 &&
-      !HandleUnionActiveMemberChange(Info, E->getLHS(), Result))
+      !MaybeHandleUnionActiveMemberChange(Info, E->getLHS(), Result))
     return false;
 
   return handleAssignment(this->Info, E, Result, E->getLHS()->getType(),
diff --git a/clang/test/SemaCXX/cxx2a-virtual-base-used.cpp b/clang/test/SemaCXX/cxx2a-virtual-base-used.cpp
new file mode 100644
index 0000000..196a3ab
--- /dev/null
+++ b/clang/test/SemaCXX/cxx2a-virtual-base-used.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -std=c++20 -verify=cxx20 -triple=x86_64-linux-gnu %s
+// Fixes assertion triggered by https://github.com/llvm/llvm-project/issues/65982
+
+struct A { int y; };
+struct B : virtual public A {};
+struct X : public B {};
+
+void member_with_virtual_inheritance() {
+  X x;
+  x.B::y = 1;
+}
-- 
2.7.4