Skip to content

Commit 9ae7aa7

Browse files
authored
[clang][bytecode] Diagnose comparing pointers to fields... (#137159)
... with different access specifiers.
1 parent d859cb6 commit 9ae7aa7

File tree

4 files changed

+86
-1
lines changed

4 files changed

+86
-1
lines changed

clang/lib/AST/ByteCode/Interp.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,19 @@ inline bool CmpHelper<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
10071007
return false;
10081008
}
10091009

1010+
// Diagnose comparisons between fields with different access specifiers.
1011+
if (std::optional<std::pair<Pointer, Pointer>> Split =
1012+
Pointer::computeSplitPoint(LHS, RHS)) {
1013+
const FieldDecl *LF = Split->first.getField();
1014+
const FieldDecl *RF = Split->second.getField();
1015+
if (LF && RF && !LF->getParent()->isUnion() &&
1016+
LF->getAccess() != RF->getAccess()) {
1017+
S.CCEDiag(S.Current->getSource(OpPC),
1018+
diag::note_constexpr_pointer_comparison_differing_access)
1019+
<< LF << LF->getAccess() << RF << RF->getAccess() << LF->getParent();
1020+
}
1021+
}
1022+
10101023
unsigned VL = LHS.getByteOffset();
10111024
unsigned VR = RHS.getByteOffset();
10121025
S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));

clang/lib/AST/ByteCode/Pointer.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,48 @@ bool Pointer::pointsToLiteral() const {
571571
return E && !isa<MaterializeTemporaryExpr, StringLiteral>(E);
572572
}
573573

574+
std::optional<std::pair<Pointer, Pointer>>
575+
Pointer::computeSplitPoint(const Pointer &A, const Pointer &B) {
576+
if (!A.isBlockPointer() || !B.isBlockPointer())
577+
return std::nullopt;
578+
579+
if (A.asBlockPointer().Pointee != B.asBlockPointer().Pointee)
580+
return std::nullopt;
581+
if (A.isRoot() && B.isRoot())
582+
return std::nullopt;
583+
584+
if (A == B)
585+
return std::make_pair(A, B);
586+
587+
auto getBase = [](const Pointer &P) -> Pointer {
588+
if (P.isArrayElement())
589+
return P.expand().getArray();
590+
return P.getBase();
591+
};
592+
593+
Pointer IterA = A;
594+
Pointer IterB = B;
595+
Pointer CurA = IterA;
596+
Pointer CurB = IterB;
597+
for (;;) {
598+
if (IterA.asBlockPointer().Base > IterB.asBlockPointer().Base) {
599+
CurA = IterA;
600+
IterA = getBase(IterA);
601+
} else {
602+
CurB = IterB;
603+
IterB = getBase(IterB);
604+
}
605+
606+
if (IterA == IterB)
607+
return std::make_pair(CurA, CurB);
608+
609+
if (IterA.isRoot() && IterB.isRoot())
610+
return std::nullopt;
611+
}
612+
613+
llvm_unreachable("The loop above should've returned.");
614+
}
615+
574616
std::optional<APValue> Pointer::toRValue(const Context &Ctx,
575617
QualType ResultType) const {
576618
const ASTContext &ASTCtx = Ctx.getASTContext();

clang/lib/AST/ByteCode/Pointer.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,11 @@ class Pointer {
492492
return ElemDesc ? ElemDesc->ElemRecord : nullptr;
493493
}
494494
/// Returns the field information.
495-
const FieldDecl *getField() const { return getFieldDesc()->asFieldDecl(); }
495+
const FieldDecl *getField() const {
496+
if (const Descriptor *FD = getFieldDesc())
497+
return FD->asFieldDecl();
498+
return nullptr;
499+
}
496500

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

731+
static std::optional<std::pair<Pointer, Pointer>>
732+
computeSplitPoint(const Pointer &A, const Pointer &B);
733+
727734
/// Whether this points to a block that's been created for a "literal lvalue",
728735
/// i.e. a non-MaterializeTemporaryExpr Expr.
729736
bool pointsToLiteral() const;

clang/test/AST/ByteCode/records.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,3 +1787,26 @@ namespace IntegralBaseCast {
17871787

17881788
static_assert(f() == 0, "");
17891789
}
1790+
1791+
namespace AccessMismatch {
1792+
struct A {
1793+
public:
1794+
constexpr A() : a(0), b(0) {}
1795+
int a;
1796+
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}}
1797+
private:
1798+
int b;
1799+
};
1800+
static_assert(A().cmp(), ""); // both-error {{constant expression}} \
1801+
// both-note {{in call}}
1802+
1803+
class B {
1804+
public:
1805+
A a;
1806+
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}}
1807+
protected:
1808+
A b;
1809+
};
1810+
static_assert(B().cmp(), ""); // both-error {{constant expression}} \
1811+
// both-note {{in call}}
1812+
}

0 commit comments

Comments
 (0)