Skip to content

Commit db4e927

Browse files
authored
[clang][bytecode] Misc union fixes (#146824)
First, don't forget to also activate fields which are bitfields on assignment. Second, when deactivating fields, recurse into records.
1 parent 6c153e5 commit db4e927

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
@@ -1930,8 +1930,10 @@ bool StoreBitField(InterpState &S, CodePtr OpPC) {
19301930
const Pointer &Ptr = S.Stk.peek<Pointer>();
19311931
if (!CheckStore(S, OpPC, Ptr))
19321932
return false;
1933-
if (Ptr.canBeInitialized())
1933+
if (Ptr.canBeInitialized()) {
19341934
Ptr.initialize();
1935+
Ptr.activate();
1936+
}
19351937
if (const auto *FD = Ptr.getField())
19361938
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
19371939
else
@@ -1945,8 +1947,10 @@ bool StoreBitFieldPop(InterpState &S, CodePtr OpPC) {
19451947
const Pointer &Ptr = S.Stk.pop<Pointer>();
19461948
if (!CheckStore(S, OpPC, Ptr))
19471949
return false;
1948-
if (Ptr.canBeInitialized())
1950+
if (Ptr.canBeInitialized()) {
19491951
Ptr.initialize();
1952+
Ptr.activate();
1953+
}
19501954
if (const auto *FD = Ptr.getField())
19511955
Ptr.deref<T>() = Value.truncate(FD->getBitWidthValue());
19521956
else

clang/lib/AST/ByteCode/Pointer.cpp

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -505,8 +505,19 @@ void Pointer::activate() const {
505505
auto activate = [](Pointer &P) -> void {
506506
P.getInlineDesc()->IsActive = true;
507507
};
508-
auto deactivate = [](Pointer &P) -> void {
508+
509+
std::function<void(Pointer &)> deactivate;
510+
deactivate = [&deactivate](Pointer &P) -> void {
509511
P.getInlineDesc()->IsActive = false;
512+
513+
if (const Record *R = P.getRecord()) {
514+
for (const Record::Field &F : R->fields()) {
515+
Pointer FieldPtr = P.atField(F.Offset);
516+
if (FieldPtr.getInlineDesc()->IsActive)
517+
deactivate(FieldPtr);
518+
}
519+
// FIXME: Bases?
520+
}
510521
};
511522

512523
// Unions might be nested etc., so find the topmost Pointer that's
@@ -516,21 +527,31 @@ void Pointer::activate() const {
516527
UnionPtr = UnionPtr.getBase();
517528

518529
assert(UnionPtr.getFieldDesc()->isUnion());
519-
520530
const Record *UnionRecord = UnionPtr.getRecord();
531+
532+
// The direct child pointer of the union that's on the path from
533+
// this pointer to the union.
534+
Pointer ChildPtr = *this;
535+
assert(ChildPtr != UnionPtr);
536+
while (true) {
537+
if (ChildPtr.getBase() == UnionPtr)
538+
break;
539+
ChildPtr = ChildPtr.getBase();
540+
}
541+
assert(ChildPtr.getBase() == UnionPtr);
542+
521543
for (const Record::Field &F : UnionRecord->fields()) {
522544
Pointer FieldPtr = UnionPtr.atField(F.Offset);
523-
if (FieldPtr == *this) {
545+
if (FieldPtr == ChildPtr) {
546+
// No need to deactivate, will be activated in the next loop.
524547
} else {
525548
deactivate(FieldPtr);
526-
// FIXME: Recurse.
527549
}
528550
}
529551

530552
Pointer B = *this;
531553
while (B != UnionPtr) {
532554
activate(B);
533-
// FIXME: Need to de-activate other fields of parent records.
534555
B = B.getBase();
535556
}
536557
}

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)