Skip to content

Commit

Permalink
[clang] Bail out when handling union access with virtual inheritance
Browse files Browse the repository at this point in the history
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: llvm/llvm-project#65982
(cherry picked from commit 660876a4019b81b5a7427a3dcec5ce8c39cd1ee0)
  • Loading branch information
antoniofrighetto authored and llvmbot committed Sep 25, 2023
1 parent 4813589 commit 7ae78be
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
17 changes: 12 additions & 5 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -7748,7 +7755,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);
Expand Down Expand Up @@ -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(),
Expand Down
11 changes: 11 additions & 0 deletions clang/test/SemaCXX/cxx2a-virtual-base-used.cpp
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 7ae78be

Please sign in to comment.