[clang][bytecode] Add a scope to function calls

[tbaederr]

We need a place to destroy the temporaries created for call arguments.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

We need a place to destroy the temporaries created for call arguments.

---

Full diff: https://github.com/llvm/llvm-project/pull/140441.diff

3 Files Affected:

• (modified) clang/lib/AST/ByteCode/Compiler.cpp (+70-47)
• (modified) clang/lib/AST/ByteCode/Compiler.h (+33-26)
• (modified) clang/test/AST/ByteCode/lifetimes.cpp (+18)

diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 5017c9b76e6d1..aa8f009f58bb9 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -45,10 +45,6 @@ template <class Emitter> class DeclScope final : public LocalScope<Emitter> {
     Ctx->InitStack.push_back(InitLink::Decl(VD));
   }
 
-  void addExtended(const Scope::Local &Local) override {
-    return this->addLocal(Local);
-  }
-
   ~DeclScope() {
     this->Ctx->InitializingDecl = OldInitializingDecl;
     this->Ctx->InitStack.pop_back();
@@ -2021,6 +2017,45 @@ bool Compiler<Emitter>::visitArrayElemInit(unsigned ElemIndex, const Expr *Init,
   return this->emitFinishInitPop(Init);
 }
 
+template <class Emitter>
+bool Compiler<Emitter>::visitCallArgs(ArrayRef<const Expr *> Args,
+                                      const FunctionDecl *FuncDecl) {
+  assert(VarScope->getKind() == ScopeKind::Call);
+  llvm::BitVector NonNullArgs = collectNonNullArgs(FuncDecl, Args);
+
+  unsigned ArgIndex = 0;
+  for (const Expr *Arg : Args) {
+    if (std::optional<PrimType> T = classify(Arg)) {
+      if (!this->visit(Arg))
+        return false;
+    } else {
+
+      std::optional<unsigned> LocalIndex = allocateLocal(
+          Arg, Arg->getType(), /*ExtendingDecl=*/nullptr, ScopeKind::Call);
+      if (!LocalIndex)
+        return false;
+
+      if (!this->emitGetPtrLocal(*LocalIndex, Arg))
+        return false;
+      InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex));
+      if (!this->visitInitializer(Arg))
+        return false;
+    }
+
+    if (FuncDecl && NonNullArgs[ArgIndex]) {
+      PrimType ArgT = classify(Arg).value_or(PT_Ptr);
+      if (ArgT == PT_Ptr) {
+        if (!this->emitCheckNonNullArg(ArgT, Arg))
+          return false;
+      }
+    }
+
+    ++ArgIndex;
+  }
+
+  return true;
+}
+
 template <class Emitter>
 bool Compiler<Emitter>::VisitInitListExpr(const InitListExpr *E) {
   return this->visitInitList(E->inits(), E->getArrayFiller(), E);
@@ -4343,7 +4378,7 @@ bool Compiler<Emitter>::emitConst(const APSInt &Value, const Expr *E) {
 template <class Emitter>
 unsigned Compiler<Emitter>::allocateLocalPrimitive(
     DeclTy &&Src, PrimType Ty, bool IsConst, const ValueDecl *ExtendingDecl,
-    bool IsConstexprUnknown) {
+    ScopeKind SC, bool IsConstexprUnknown) {
   // Make sure we don't accidentally register the same decl twice.
   if (const auto *VD =
           dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) {
@@ -4364,14 +4399,14 @@ unsigned Compiler<Emitter>::allocateLocalPrimitive(
   if (ExtendingDecl)
     VarScope->addExtended(Local, ExtendingDecl);
   else
-    VarScope->add(Local, false);
+    VarScope->addForScopeKind(Local, SC);
   return Local.Offset;
 }
 
 template <class Emitter>
 std::optional<unsigned>
 Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty,
-                                 const ValueDecl *ExtendingDecl,
+                                 const ValueDecl *ExtendingDecl, ScopeKind SC,
                                  bool IsConstexprUnknown) {
   // Make sure we don't accidentally register the same decl twice.
   if ([[maybe_unused]] const auto *VD =
@@ -4409,7 +4444,7 @@ Compiler<Emitter>::allocateLocal(DeclTy &&Src, QualType Ty,
   if (ExtendingDecl)
     VarScope->addExtended(Local, ExtendingDecl);
   else
-    VarScope->add(Local, false);
+    VarScope->addForScopeKind(Local, SC);
   return Local.Offset;
 }
 
@@ -4676,7 +4711,7 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD,
     if (VarT) {
       unsigned Offset = this->allocateLocalPrimitive(
           VD, *VarT, VD->getType().isConstQualified(), nullptr,
-          IsConstexprUnknown);
+          ScopeKind::Block, IsConstexprUnknown);
       if (Init) {
         // If this is a toplevel declaration, create a scope for the
         // initializer.
@@ -4692,8 +4727,9 @@ VarCreationState Compiler<Emitter>::visitVarDecl(const VarDecl *VD,
         }
       }
     } else {
-      if (std::optional<unsigned> Offset = this->allocateLocal(
-              VD, VD->getType(), nullptr, IsConstexprUnknown)) {
+      if (std::optional<unsigned> Offset =
+              this->allocateLocal(VD, VD->getType(), nullptr, ScopeKind::Block,
+                                  IsConstexprUnknown)) {
         if (!Init)
           return true;
 
@@ -4881,26 +4917,28 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
   if (FuncDecl) {
     if (unsigned BuiltinID = FuncDecl->getBuiltinID())
       return VisitBuiltinCallExpr(E, BuiltinID);
-  }
 
-  // Calls to replaceable operator new/operator delete.
-  if (FuncDecl &&
-      FuncDecl->isUsableAsGlobalAllocationFunctionInConstantEvaluation()) {
-    if (FuncDecl->getDeclName().isAnyOperatorNew()) {
-      return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_new);
-    } else {
-      assert(FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_Delete);
-      return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_delete);
+    // Calls to replaceable operator new/operator delete.
+    if (FuncDecl->isUsableAsGlobalAllocationFunctionInConstantEvaluation()) {
+      if (FuncDecl->getDeclName().isAnyOperatorNew()) {
+        return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_new);
+      } else {
+        assert(FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_Delete);
+        return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_delete);
+      }
+    }
+
+    // Explicit calls to trivial destructors
+    if (const auto *DD = dyn_cast<CXXDestructorDecl>(FuncDecl);
+        DD && DD->isTrivial()) {
+      const auto *MemberCall = cast<CXXMemberCallExpr>(E);
+      if (!this->visit(MemberCall->getImplicitObjectArgument()))
+        return false;
+      return this->emitCheckDestruction(E) && this->emitPopPtr(E);
     }
   }
-  // Explicit calls to trivial destructors
-  if (const auto *DD = dyn_cast_if_present<CXXDestructorDecl>(FuncDecl);
-      DD && DD->isTrivial()) {
-    const auto *MemberCall = cast<CXXMemberCallExpr>(E);
-    if (!this->visit(MemberCall->getImplicitObjectArgument()))
-      return false;
-    return this->emitCheckDestruction(E) && this->emitPopPtr(E);
-  }
+
+  BlockScope<Emitter> CallScope(this, ScopeKind::Call);
 
   QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
   std::optional<PrimType> T = classify(ReturnType);
@@ -4996,23 +5034,8 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
       return false;
   }
 
-  llvm::BitVector NonNullArgs = collectNonNullArgs(FuncDecl, Args);
-  // Put arguments on the stack.
-  unsigned ArgIndex = 0;
-  for (const auto *Arg : Args) {
-    if (!this->visit(Arg))
-      return false;
-
-    // If we know the callee already, check the known parametrs for nullability.
-    if (FuncDecl && NonNullArgs[ArgIndex]) {
-      PrimType ArgT = classify(Arg).value_or(PT_Ptr);
-      if (ArgT == PT_Ptr) {
-        if (!this->emitCheckNonNullArg(ArgT, Arg))
-          return false;
-      }
-    }
-    ++ArgIndex;
-  }
+  if (!this->visitCallArgs(Args, FuncDecl))
+    return false;
 
   // Undo the argument reversal we did earlier.
   if (IsAssignmentOperatorCall) {
@@ -5088,9 +5111,9 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
 
   // Cleanup for discarded return values.
   if (DiscardResult && !ReturnType->isVoidType() && T)
-    return this->emitPop(*T, E);
+    return this->emitPop(*T, E) && CallScope.destroyLocals();
 
-  return true;
+  return CallScope.destroyLocals();
 }
 
 template <class Emitter>
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index 56a972f452af9..ac3ad84766dc6 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -102,6 +102,8 @@ struct VarCreationState {
   bool notCreated() const { return !S; }
 };
 
+enum class ScopeKind { Call, Block };
+
 /// Compilation context for expressions.
 template <class Emitter>
 class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
@@ -305,17 +307,19 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
                      const Expr *E);
   bool visitArrayElemInit(unsigned ElemIndex, const Expr *Init,
                           std::optional<PrimType> InitT);
+  bool visitCallArgs(ArrayRef<const Expr *> Args, const FunctionDecl *FuncDecl);
 
   /// Creates a local primitive value.
   unsigned allocateLocalPrimitive(DeclTy &&Decl, PrimType Ty, bool IsConst,
                                   const ValueDecl *ExtendingDecl = nullptr,
+                                  ScopeKind SC = ScopeKind::Block,
                                   bool IsConstexprUnknown = false);
 
   /// Allocates a space storing a local given its type.
   std::optional<unsigned>
   allocateLocal(DeclTy &&Decl, QualType Ty = QualType(),
                 const ValueDecl *ExtendingDecl = nullptr,
-                bool IsConstexprUnknown = false);
+                ScopeKind = ScopeKind::Block, bool IsConstexprUnknown = false);
   std::optional<unsigned> allocateTemporary(const Expr *E);
 
 private:
@@ -451,28 +455,16 @@ extern template class Compiler<EvalEmitter>;
 /// Scope chain managing the variable lifetimes.
 template <class Emitter> class VariableScope {
 public:
-  VariableScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
-      : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD) {
+  VariableScope(Compiler<Emitter> *Ctx, const ValueDecl *VD,
+                ScopeKind Kind = ScopeKind::Block)
+      : Ctx(Ctx), Parent(Ctx->VarScope), ValDecl(VD), Kind(Kind) {
     Ctx->VarScope = this;
   }
 
   virtual ~VariableScope() { Ctx->VarScope = this->Parent; }
 
-  void add(const Scope::Local &Local, bool IsExtended) {
-    if (IsExtended)
-      this->addExtended(Local);
-    else
-      this->addLocal(Local);
-  }
-
   virtual void addLocal(const Scope::Local &Local) {
-    if (this->Parent)
-      this->Parent->addLocal(Local);
-  }
-
-  virtual void addExtended(const Scope::Local &Local) {
-    if (this->Parent)
-      this->Parent->addExtended(Local);
+    llvm_unreachable("Shouldn't be called");
   }
 
   void addExtended(const Scope::Local &Local, const ValueDecl *ExtendingDecl) {
@@ -496,10 +488,28 @@ template <class Emitter> class VariableScope {
       this->addLocal(Local);
   }
 
+  /// Like addExtended, but adds to the nearest scope of the given kind.
+  void addForScopeKind(const Scope::Local &Local, ScopeKind Kind) {
+    VariableScope *P = this;
+    while (P) {
+      if (P->Kind == Kind) {
+        P->addLocal(Local);
+        return;
+      }
+      P = P->Parent;
+      if (!P)
+        break;
+    }
+
+    // Add to this scope.
+    this->addLocal(Local);
+  }
+
   virtual void emitDestruction() {}
   virtual bool emitDestructors(const Expr *E = nullptr) { return true; }
   virtual bool destroyLocals(const Expr *E = nullptr) { return true; }
   VariableScope *getParent() const { return Parent; }
+  ScopeKind getKind() const { return Kind; }
 
 protected:
   /// Compiler instance.
@@ -507,12 +517,14 @@ template <class Emitter> class VariableScope {
   /// Link to the parent scope.
   VariableScope *Parent;
   const ValueDecl *ValDecl = nullptr;
+  ScopeKind Kind;
 };
 
 /// Generic scope for local variables.
 template <class Emitter> class LocalScope : public VariableScope<Emitter> {
 public:
-  LocalScope(Compiler<Emitter> *Ctx) : VariableScope<Emitter>(Ctx, nullptr) {}
+  LocalScope(Compiler<Emitter> *Ctx, ScopeKind Kind = ScopeKind::Block)
+      : VariableScope<Emitter>(Ctx, nullptr, Kind) {}
   LocalScope(Compiler<Emitter> *Ctx, const ValueDecl *VD)
       : VariableScope<Emitter>(Ctx, VD) {}
 
@@ -599,16 +611,11 @@ template <class Emitter> class LocalScope : public VariableScope<Emitter> {
 };
 
 /// Scope for storage declared in a compound statement.
+// FIXME: Remove?
 template <class Emitter> class BlockScope final : public LocalScope<Emitter> {
 public:
-  BlockScope(Compiler<Emitter> *Ctx) : LocalScope<Emitter>(Ctx) {}
-
-  void addExtended(const Scope::Local &Local) override {
-    // If we to this point, just add the variable as a normal local
-    // variable. It will be destroyed at the end of the block just
-    // like all others.
-    this->addLocal(Local);
-  }
+  BlockScope(Compiler<Emitter> *Ctx, ScopeKind Kind = ScopeKind::Block)
+      : LocalScope<Emitter>(Ctx, Kind) {}
 };
 
 template <class Emitter> class ArrayIndexScope final {
diff --git a/clang/test/AST/ByteCode/lifetimes.cpp b/clang/test/AST/ByteCode/lifetimes.cpp
index 658f696cad2e8..5e021387348b5 100644
--- a/clang/test/AST/ByteCode/lifetimes.cpp
+++ b/clang/test/AST/ByteCode/lifetimes.cpp
@@ -88,3 +88,21 @@ namespace PseudoDtor {
                          // both-note {{visible outside that expression}}
   };
 }
+
+/// Diagnostic differences
+namespace CallScope {
+  struct Q {
+    int n = 0;
+    constexpr int f() const { return 0; }
+  };
+  constexpr Q *out_of_lifetime(Q q) { return &q; } // both-warning {{address of stack}}
+  constexpr int k3 = out_of_lifetime({})->n; // both-error {{must be initialized by a constant expression}} \
+                                             // expected-note {{read of temporary whose lifetime has ended}} \
+                                             // expected-note {{temporary created here}} \
+                                             // ref-note {{read of object outside its lifetime}}
+
+  constexpr int k4 = out_of_lifetime({})->f(); // both-error {{must be initialized by a constant expression}} \
+                                               // expected-note {{member call on temporary whose lifetime has ended}} \
+                                               // expected-note {{temporary created here}} \
+                                               // ref-note {{member call on object outside its lifetime}}
+}