Skip to content

[clang][bytecode] Diagnose comparing pointers to fields... #137159

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.

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 1 commit into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions clang/lib/AST/ByteCode/Interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,19 @@ inline bool CmpHelper<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
return false;
}

// Diagnose comparisons between fields with different access specifiers.
if (std::optional<std::pair<Pointer, Pointer>> Split =
Pointer::computeSplitPoint(LHS, RHS)) {
const FieldDecl *LF = Split->first.getField();
const FieldDecl *RF = Split->second.getField();
if (LF && RF && !LF->getParent()->isUnion() &&
LF->getAccess() != RF->getAccess()) {
S.CCEDiag(S.Current->getSource(OpPC),
diag::note_constexpr_pointer_comparison_differing_access)
<< LF << LF->getAccess() << RF << RF->getAccess() << LF->getParent();
}
}

unsigned VL = LHS.getByteOffset();
unsigned VR = RHS.getByteOffset();
S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
Expand Down
42 changes: 42 additions & 0 deletions clang/lib/AST/ByteCode/Pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,48 @@ bool Pointer::pointsToLiteral() const {
return E && !isa<MaterializeTemporaryExpr, StringLiteral>(E);
}

std::optional<std::pair<Pointer, Pointer>>
Pointer::computeSplitPoint(const Pointer &A, const Pointer &B) {
if (!A.isBlockPointer() || !B.isBlockPointer())
return std::nullopt;

if (A.asBlockPointer().Pointee != B.asBlockPointer().Pointee)
return std::nullopt;
if (A.isRoot() && B.isRoot())
return std::nullopt;

if (A == B)
return std::make_pair(A, B);

auto getBase = [](const Pointer &P) -> Pointer {
if (P.isArrayElement())
return P.expand().getArray();
return P.getBase();
};

Pointer IterA = A;
Pointer IterB = B;
Pointer CurA = IterA;
Pointer CurB = IterB;
for (;;) {
if (IterA.asBlockPointer().Base > IterB.asBlockPointer().Base) {
CurA = IterA;
IterA = getBase(IterA);
} else {
CurB = IterB;
IterB = getBase(IterB);
}

if (IterA == IterB)
return std::make_pair(CurA, CurB);

if (IterA.isRoot() && IterB.isRoot())
return std::nullopt;
}

llvm_unreachable("The loop above should've returned.");
}

std::optional<APValue> Pointer::toRValue(const Context &Ctx,
QualType ResultType) const {
const ASTContext &ASTCtx = Ctx.getASTContext();
Expand Down
9 changes: 8 additions & 1 deletion clang/lib/AST/ByteCode/Pointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,11 @@ class Pointer {
return ElemDesc ? ElemDesc->ElemRecord : nullptr;
}
/// Returns the field information.
const FieldDecl *getField() const { return getFieldDesc()->asFieldDecl(); }
const FieldDecl *getField() const {
if (const Descriptor *FD = getFieldDesc())
return FD->asFieldDecl();
return nullptr;
}

/// Checks if the storage is extern.
bool isExtern() const {
Expand Down Expand Up @@ -724,6 +728,9 @@ class Pointer {
/// Checks if both given pointers point to the same block.
static bool pointToSameBlock(const Pointer &A, const Pointer &B);

static std::optional<std::pair<Pointer, Pointer>>
computeSplitPoint(const Pointer &A, const Pointer &B);

/// Whether this points to a block that's been created for a "literal lvalue",
/// i.e. a non-MaterializeTemporaryExpr Expr.
bool pointsToLiteral() const;
Expand Down
23 changes: 23 additions & 0 deletions clang/test/AST/ByteCode/records.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1787,3 +1787,26 @@ namespace IntegralBaseCast {

static_assert(f() == 0, "");
}

namespace AccessMismatch {
struct A {
public:
constexpr A() : a(0), b(0) {}
int a;
constexpr bool cmp() const { return &a < &b; } // both-note {{comparison of address of fields 'a' and 'b' of 'A' with differing access specifiers (public vs private) has unspecified value}}
private:
int b;
};
static_assert(A().cmp(), ""); // both-error {{constant expression}} \
// both-note {{in call}}

class B {
public:
A a;
constexpr bool cmp() const { return &a.a < &b.a; } // both-note {{comparison of address of fields 'a' and 'b' of 'B' with differing access specifiers (public vs protected) has unspecified value}}
protected:
A b;
};
static_assert(B().cmp(), ""); // both-error {{constant expression}} \
// both-note {{in call}}
}
Loading