Skip to content

Commit 26b5446

Browse files
committed
[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.
1 parent 113ea3d commit 26b5446

File tree

3 files changed

+136
-11
lines changed

3 files changed

+136
-11
lines changed

clang/lib/AST/ByteCode/Interp.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1948,8 +1948,10 @@ bool StoreBitField(InterpState &S, CodePtr OpPC) {
19481948
const Pointer &Ptr = S.Stk.peek<Pointer>();
19491949
if (!CheckStore(S, OpPC, Ptr))
19501950
return false;
1951-
if (Ptr.canBeInitialized())
1951+
if (Ptr.canBeInitialized()) {
19521952
Ptr.initialize();
1953+
Ptr.activate();
1954+
}
19531955
if (const auto *FD = Ptr.getField())
19541956
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
19551957
else
@@ -1963,8 +1965,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
19631965
const Pointer &Ptr = S.Stk.pop<Pointer>();
19641966
if (!CheckStore(S, OpPC, Ptr))
19651967
return false;
1966-
if (Ptr.canBeInitialized())
1968+
if (Ptr.canBeInitialized()) {
19671969
Ptr.initialize();
1970+
Ptr.activate();
1971+
}
19681972
if (const auto *FD = Ptr.getField())
19691973
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
19701974
else

clang/lib/AST/ByteCode/Pointer.cpp

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,19 @@ void Pointer::activate() const {
481481
auto activate = [](Pointer &P) -> void {
482482
P.getInlineDesc()->IsActive = true;
483483
};
484-
auto deactivate = [](Pointer &P) -> void {
484+
485+
std::function<void(Pointer &)> deactivate;
486+
deactivate = [&deactivate](Pointer &P) -> void {
485487
P.getInlineDesc()->IsActive = false;
488+
489+
if (const Record *R = P.getRecord()) {
490+
for (const Record::Field &F : R->fields()) {
491+
Pointer FieldPtr = P.atField(F.Offset);
492+
if (FieldPtr.getInlineDesc()->IsActive)
493+
deactivate(FieldPtr);
494+
}
495+
// FIXME: Bases?
496+
}
486497
};
487498

488499
// Unions might be nested etc., so find the topmost Pointer that's
@@ -492,21 +503,31 @@ void Pointer::activate() const {
492503
UnionPtr = UnionPtr.getBase();
493504

494505
assert(UnionPtr.getFieldDesc()->isUnion());
495-
496506
const Record *UnionRecord = UnionPtr.getRecord();
507+
508+
// The direct child pointer of the union that's on the path from
509+
// this pointer to the union.
510+
Pointer ChildPtr = *this;
511+
assert(ChildPtr != UnionPtr);
512+
while (true) {
513+
if (ChildPtr.getBase() == UnionPtr)
514+
break;
515+
ChildPtr = ChildPtr.getBase();
516+
}
517+
assert(ChildPtr.getBase() == UnionPtr);
518+
497519
for (const Record::Field &F : UnionRecord->fields()) {
498520
Pointer FieldPtr = UnionPtr.atField(F.Offset);
499-
if (FieldPtr == *this) {
521+
if (FieldPtr == ChildPtr) {
522+
// No need to deactivate, will be activated in the next loop.
500523
} else {
501524
deactivate(FieldPtr);
502-
// FIXME: Recurse.
503525
}
504526
}
505527

506528
Pointer B = *this;
507529
while (B != UnionPtr) {
508530
activate(B);
509-
// FIXME: Need to de-activate other fields of parent records.
510531
B = B.getBase();
511532
}
512533
}

clang/test/AST/ByteCode/unions.cpp

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=expected,both %s
2-
// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
3-
// RUN: %clang_cc1 -verify=ref,both %s
4-
// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
1+
// RUN: %clang_cc1 -verify=expected,both %s -fexperimental-new-constant-interpreter
2+
// RUN: %clang_cc1 -std=c++20 -verify=expected,both %s -fexperimental-new-constant-interpreter
3+
// RUN: %clang_cc1 -verify=ref,both %s
4+
// RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
55

66
union U {
77
int a;
@@ -612,6 +612,106 @@ namespace CopyEmptyUnion {
612612
}
613613
static_assert(foo() == 1);
614614
}
615+
616+
namespace BitFields {
617+
constexpr bool simple() {
618+
union U {
619+
unsigned a : 1;
620+
unsigned b : 1;
621+
};
622+
623+
U u{1};
624+
u.b = 1;
625+
return u.b;
626+
}
627+
static_assert(simple());
628+
}
629+
630+
namespace deactivateRecurses {
631+
632+
constexpr int foo() {
633+
struct A {
634+
struct {
635+
int a;
636+
};
637+
int b;
638+
};
639+
struct B {
640+
struct {
641+
int a;
642+
int b;
643+
};
644+
};
645+
646+
union U {
647+
A a;
648+
B b;
649+
} u;
650+
651+
u.b.a = 10;
652+
++u.b.a;
653+
654+
u.a.a = 10;
655+
++u.a.a;
656+
657+
if (__builtin_constant_p(u.b.a))
658+
return 10;
659+
660+
return 1;
661+
}
662+
static_assert(foo() == 1);
663+
}
664+
665+
namespace AnonymousUnion {
666+
struct Long {
667+
struct {
668+
unsigned is_long;
669+
};
670+
unsigned Size;
671+
};
672+
673+
struct Short {
674+
struct {
675+
unsigned is_long;
676+
unsigned Size;
677+
};
678+
char data;
679+
};
680+
681+
union Rep {
682+
Short S;
683+
Long L;
684+
};
685+
686+
#define assert_active(F) if (!__builtin_is_within_lifetime(&F)) (1/0);
687+
#define assert_inactive(F) if ( __builtin_is_within_lifetime(&F)) (1/0);
688+
consteval int test() {
689+
union UU {
690+
struct {
691+
Rep R;
692+
int a;
693+
};
694+
} U;
695+
696+
U.R.S.Size = 10;
697+
assert_active(U);
698+
assert_active(U.R);
699+
assert_active(U.R.S);
700+
assert_active(U.R.S.Size);
701+
702+
U.a = 10;
703+
assert_active(U.a);
704+
assert_active(U);
705+
706+
assert_active(U);
707+
assert_active(U.R);
708+
assert_active(U.R.S);
709+
assert_active(U.R.S.Size);
710+
711+
return 1;
712+
}
713+
static_assert(test() == 1);
714+
}
615715
#endif
616716

617717
namespace AddressComparison {

0 commit comments

Comments
 (0)