Skip to content

[clang][bytecode] Assert on virtual func call from array elem #158502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 6, 2025
Merged

[clang][bytecode] Assert on virtual func call from array elem #158502

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c01c2b
[Clang][Interp] Assert on virtual func call from array elem
mariusdr Sep 14, 2025
95717b9
Add diagnostic when dynamic type is not constant
mariusdr Sep 21, 2025
57bc74e
clang-format
mariusdr Sep 21, 2025
89cbeb7
do not use `CheckConstant`, add test cases
mariusdr Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 38 additions & 17 deletions clang/lib/AST/ByteCode/Interp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
return true;
}

bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc,
bool NoDiag) {
assert(Desc);

const auto *D = Desc->asVarDecl();
Expand All @@ -470,15 +471,15 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
}

if (IsConstant) {
if (S.getLangOpts().CPlusPlus) {
if (S.getLangOpts().CPlusPlus && !NoDiag) {
S.CCEDiag(S.Current->getLocation(OpPC),
S.getLangOpts().CPlusPlus11
? diag::note_constexpr_ltor_non_constexpr
: diag::note_constexpr_ltor_non_integral,
1)
<< D << T;
S.Note(D->getLocation(), diag::note_declared_at);
} else {
} else if (!NoDiag) {
S.CCEDiag(S.Current->getLocation(OpPC));
}
return true;
Expand All @@ -493,16 +494,18 @@ bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
return true;
}

diagnoseNonConstVariable(S, OpPC, D);
if (!NoDiag)
diagnoseNonConstVariable(S, OpPC, D);
return false;
}

static bool CheckConstant(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
static bool CheckConstant(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
bool NoDiag = false) {
if (!Ptr.isStatic() || !Ptr.isBlockPointer())
return true;
if (!Ptr.getDeclID())
return true;
return CheckConstant(S, OpPC, Ptr.getDeclDesc());
return CheckConstant(S, OpPC, Ptr.getDeclDesc(), NoDiag);
}

bool CheckNull(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
Expand Down Expand Up @@ -1636,6 +1639,33 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
return true;
}

static bool GetDynamicDecl(InterpState &S, CodePtr OpPC, Pointer TypePtr,
const CXXRecordDecl *&DynamicDecl) {
while (TypePtr.isBaseClass())
TypePtr = TypePtr.getBase();

QualType DynamicType = TypePtr.getType();
if (DynamicType->isPointerType() || DynamicType->isReferenceType()) {
DynamicDecl = DynamicType->getPointeeCXXRecordDecl();
} else if (DynamicType->isArrayType()) {
const Type *ElemType = DynamicType->getPointeeOrArrayElementType();
assert(ElemType);
DynamicDecl = ElemType->getAsCXXRecordDecl();
} else {
DynamicDecl = DynamicType->getAsCXXRecordDecl();
}

if (!CheckConstant(S, OpPC, TypePtr, true)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is actually the relevant part of CheckConstant? Is it just the call to QualType::isConstant()?

Copy link
Contributor Author

@mariusdr mariusdr Sep 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking only DynamicType.isConstant() would reject valid code like

  struct A {
    constexpr virtual void foo(int &a) {
      a = 1;
    }
  };

  constexpr int m() {
    A b;
    int a;
    b.foo(a);
    return a;
  }
  static_assert(m() == 1, "");

here TypePtr.isStatic() in GetDynamicDecl() is false so CheckConstant() would return early but I'm not sure if it is sufficient to only check those two things. Tests pass at least with if (TypePtr.isStatic() && !DynamicType.isConstant(S.getASTContext())) instead of CheckConstant().

Copy link
Contributor Author

@mariusdr mariusdr Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is actually the relevant part of CheckConstant? Is it just the call to QualType::isConstant()?

Removed the call to CheckConstant, added more test cases. Check should be more explicit now, i.e. check storage class of this, and if its var decl is constexpr.

const Expr *E = S.Current->getExpr(OpPC);
APValue V = TypePtr.toAPValue(S.getASTContext());
QualType TT = S.getASTContext().getLValueReferenceType(DynamicType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did Pointer::toDiagnosticString() not work here?

Copy link
Contributor Author

@mariusdr mariusdr Sep 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output is slightly different because toDiagnosticString() (~ APValue::getAsString(V, TypePtr.getType()) adds a '&' prefix to the output while the code above does not, because the input QualType is not a reference type, refer to:

llvm-project/clang/lib/AST/APValue.cpp

Lines 819 to 821 in 6a7aec1

if (!IsReference)
Out << '&';
else if (isLValueOnePastTheEnd())

we would get messages like

note: virtual function called on object '&k' whose dynamic type is not constant

while the other interpreter outputs

note: virtual function called on object 'k' whose dynamic type is not constant

S.FFDiag(E, diag::note_constexpr_polymorphic_unknown_dynamic_type)
<< AccessKinds::AK_MemberCall << V.getAsString(S.getASTContext(), TT);
return false;
}
return true;
}

bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func,
uint32_t VarArgSize) {
assert(Func->hasThisPointer());
Expand All @@ -1660,17 +1690,8 @@ bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func,
}

const CXXRecordDecl *DynamicDecl = nullptr;
{
Pointer TypePtr = ThisPtr;
while (TypePtr.isBaseClass())
TypePtr = TypePtr.getBase();

QualType DynamicType = TypePtr.getType();
if (DynamicType->isPointerType() || DynamicType->isReferenceType())
DynamicDecl = DynamicType->getPointeeCXXRecordDecl();
else
DynamicDecl = DynamicType->getAsCXXRecordDecl();
}
if (!GetDynamicDecl(S, OpPC, ThisPtr, DynamicDecl))
return false;
assert(DynamicDecl);

const auto *StaticDecl = cast<CXXRecordDecl>(Func->getParentDecl());
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/ByteCode/Interp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ bool CheckDowncast(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);

/// Checks if the Descriptor is of a constexpr or const global variable.
bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc);
bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc,
bool NoDiag = false);

