Skip to content

Commit e93893f

Browse files
tbaederrgithub-actions[bot]
tbaederr
authored and
github-actions[bot]
committed
Automerge: [clang][bytecode] Add AccessFlags to Block (#152590)
This way, we can check a single uint8_t for != 0 to know whether this block is accessible or not. If not, we still need to figure out why not and diagnose appropriately of course.
2 parents 83911a7 + 7a6c981 commit e93893f

File tree

12 files changed

+143
-90
lines changed

12 files changed

+143
-90
lines changed

‎clang/lib/AST/ByteCode/Compiler.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7140,10 +7140,6 @@ bool Compiler<Emitter>::emitDestruction(const Descriptor *Desc,
71407140
assert(!Desc->isPrimitive());
71417141
assert(!Desc->isPrimitiveArray());
71427142

7143-
// Can happen if the decl is invalid.
7144-
if (Desc->isDummy())
7145-
return true;
7146-
71477143
// Arrays.
71487144
if (Desc->isArray()) {
71497145
const Descriptor *ElemDesc = Desc->ElemDesc;

‎clang/lib/AST/ByteCode/Descriptor.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ Descriptor::Descriptor(const DeclTy &D, const Record *R, MetadataSize MD,
367367
Descriptor::Descriptor(const DeclTy &D, MetadataSize MD)
368368
: Source(D), ElemSize(1), Size(1), MDSize(MD.value_or(0)),
369369
AllocSize(MDSize), ElemRecord(nullptr), IsConst(true), IsMutable(false),
370-
IsTemporary(false), IsDummy(true) {
370+
IsTemporary(false) {
371371
assert(Source && "Missing source");
372372
}
373373

@@ -455,7 +455,7 @@ SourceInfo Descriptor::getLoc() const {
455455
}
456456

457457
bool Descriptor::hasTrivialDtor() const {
458-
if (isPrimitive() || isPrimitiveArray() || isDummy())
458+
if (isPrimitive() || isPrimitiveArray())
459459
return true;
460460

461461
if (isRecord()) {
@@ -464,8 +464,9 @@ bool Descriptor::hasTrivialDtor() const {
464464
return !Dtor || Dtor->isTrivial();
465465
}
466466

467+
if (!ElemDesc)
468+
return true;
467469
// Composite arrays.
468-
assert(ElemDesc);
469470
return ElemDesc->hasTrivialDtor();
470471
}
471472

‎clang/lib/AST/ByteCode/Descriptor.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,6 @@ struct Descriptor final {
166166
const bool IsVolatile = false;
167167
/// Flag indicating if the block is an array.
168168
const bool IsArray = false;
169-
/// Flag indicating if this is a dummy descriptor.
170-
bool IsDummy = false;
171169
bool IsConstexprUnknown = false;
172170

173171
/// Storage management methods.
@@ -203,9 +201,6 @@ struct Descriptor final {
203201
/// Allocates a dummy descriptor.
204202
Descriptor(const DeclTy &D, MetadataSize MD = std::nullopt);
205203

206-
/// Make this descriptor a dummy descriptor.
207-
void makeDummy() { IsDummy = true; }
208-
209204
QualType getType() const;
210205
QualType getElemQualType() const;
211206
QualType getDataType(const ASTContext &Ctx) const;
@@ -273,8 +268,6 @@ struct Descriptor final {
273268
bool isRecord() const { return !IsArray && ElemRecord; }
274269
/// Checks if the descriptor is of a union.
275270
bool isUnion() const;
276-
/// Checks if this is a dummy descriptor.
277-
bool isDummy() const { return IsDummy; }
278271

279272
/// Whether variables of this descriptor need their destructor called or not.
280273
bool hasTrivialDtor() const;

‎clang/lib/AST/ByteCode/Disasm.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ LLVM_DUMP_METHOD void Program::dump(llvm::raw_ostream &OS) const {
338338
}
339339

340340
OS << "\n";
341-
if (GP.isInitialized() && Desc->isPrimitive() && !Desc->isDummy()) {
341+
if (GP.isInitialized() && Desc->isPrimitive() && !G->block()->isDummy()) {
342342
OS << " ";
343343
{
344344
ColorScope SC(OS, true, {llvm::raw_ostream::BRIGHT_CYAN, false});
@@ -394,8 +394,6 @@ LLVM_DUMP_METHOD void Descriptor::dump(llvm::raw_ostream &OS) const {
394394
else if (isUnknownSizeArray())
395395
OS << " unknown-size-array";
396396

397-
if (isDummy())
398-
OS << " dummy";
399397
if (IsConstexprUnknown)
400398
OS << " constexpr-unknown";
401399
}
@@ -541,11 +539,12 @@ LLVM_DUMP_METHOD void Block::dump(llvm::raw_ostream &OS) const {
541539
else
542540
OS << "-\n";
543541
OS << " Pointers: " << NPointers << "\n";
544-
OS << " Dead: " << IsDead << "\n";
542+
OS << " Dead: " << isDead() << "\n";
545543
OS << " Static: " << IsStatic << "\n";
546-
OS << " Extern: " << IsExtern << "\n";
544+
OS << " Extern: " << isExtern() << "\n";
547545
OS << " Initialized: " << IsInitialized << "\n";
548-
OS << " Weak: " << IsWeak << "\n";
546+
OS << " Weak: " << isWeak() << "\n";
547+
OS << " Dummy: " << isDummy() << '\n';
549548
OS << " Dynamic: " << IsDynamic << "\n";
550549
}
551550

‎clang/lib/AST/ByteCode/DynamicAllocator.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ void DynamicAllocator::cleanup() {
2424
auto &AllocSite = Iter.second;
2525
for (auto &Alloc : AllocSite.Allocations) {
2626
Block *B = Alloc.block();
27-
assert(!B->IsDead);
27+
assert(!B->isDead());
2828
assert(B->isInitialized());
2929
B->invokeDtor();
3030

@@ -121,15 +121,15 @@ bool DynamicAllocator::deallocate(const Expr *Source,
121121
assert(!Site.empty());
122122

123123
// Find the Block to delete.
124-
auto AllocIt = llvm::find_if(Site.Allocations, [&](const Allocation &A) {
124+
auto *AllocIt = llvm::find_if(Site.Allocations, [&](const Allocation &A) {
125125
return BlockToDelete == A.block();
126126
});
127127

128128
assert(AllocIt != Site.Allocations.end());
129129

130130
Block *B = AllocIt->block();
131131
assert(B->isInitialized());
132-
assert(!B->IsDead);
132+
assert(!B->isDead());
133133
B->invokeDtor();
134134

135135
// Almost all our dynamic allocations have a pointer pointing to them
@@ -139,7 +139,7 @@ bool DynamicAllocator::deallocate(const Expr *Source,
139139
// over to a DeadBlock and simply keep the block in a separate DeadAllocations
140140
// list.
141141
if (B->hasPointers()) {
142-
B->IsDead = true;
142+
B->AccessFlags |= Block::DeadFlag;
143143
DeadAllocations.push_back(std::move(*AllocIt));
144144
Site.Allocations.erase(AllocIt);
145145

‎clang/lib/AST/ByteCode/Interp.cpp

Lines changed: 77 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -738,19 +738,21 @@ static bool CheckWeak(InterpState &S, CodePtr OpPC, const Block *B) {
738738
bool CheckGlobalLoad(InterpState &S, CodePtr OpPC, const Block *B) {
739739
const auto &Desc =
740740
*reinterpret_cast<const GlobalInlineDescriptor *>(B->rawData());
741-
if (!CheckExtern(S, OpPC, Pointer(const_cast<Block *>(B))))
742-
return false;
741+
if (!B->isAccessible()) {
742+
if (!CheckExtern(S, OpPC, Pointer(const_cast<Block *>(B))))
743+
return false;
744+
if (!CheckDummy(S, OpPC, B, AK_Read))
745+
return false;
746+
return CheckWeak(S, OpPC, B);
747+
}
748+
743749
if (!CheckConstant(S, OpPC, B->getDescriptor()))
744750
return false;
745-
if (!CheckDummy(S, OpPC, B, AK_Read))
746-
return false;
747751
if (Desc.InitState != GlobalInitState::Initialized)
748752
return DiagnoseUninitialized(S, OpPC, B->isExtern(), B->getDescriptor(),
749753
AK_Read);
750754
if (!CheckTemporary(S, OpPC, B, AK_Read))
751755
return false;
752-
if (!CheckWeak(S, OpPC, B))
753-
return false;
754756
if (B->getDescriptor()->IsVolatile) {
755757
if (!S.getLangOpts().CPlusPlus)
756758
return Invalid(S, OpPC);
@@ -790,14 +792,32 @@ bool CheckLocalLoad(InterpState &S, CodePtr OpPC, const Block *B) {
790792

791793
bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
792794
AccessKinds AK) {
793-
if (!CheckLive(S, OpPC, Ptr, AK))
794-
return false;
795-
if (!CheckExtern(S, OpPC, Ptr))
795+
if (!Ptr.isBlockPointer()) {
796+
if (Ptr.isZero()) {
797+
const auto &Src = S.Current->getSource(OpPC);
798+
799+
if (Ptr.isField())
800+
S.FFDiag(Src, diag::note_constexpr_null_subobject) << CSK_Field;
801+
else
802+
S.FFDiag(Src, diag::note_constexpr_access_null) << AK;
803+
}
796804
return false;
805+
}
806+
807+
// Block pointers are the only ones we can actually read from.
808+
if (!Ptr.block()->isAccessible()) {
809+
if (!CheckLive(S, OpPC, Ptr, AK))
810+
return false;
811+
if (!CheckExtern(S, OpPC, Ptr))
812+
return false;
813+
if (!CheckDummy(S, OpPC, Ptr.block(), AK))
814+
return false;
815+
if (!CheckWeak(S, OpPC, Ptr.block()))
816+
return false;
817+
}
818+
797819
if (!CheckConstant(S, OpPC, Ptr))
798820
return false;
799-
if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK))
800-
return false;
801821
if (!CheckRange(S, OpPC, Ptr, AK))
802822
return false;
803823
if (!CheckActive(S, OpPC, Ptr, AK))
@@ -806,10 +826,9 @@ bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
806826
return false;
807827
if (!Ptr.isInitialized())
808828
return DiagnoseUninitialized(S, OpPC, Ptr, AK);
809-
if (Ptr.isBlockPointer() && !CheckTemporary(S, OpPC, Ptr.block(), AK))
810-
return false;
811-
if (Ptr.isBlockPointer() && !CheckWeak(S, OpPC, Ptr.block()))
829+
if (!CheckTemporary(S, OpPC, Ptr.block(), AK))
812830
return false;
831+
813832
if (!CheckMutable(S, OpPC, Ptr))
814833
return false;
815834
if (!CheckVolatile(S, OpPC, Ptr, AK))
@@ -820,35 +839,54 @@ bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
820839
/// This is not used by any of the opcodes directly. It's used by
821840
/// EvalEmitter to do the final lvalue-to-rvalue conversion.
822841
bool CheckFinalLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
823-
if (!CheckLive(S, OpPC, Ptr, AK_Read))
842+
if (!Ptr.isBlockPointer()) {
843+
if (Ptr.isZero()) {
844+
const auto &Src = S.Current->getSource(OpPC);
845+
846+
if (Ptr.isField())
847+
S.FFDiag(Src, diag::note_constexpr_null_subobject) << CSK_Field;
848+
else
849+
S.FFDiag(Src, diag::note_constexpr_access_null) << AK_Read;
850+
}
824851
return false;
852+
}
853+
854+
assert(Ptr.isBlockPointer());
855+
if (!Ptr.block()->isAccessible()) {
856+
if (!CheckLive(S, OpPC, Ptr, AK_Read))
857+
return false;
858+
if (!CheckExtern(S, OpPC, Ptr))
859+
return false;
860+
if (!CheckDummy(S, OpPC, Ptr.block(), AK_Read))
861+
return false;
862+
return CheckWeak(S, OpPC, Ptr.block());
863+
}
864+
825865
if (!CheckConstant(S, OpPC, Ptr))
826866
return false;
827867

828-
if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Read))
829-
return false;
830-
if (!CheckExtern(S, OpPC, Ptr))
831-
return false;
832868
if (!CheckActive(S, OpPC, Ptr, AK_Read))
833869
return false;
834870
if (!CheckLifetime(S, OpPC, Ptr.getLifetime(), AK_Read))
835871
return false;
836872
if (!Ptr.isInitialized())
837873
return DiagnoseUninitialized(S, OpPC, Ptr, AK_Read);
838-
if (Ptr.isBlockPointer() && !CheckTemporary(S, OpPC, Ptr.block(), AK_Read))
839-
return false;
840-
if (Ptr.isBlockPointer() && !CheckWeak(S, OpPC, Ptr.block()))
874+
if (!CheckTemporary(S, OpPC, Ptr.block(), AK_Read))
841875
return false;
842876
if (!CheckMutable(S, OpPC, Ptr))
843877
return false;
844878
return true;
845879
}
846880

847881
bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
848-
if (!CheckLive(S, OpPC, Ptr, AK_Assign))
849-
return false;
850-
if (Ptr.isBlockPointer() && !CheckDummy(S, OpPC, Ptr.block(), AK_Assign))
882+
if (!Ptr.isBlockPointer())
851883
return false;
884+
885+
if (!Ptr.block()->isAccessible()) {
886+
if (!CheckLive(S, OpPC, Ptr, AK_Assign))
887+
return false;
888+
return CheckDummy(S, OpPC, Ptr.block(), AK_Assign);
889+
}
852890
if (!CheckLifetime(S, OpPC, Ptr.getLifetime(), AK_Assign))
853891
return false;
854892
if (!CheckExtern(S, OpPC, Ptr))
@@ -1124,11 +1162,10 @@ bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) {
11241162
}
11251163

11261164
bool CheckDummy(InterpState &S, CodePtr OpPC, const Block *B, AccessKinds AK) {
1127-
const Descriptor *Desc = B->getDescriptor();
1128-
if (!Desc->isDummy())
1165+
if (!B->isDummy())
11291166
return true;
11301167

1131-
const ValueDecl *D = Desc->asValueDecl();
1168+
const ValueDecl *D = B->getDescriptor()->asValueDecl();
11321169
if (!D)
11331170
return false;
11341171

@@ -1835,14 +1872,21 @@ bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
18351872
if (Ptr.inUnion() && Ptr.getBase().getRecord()->isUnion())
18361873
Ptr.activate();
18371874

1875+
if (!Ptr.isBlockPointer())
1876+
return false;
1877+
18381878
// Similar to CheckStore(), but with the additional CheckTemporary() call and
18391879
// the AccessKinds are different.
1880+
1881+
if (!Ptr.block()->isAccessible()) {
1882+
if (!CheckExtern(S, OpPC, Ptr))
1883+
return false;
1884+
if (!CheckLive(S, OpPC, Ptr, AK_Construct))
1885+
return false;
1886+
return CheckDummy(S, OpPC, Ptr.block(), AK_Construct);
1887+
}
18401888
if (!CheckTemporary(S, OpPC, Ptr.block(), AK_Construct))
18411889
return false;
1842-
if (!CheckLive(S, OpPC, Ptr, AK_Construct))
1843-
return false;
1844-
if (!CheckDummy(S, OpPC, Ptr.block(), AK_Construct))
1845-
return false;
18461890

18471891
// CheckLifetime for this and all base pointers.
18481892
for (Pointer P = Ptr;;) {
@@ -1853,8 +1897,7 @@ bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
18531897
break;
18541898
P = P.getBase();
18551899
}
1856-
if (!CheckExtern(S, OpPC, Ptr))
1857-
return false;
1900+
18581901
if (!CheckRange(S, OpPC, Ptr, AK_Construct))
18591902
return false;
18601903
if (!CheckGlobal(S, OpPC, Ptr))

‎clang/lib/AST/ByteCode/InterpBlock.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void Block::removePointer(Pointer *P) {
6464
}
6565

6666
void Block::cleanup() {
67-
if (Pointers == nullptr && !IsDynamic && IsDead)
67+
if (Pointers == nullptr && !IsDynamic && isDead())
6868
(reinterpret_cast<DeadBlock *>(this + 1) - 1)->free();
6969
}
7070

@@ -113,8 +113,8 @@ bool Block::hasPointer(const Pointer *P) const {
113113
#endif
114114

115115
DeadBlock::DeadBlock(DeadBlock *&Root, Block *Blk)
116-
: Root(Root), B(~0u, Blk->Desc, Blk->IsStatic, Blk->IsExtern, Blk->IsWeak,
117-
/*isDead=*/true) {
116+
: Root(Root), B(~0u, Blk->Desc, Blk->isExtern(), Blk->IsStatic,
117+
Blk->isWeak(), Blk->isDummy(), /*IsDead=*/true) {
118118
// Add the block to the chain of dead blocks.
119119
if (Root)
120120
Root->Prev = this;

0 commit comments

Comments
 (0)