[clang] 660876a - [clang] Bail out when handling union access with virtual inheritance

[Antonio Frighetto via cfe-commits]

Author: Antonio Frighetto
Date: 2023-09-14T08:48:59+02:00
New Revision: 660876a4019b81b5a7427a3dcec5ce8c39cd1ee0

URL: https://github.com/llvm/llvm-project/commit/660876a4019b81b5a7427a3dcec5ce8c39cd1ee0
DIFF: https://github.com/llvm/llvm-project/commit/660876a4019b81b5a7427a3dcec5ce8c39cd1ee0.diff

LOG: [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

Added: 
    clang/test/SemaCXX/cxx2a-virtual-base-used.cpp

Modified: 
    clang/lib/AST/ExprConstant.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index dfa48e9c030b6a3..fea06b97259fe31 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -6062,8 +6062,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;
 
@@ -6118,8 +6119,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()));
@@ -7806,7 +7813,7 @@ class ExprEvaluatorBase
         // 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);
@@ -8679,7 +8686,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 000000000000000..196a3ab05564b96
--- /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;
+}