[clang] [clang][bytecode] Misc union fixes (PR #146824)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 5 09:30:52 PDT 2025
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/146824 at github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/146824
>From df7c4f770e726efacf3a70328b31936add017eea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Sat, 5 Jul 2025 18:10:38 +0200
Subject: [PATCH 1/2] speculating
---
clang/lib/AST/ByteCode/Interp.cpp | 7 +++++++
clang/lib/AST/ByteCode/InterpState.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 51cf0c59f0b50..3de9d70c0f0e2 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -376,6 +376,9 @@ bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
}
}
+ if (S.speculating())
+ return false;
+
const SourceInfo &Loc = S.Current->getSource(OpPC);
S.FFDiag(Loc, diag::note_constexpr_access_inactive_union_member)
<< AK << InactiveField << !ActiveField << ActiveField;
@@ -702,6 +705,9 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
return false;
}
+ if (S.speculating())
+ return false;
+
if (!S.checkingPotentialConstantExpression()) {
S.FFDiag(S.Current->getSource(OpPC), diag::note_constexpr_access_uninit)
<< AK << /*uninitialized=*/true << S.Current->getRange(OpPC);
@@ -1538,6 +1544,7 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
if (!CheckCallDepth(S, OpPC))
return cleanup();
+ // llvm::errs() << "Calling " << Func->getName() << '\n';
auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize);
InterpFrame *FrameBefore = S.Current;
S.Current = NewFrame.get();
diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h
index 08765561985e2..cbf8a92f3a6e2 100644
--- a/clang/lib/AST/ByteCode/InterpState.h
+++ b/clang/lib/AST/ByteCode/InterpState.h
@@ -55,6 +55,7 @@ class InterpState final : public State, public SourceMapper {
InterpState &operator=(const InterpState &) = delete;
bool diagnosing() const { return getEvalStatus().Diag != nullptr; }
+ bool speculating() const { return SpeculationDepth != 0; }
// Stack frame accessors.
Frame *getSplitFrame() { return Parent.getCurrentFrame(); }
>From f25ea5ce16cd741a562fbe0ce0137d2949315293 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 3 Jul 2025 09:07:08 +0200
Subject: [PATCH 2/2] [clang][bytecode] Misc union fixes
First, don't forget to also activate fields which are bitfields on
assignment.
Second, when deactivating fields, recurse into records.
---
clang/lib/AST/ByteCode/Interp.h | 8 ++-
clang/lib/AST/ByteCode/Pointer.cpp | 31 +++++++--
clang/test/AST/ByteCode/unions.cpp | 108 +++++++++++++++++++++++++++--
3 files changed, 136 insertions(+), 11 deletions(-)
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f9623131809e5..ddd0b44bb37bf 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -1948,8 +1948,10 @@ bool StoreBitField(InterpState &S, CodePtr OpPC) {
const Pointer &Ptr = S.Stk.peek<Pointer>();
if (!CheckStore(S, OpPC, Ptr))
return false;
- if (Ptr.canBeInitialized())
+ if (Ptr.canBeInitialized()) {
Ptr.initialize();
+ Ptr.activate();
+ }
if (const auto *FD = Ptr.getField())
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
else
@@ -1963,8 +1965,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
const Pointer &Ptr = S.Stk.pop<Pointer>();
if (!CheckStore(S, OpPC, Ptr))
return false;
- if (Ptr.canBeInitialized())
+ if (Ptr.canBeInitialized()) {
Ptr.initialize();
+ Ptr.activate();
+ }
if (const auto *FD = Ptr.getField())
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
else
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 0ad47645d39cc..f5b0f0003a650 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -481,8 +481,19 @@ void Pointer::activate() const {
auto activate = [](Pointer &P) -> void {
P.getInlineDesc()->IsActive = true;
};
- auto deactivate = [](Pointer &P) -> void {
+
+ std::function<void(Pointer &)> deactivate;
+ deactivate = [&deactivate](Pointer &P) -> void {
P.getInlineDesc()->IsActive = false;
+
+ if (const Record *R = P.getRecord()) {
+ for (const Record::Field &F : R->fields()) {
+ Pointer FieldPtr = P.atField(F.Offset);
+ if (FieldPtr.getInlineDesc()->IsActive)
+ deactivate(FieldPtr);
+ }
+ // FIXME: Bases?
+ }
};
// Unions might be nested etc., so find the topmost Pointer that's
@@ -492,21 +503,31 @@ void Pointer::activate() const {
UnionPtr = UnionPtr.getBase();
assert(UnionPtr.getFieldDesc()->isUnion());
-
const Record *UnionRecord = UnionPtr.getRecord();
+
+ // The direct child pointer of the union that's on the path from
+ // this pointer to the union.
+ Pointer ChildPtr = *this;
+ assert(ChildPtr != UnionPtr);
+ while (true) {
+ if (ChildPtr.getBase() == UnionPtr)
+ break;
+ ChildPtr = ChildPtr.getBase();
+ }
+ assert(ChildPtr.getBase() == UnionPtr);
+
for (const Record::Field &F : UnionRecord->fields()) {
Pointer FieldPtr = UnionPtr.atField(F.Offset);
- if (FieldPtr == *this) {
+ if (FieldPtr == ChildPtr) {
+ // No need to deactivate, will be activated in the next loop.
} else {
deactivate(FieldPtr);
- // FIXME: Recurse.
}
}
Pointer B = *this;
while (B != UnionPtr) {
activate(B);
- // FIXME: Need to de-activate other fields of parent records.
B = B.getBase();
}
}
diff --git a/clang/test/AST/ByteCode/unions.cpp b/clang/test/AST/ByteCode/unions.cpp
index 0e5f83b9572b3..f97990c1ff849 100644
--- a/clang/test/AST/ByteCode/unions.cpp
+++ b/clang/test/AST/ByteCode/unions.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
-// RUN: %clang_cc1 -verify=ref,both %s
-// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
+// RUN: %clang_cc1 -verify=expected,both %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -std=c++20 -verify=expected,both %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -verify=ref,both %s
+// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
union U {
int a;
@@ -612,6 +612,106 @@ namespace CopyEmptyUnion {
}
static_assert(foo() == 1);
}
+
+namespace BitFields {
+ constexpr bool simple() {
+ union U {
+ unsigned a : 1;
+ unsigned b : 1;
+ };
+
+ U u{1};
+ u.b = 1;
+ return u.b;
+ }
+ static_assert(simple());
+}
+
+namespace deactivateRecurses {
+
+ constexpr int foo() {
+ struct A {
+ struct {
+ int a;
+ };
+ int b;
+ };
+ struct B {
+ struct {
+ int a;
+ int b;
+ };
+ };
+
+ union U {
+ A a;
+ B b;
+ } u;
+
+ u.b.a = 10;
+ ++u.b.a;
+
+ u.a.a = 10;
+ ++u.a.a;
+
+ if (__builtin_constant_p(u.b.a))
+ return 10;
+
+ return 1;
+ }
+ static_assert(foo() == 1);
+}
+
+namespace AnonymousUnion {
+ struct Long {
+ struct {
+ unsigned is_long;
+ };
+ unsigned Size;
+ };
+
+ struct Short {
+ struct {
+ unsigned is_long;
+ unsigned Size;
+ };
+ char data;
+ };
+
+ union Rep {
+ Short S;
+ Long L;
+ };
+
+#define assert_active(F) if (!__builtin_is_within_lifetime(&F)) (1/0);
+#define assert_inactive(F) if ( __builtin_is_within_lifetime(&F)) (1/0);
+ consteval int test() {
+ union UU {
+ struct {
+ Rep R;
+ int a;
+ };
+ } U;
+
+ U.R.S.Size = 10;
+ assert_active(U);
+ assert_active(U.R);
+ assert_active(U.R.S);
+ assert_active(U.R.S.Size);
+
+ U.a = 10;
+ assert_active(U.a);
+ assert_active(U);
+
+ assert_active(U);
+ assert_active(U.R);
+ assert_active(U.R.S);
+ assert_active(U.R.S.Size);
+
+ return 1;
+ }
+ static_assert(test() == 1);
+}
#endif
namespace AddressComparison {
More information about the cfe-commits
mailing list