[clang] fb2c761 - [clang][bytecode] Fix comparing pointers pointing to base classes (#146285)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 5 10:42:53 PDT 2025
Author: Timm Baeder
Date: 2025-07-05T19:42:50+02:00
New Revision: fb2c7610e831646c5e01986306e8771730c937ff
URL: https://github.com/llvm/llvm-project/commit/fb2c7610e831646c5e01986306e8771730c937ff
DIFF: https://github.com/llvm/llvm-project/commit/fb2c7610e831646c5e01986306e8771730c937ff.diff
LOG: [clang][bytecode] Fix comparing pointers pointing to base classes (#146285)
In the attached test case, one pointer points to the `Derived` class and
one to `Base`, but they should compare equal. They didn't because those
two bases are saved at different offsets in the block. Use
`computeOffsetForComparison` not just for unions and fix it to work in
the more general cases.
Added:
Modified:
clang/lib/AST/ByteCode/Interp.h
clang/lib/AST/ByteCode/Pointer.cpp
clang/lib/AST/ByteCode/Pointer.h
clang/test/AST/ByteCode/literals.cpp
clang/test/AST/ByteCode/new-delete.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f9623131809e5..5d760f2f891e1 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1139,30 +1139,12 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
}
if (Pointer::hasSameBase(LHS, RHS)) {
- if (LHS.inUnion() && RHS.inUnion()) {
- // If the pointers point into a union, things are a little more
- // complicated since the offset we save in interp::Pointer can't be used
- // to compare the pointers directly.
- size_t A = LHS.computeOffsetForComparison();
- size_t B = RHS.computeOffsetForComparison();
- S.Stk.push<BoolT>(BoolT::from(Fn(Compare(A, B))));
- return true;
- }
-
- unsigned VL = LHS.getByteOffset();
- unsigned VR = RHS.getByteOffset();
- // In our Pointer class, a pointer to an array and a pointer to the first
- // element in the same array are NOT equal. They have the same Base value,
- // but a
diff erent Offset. This is a pretty rare case, so we fix this here
- // by comparing pointers to the first elements.
- if (!LHS.isZero() && LHS.isArrayRoot())
- VL = LHS.atIndex(0).getByteOffset();
- if (!RHS.isZero() && RHS.isArrayRoot())
- VR = RHS.atIndex(0).getByteOffset();
-
- S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
+ size_t A = LHS.computeOffsetForComparison();
+ size_t B = RHS.computeOffsetForComparison();
+ S.Stk.push<BoolT>(BoolT::from(Fn(Compare(A, B))));
return true;
}
+
// Otherwise we need to do a bunch of extra checks before returning Unordered.
if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && !RHS.isZero() &&
RHS.getOffset() == 0) {
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 0ad47645d39cc..f049f1f2936cc 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -338,16 +338,28 @@ void Pointer::print(llvm::raw_ostream &OS) const {
}
}
-/// Compute an integer that can be used to compare this pointer to
-/// another one.
size_t Pointer::computeOffsetForComparison() const {
+ if (isIntegralPointer())
+ return asIntPointer().Value + Offset;
+ if (isTypeidPointer())
+ return reinterpret_cast<uintptr_t>(asTypeidPointer().TypePtr) + Offset;
+
if (!isBlockPointer())
return Offset;
size_t Result = 0;
Pointer P = *this;
- while (!P.isRoot()) {
- if (P.isArrayRoot()) {
+ while (true) {
+
+ if (P.isVirtualBaseClass()) {
+ Result += getInlineDesc()->Offset;
+ P = P.getBase();
+ continue;
+ }
+
+ if (P.isBaseClass()) {
+ if (P.getRecord()->getNumVirtualBases() > 0)
+ Result += P.getInlineDesc()->Offset;
P = P.getBase();
continue;
}
@@ -358,14 +370,26 @@ size_t Pointer::computeOffsetForComparison() const {
continue;
}
+ if (P.isRoot()) {
+ if (P.isOnePastEnd())
+ ++Result;
+ break;
+ }
+
if (const Record *R = P.getBase().getRecord(); R && R->isUnion()) {
// Direct child of a union - all have offset 0.
P = P.getBase();
continue;
}
+ // Fields, etc.
Result += P.getInlineDesc()->Offset;
+ if (P.isOnePastEnd())
+ ++Result;
+
P = P.getBase();
+ if (P.isRoot())
+ break;
}
return Result;
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 0234ab02ab8f6..d525f84fd6605 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -761,6 +761,9 @@ class Pointer {
/// Prints the pointer.
void print(llvm::raw_ostream &OS) const;
+ /// Compute an integer that can be used to compare this pointer to
+ /// another one. This is usually NOT the same as the pointer offset
+ /// regarding the AST record layout.
size_t computeOffsetForComparison() const;
private:
diff --git a/clang/test/AST/ByteCode/literals.cpp b/clang/test/AST/ByteCode/literals.cpp
index 699746c0b2c4a..ddf1d2bebdbd0 100644
--- a/clang/test/AST/ByteCode/literals.cpp
+++ b/clang/test/AST/ByteCode/literals.cpp
@@ -1418,3 +1418,15 @@ constexpr int usingDirectiveDecl() {
return FB;
}
static_assert(usingDirectiveDecl() == 10, "");
+
+namespace OnePastEndCmp {
+ struct S {
+ int a;
+ int b;
+ };
+
+ constexpr S s{12,13};
+ constexpr const int *p = &s.a;
+ constexpr const int *q = &s.a + 1;
+ static_assert(p != q, "");
+}
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 9c293e5d15fc8..840736f332250 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -1022,6 +1022,42 @@ namespace OpNewNothrow {
// both-note {{in call to}}
}
+namespace BaseCompare {
+ struct Cmp {
+ void *p;
+
+ template<typename T>
+ constexpr Cmp(T *t) : p(t) {}
+
+ constexpr friend bool operator==(Cmp a, Cmp b) {
+ return a.p == b.p;
+ }
+ };
+
+ class Base {};
+ class Derived : public Base {};
+ constexpr bool foo() {
+ Derived *D = std::allocator<Derived>{}.allocate(1);;
+ std::construct_at<Derived>(D);
+
+ Derived *d = D;
+ Base *b = D;
+
+ Cmp ca(d);
+ Cmp cb(b);
+
+ if (ca == cb) {
+ std::allocator<Derived>{}.deallocate(D);
+ return true;
+ }
+ std::allocator<Derived>{}.deallocate(D);
+
+ return false;
+
+ }
+ static_assert(foo());
+}
+
#else
/// Make sure we reject this prior to C++20
constexpr int a() { // both-error {{never produces a constant expression}}
More information about the cfe-commits
mailing list