/// Checks if a pointer points to a mutable field.
bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
Expand Down
42 changes: 42 additions & 0 deletions clang/test/AST/ByteCode/cxx20.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,10 @@ namespace Virtual {
virtual constexpr int f() { return 10; }
};

K k;
static_assert(k.f() == 10); // both-error {{not an integral constant expression}} \
// both-note {{virtual function called on object 'k' whose dynamic type is not constant}}

class L : public K {
public:
int b = f();
Expand All @@ -1083,6 +1087,18 @@ namespace Virtual {
static_assert(l.a == 10);
static_assert(l.b == 10);
static_assert(l.c == 10);

struct M {
K& mk = k;
};
static_assert(M{}.mk.f() == 10); // both-error {{not an integral constant expression}} \
// both-note {{virtual function called on object 'k' whose dynamic type is not constant}}

struct N {
K* mk = &k;
};
static_assert(N{}.mk->f() == 10); // both-error {{not an integral constant expression}} \
// both-note {{virtual function called on object 'k' whose dynamic type is not constant}}
}

namespace DiscardedTrivialCXXConstructExpr {
Expand All @@ -1100,3 +1116,29 @@ namespace DiscardedTrivialCXXConstructExpr {
constexpr int y = foo(12); // both-error {{must be initialized by a constant expression}} \
// both-note {{in call to}}
}

namespace VirtualFunctionCallThroughArrayElem {
struct X {
constexpr virtual int foo() const {
return 3;
}
};
constexpr X xs[5];
static_assert(xs[3].foo() == 3);

constexpr X xs2[1][2];
static_assert(xs2[0].foo() == 3); // both-error {{is not a structure or union}}
static_assert(xs2[0][0].foo() == 3);

struct Y: public X {
constexpr int foo() const override {
return 1;
}
};
constexpr Y ys[20];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you include the original reproducer from #152893 as well? This patch fixes the assertion failure but the diagnostic output is still different between the two interpreters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added that test case and the diagnostic. For normal globals the interpreters also disagreed in their diagnostics outputs (https://godbolt.org/z/84zeeYxd3), now they raise the same messages.

static_assert(ys[12].foo() == static_cast<const X&>(ys[12]).foo());

X a[3][4];
static_assert(a[2][3].foo()); // both-error {{not an integral constant expression}} \
// both-note {{virtual function called on object 'a[2][3]' whose dynamic type is not constant}}
}
Loading