From 26b5446d582e1dade9aea0091be36cf813effb4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= Date: Thu, 3 Jul 2025 09:07:08 +0200 Subject: [PATCH] [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(); 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() = Value.truncate(FD->getBitWidthValue()); else @@ -1963,8 +1965,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) { const Pointer &Ptr = S.Stk.pop(); 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() = 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 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 {