Skip to content

Commit eef8116

Browse files
authored
[clang][bytecode] Only visit local variables if they have constant init (#107576)
See the comment I added for why this is weird. We might want to have a different mechanism for this in the future. Fixes #101801
1 parent 60eb9b2 commit eef8116

File tree

4 files changed

+46
-9
lines changed

4 files changed

+46
-9
lines changed

clang/lib/AST/ByteCode/Compiler.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5615,11 +5615,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
56155615
if (isa<DecompositionDecl>(VD))
56165616
return revisit(VD);
56175617

5618-
// Visit local const variables like normal.
5619-
if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
5620-
VD->isStaticDataMember()) &&
5618+
if ((VD->hasGlobalStorage() || VD->isStaticDataMember()) &&
56215619
typeShouldBeVisited(VD->getType()))
56225620
return revisit(VD);
5621+
5622+
// FIXME: The evaluateValue() check here is a little ridiculous, since
5623+
// it will ultimately call into Context::evaluateAsInitializer(). In
5624+
// other words, we're evaluating the initializer, just to know if we can
5625+
// evaluate the initializer.
5626+
if (VD->isLocalVarDecl() && typeShouldBeVisited(VD->getType()) &&
5627+
VD->getInit() && !VD->getInit()->isValueDependent() &&
5628+
VD->evaluateValue())
5629+
return revisit(VD);
56235630
}
56245631
} else {
56255632
if (const auto *VD = dyn_cast<VarDecl>(D);

clang/lib/AST/ByteCode/Context.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
4444
bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
4545
++EvalID;
4646
bool Recursing = !Stk.empty();
47+
size_t StackSizeBefore = Stk.size();
4748
Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
4849

4950
auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue());
5051

5152
if (Res.isInvalid()) {
5253
C.cleanup();
53-
Stk.clear();
54+
Stk.clearTo(StackSizeBefore);
5455
return false;
5556
}
5657

@@ -60,7 +61,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
6061
#ifndef NDEBUG
6162
// Make sure we don't rely on some value being still alive in
6263
// InterpStack memory.
63-
Stk.clear();
64+
Stk.clearTo(StackSizeBefore);
6465
#endif
6566
}
6667

@@ -72,12 +73,13 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
7273
bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
7374
++EvalID;
7475
bool Recursing = !Stk.empty();
76+
size_t StackSizeBefore = Stk.size();
7577
Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
7678

7779
auto Res = C.interpretExpr(E);
7880
if (Res.isInvalid()) {
7981
C.cleanup();
80-
Stk.clear();
82+
Stk.clearTo(StackSizeBefore);
8183
return false;
8284
}
8385

@@ -87,7 +89,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
8789
#ifndef NDEBUG
8890
// Make sure we don't rely on some value being still alive in
8991
// InterpStack memory.
90-
Stk.clear();
92+
Stk.clearTo(StackSizeBefore);
9193
#endif
9294
}
9395

@@ -99,6 +101,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
99101
APValue &Result) {
100102
++EvalID;
101103
bool Recursing = !Stk.empty();
104+
size_t StackSizeBefore = Stk.size();
102105
Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
103106

104107
bool CheckGlobalInitialized =
@@ -107,7 +110,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
107110
auto Res = C.interpretDecl(VD, CheckGlobalInitialized);
108111
if (Res.isInvalid()) {
109112
C.cleanup();
110-
Stk.clear();
113+
Stk.clearTo(StackSizeBefore);
114+
111115
return false;
112116
}
113117

@@ -117,7 +121,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
117121
#ifndef NDEBUG
118122
// Make sure we don't rely on some value being still alive in
119123
// InterpStack memory.
120-
Stk.clear();
124+
Stk.clearTo(StackSizeBefore);
121125
#endif
122126
}
123127

clang/lib/AST/ByteCode/InterpStack.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,16 @@ void InterpStack::clear() {
3232
#endif
3333
}
3434

35+
void InterpStack::clearTo(size_t NewSize) {
36+
assert(NewSize <= size());
37+
size_t ToShrink = size() - NewSize;
38+
if (ToShrink == 0)
39+
return;
40+
41+
shrink(ToShrink);
42+
assert(size() == NewSize);
43+
}
44+
3545
void *InterpStack::grow(size_t Size) {
3646
assert(Size < ChunkSize - sizeof(StackChunk) && "Object too large");
3747

@@ -81,6 +91,21 @@ void InterpStack::shrink(size_t Size) {
8191

8292
Chunk->End -= Size;
8393
StackSize -= Size;
94+
95+
#ifndef NDEBUG
96+
size_t TypesSize = 0;
97+
for (PrimType T : ItemTypes)
98+
TYPE_SWITCH(T, { TypesSize += aligned_size<T>(); });
99+
100+
size_t StackSize = size();
101+
while (TypesSize > StackSize) {
102+
TYPE_SWITCH(ItemTypes.back(), {
103+
TypesSize -= aligned_size<T>();
104+
ItemTypes.pop_back();
105+
});
106+
}
107+
assert(TypesSize == StackSize);
108+
#endif
84109
}
85110

86111
void InterpStack::dump() const {

clang/lib/AST/ByteCode/InterpStack.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ class InterpStack final {
8686

8787
/// Clears the stack without calling any destructors.
8888
void clear();
89+
void clearTo(size_t NewSize);
8990

9091
/// Returns whether the stack is empty.
9192
bool empty() const { return StackSize == 0; }

0 commit comments

Comments
 (0)