[clang] eef8116 - [clang][bytecode] Only visit local variables if they have constant init (#107576)

[via cfe-commits]

Author: Timm Baeder
Date: 2024-09-07T06:36:21+02:00
New Revision: eef8116be169de2a7a55b2a9b003efd2c7ee0be0

URL: https://github.com/llvm/llvm-project/commit/eef8116be169de2a7a55b2a9b003efd2c7ee0be0
DIFF: https://github.com/llvm/llvm-project/commit/eef8116be169de2a7a55b2a9b003efd2c7ee0be0.diff

LOG: [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 https://github.com/llvm/llvm-project/issues/101801

Added: 
    

Modified: 
    clang/lib/AST/ByteCode/Compiler.cpp
    clang/lib/AST/ByteCode/Context.cpp
    clang/lib/AST/ByteCode/InterpStack.cpp
    clang/lib/AST/ByteCode/InterpStack.h

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index e96bafd89c7eca..7d812301b6d36d 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -5615,11 +5615,18 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
         if (isa<DecompositionDecl>(VD))
           return revisit(VD);
 
-        // Visit local const variables like normal.
-        if ((VD->hasGlobalStorage() || VD->isLocalVarDecl() ||
-             VD->isStaticDataMember()) &&
+        if ((VD->hasGlobalStorage() || VD->isStaticDataMember()) &&
             typeShouldBeVisited(VD->getType()))
           return revisit(VD);
+
+        // FIXME: The evaluateValue() check here is a little ridiculous, since
+        // it will ultimately call into Context::evaluateAsInitializer(). In
+        // other words, we're evaluating the initializer, just to know if we can
+        // evaluate the initializer.
+        if (VD->isLocalVarDecl() && typeShouldBeVisited(VD->getType()) &&
+            VD->getInit() && !VD->getInit()->isValueDependent() &&
+            VD->evaluateValue())
+          return revisit(VD);
       }
     } else {
       if (const auto *VD = dyn_cast<VarDecl>(D);

diff  --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index e682d87b703ad7..594c74cd3796d6 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -44,13 +44,14 @@ bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
 bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
   ++EvalID;
   bool Recursing = !Stk.empty();
+  size_t StackSizeBefore = Stk.size();
   Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
 
   auto Res = C.interpretExpr(E, /*ConvertResultToRValue=*/E->isGLValue());
 
   if (Res.isInvalid()) {
     C.cleanup();
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
     return false;
   }
 
@@ -60,7 +61,7 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
 #endif
   }
 
@@ -72,12 +73,13 @@ bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
 bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
   ++EvalID;
   bool Recursing = !Stk.empty();
+  size_t StackSizeBefore = Stk.size();
   Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
 
   auto Res = C.interpretExpr(E);
   if (Res.isInvalid()) {
     C.cleanup();
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
     return false;
   }
 
@@ -87,7 +89,7 @@ bool Context::evaluate(State &Parent, const Expr *E, APValue &Result) {
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
 #endif
   }
 
@@ -99,6 +101,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
                                     APValue &Result) {
   ++EvalID;
   bool Recursing = !Stk.empty();
+  size_t StackSizeBefore = Stk.size();
   Compiler<EvalEmitter> C(*this, *P, Parent, Stk);
 
   bool CheckGlobalInitialized =
@@ -107,7 +110,8 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
   auto Res = C.interpretDecl(VD, CheckGlobalInitialized);
   if (Res.isInvalid()) {
     C.cleanup();
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
+
     return false;
   }
 
@@ -117,7 +121,7 @@ bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
 #ifndef NDEBUG
     // Make sure we don't rely on some value being still alive in
     // InterpStack memory.
-    Stk.clear();
+    Stk.clearTo(StackSizeBefore);
 #endif
   }
 

diff  --git a/clang/lib/AST/ByteCode/InterpStack.cpp b/clang/lib/AST/ByteCode/InterpStack.cpp
index b8cdaeee72166c..ae3721e983741d 100644
--- a/clang/lib/AST/ByteCode/InterpStack.cpp
+++ b/clang/lib/AST/ByteCode/InterpStack.cpp
@@ -32,6 +32,16 @@ void InterpStack::clear() {
 #endif
 }
 
+void InterpStack::clearTo(size_t NewSize) {
+  assert(NewSize <= size());
+  size_t ToShrink = size() - NewSize;
+  if (ToShrink == 0)
+    return;
+
+  shrink(ToShrink);
+  assert(size() == NewSize);
+}
+
 void *InterpStack::grow(size_t Size) {
   assert(Size < ChunkSize - sizeof(StackChunk) && "Object too large");
 
@@ -81,6 +91,21 @@ void InterpStack::shrink(size_t Size) {
 
   Chunk->End -= Size;
   StackSize -= Size;
+
+#ifndef NDEBUG
+  size_t TypesSize = 0;
+  for (PrimType T : ItemTypes)
+    TYPE_SWITCH(T, { TypesSize += aligned_size<T>(); });
+
+  size_t StackSize = size();
+  while (TypesSize > StackSize) {
+    TYPE_SWITCH(ItemTypes.back(), {
+      TypesSize -= aligned_size<T>();
+      ItemTypes.pop_back();
+    });
+  }
+  assert(TypesSize == StackSize);
+#endif
 }
 
 void InterpStack::dump() const {

diff  --git a/clang/lib/AST/ByteCode/InterpStack.h b/clang/lib/AST/ByteCode/InterpStack.h
index 153d17f0d70e1b..43988bb680d1c6 100644
--- a/clang/lib/AST/ByteCode/InterpStack.h
+++ b/clang/lib/AST/ByteCode/InterpStack.h
@@ -86,6 +86,7 @@ class InterpStack final {
 
   /// Clears the stack without calling any destructors.
   void clear();
+  void clearTo(size_t NewSize);
 
   /// Returns whether the stack is empty.
   bool empty() const { return StackSize == 0; }