[clang][bytecode] Don't create function frames for builtin calls

clang/lib/AST/ByteCode/Compiler.cpp

@@ -4825,7 +4825,7 @@ bool Compiler<Emitter>::VisitBuiltinCallExpr(const CallExpr *E,
       return false;
   }
 
-  if (!Func->isUnevaluatedBuiltin()) {
+  if (!Context::isUnevaluatedBuiltin(BuiltinID)) {
     // Put arguments on the stack.
     for (const auto *Arg : E->arguments()) {
       if (!this->visit(Arg))

clang/lib/AST/ByteCode/Context.cpp

@@ -450,3 +450,9 @@ unsigned Context::collectBaseOffset(const RecordDecl *BaseDecl,
 const Record *Context::getRecord(const RecordDecl *D) const {
   return P->getOrCreateRecord(D);
 }
+
+bool Context::isUnevaluatedBuiltin(unsigned ID) {
+  return ID == Builtin::BI__builtin_classify_type ||
+         ID == Builtin::BI__builtin_os_log_format_buffer_size ||
+         ID == Builtin::BI__builtin_constant_p || ID == Builtin::BI__noop;
+}

clang/lib/AST/ByteCode/Context.h

@@ -111,6 +111,13 @@ class Context final {
 
   unsigned getEvalID() const { return EvalID; }
 
+  /// Unevaluated builtins don't get their arguments put on the stack
+  /// automatically. They instead operate on the AST of their Call
+  /// Expression.
+  /// Similar information is available via ASTContext::BuiltinInfo,
+  /// but that is not correct for our use cases.
+  static bool isUnevaluatedBuiltin(unsigned ID);
+
 private:
   /// Runs a function.
   bool Run(State &Parent, const Function *Func);

clang/lib/AST/ByteCode/Function.cpp

@@ -62,19 +62,3 @@ SourceInfo Function::getSource(CodePtr PC) const {
     return SrcMap.back().second;
   return It->second;
 }
-
-/// Unevaluated builtins don't get their arguments put on the stack
-/// automatically. They instead operate on the AST of their Call
-/// Expression.
-/// Similar information is available via ASTContext::BuiltinInfo,
-/// but that is not correct for our use cases.
-static bool isUnevaluatedBuiltin(unsigned BuiltinID) {
-  return BuiltinID == Builtin::BI__builtin_classify_type ||
-         BuiltinID == Builtin::BI__builtin_os_log_format_buffer_size ||
-         BuiltinID == Builtin::BI__builtin_constant_p ||
-         BuiltinID == Builtin::BI__noop;
-}
-
-bool Function::isUnevaluatedBuiltin() const {
-  return ::isUnevaluatedBuiltin(BuiltinID);
-}

clang/lib/AST/ByteCode/Function.h

@@ -202,8 +202,6 @@ class Function final {
 
   bool isBuiltin() const { return getBuiltinID() != 0; }
 
-  bool isUnevaluatedBuiltin() const;
-
   unsigned getNumParams() const { return ParamTypes.size(); }
 
   /// Returns the number of parameter this function takes when it's called,

clang/lib/AST/ByteCode/Interp.cpp

@@ -251,22 +251,6 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC,
   assert(S.Current);
   assert(Func);
 
-  if (Func->isUnevaluatedBuiltin())
-    return;
-
-  // Some builtin functions require us to only look at the call site, since
-  // the classified parameter types do not match.
-  if (unsigned BID = Func->getBuiltinID();
-      BID && S.getASTContext().BuiltinInfo.hasCustomTypechecking(BID)) {
-    const auto *CE =
-        cast<CallExpr>(S.Current->Caller->getExpr(S.Current->getRetPC()));
-    for (int32_t I = CE->getNumArgs() - 1; I >= 0; --I) {
-      const Expr *A = CE->getArg(I);
-      popArg(S, A);
-    }
-    return;
-  }
-
   if (S.Current->Caller && Func->isVariadic()) {
     // CallExpr we're look for is at the return PC of the current function, i.e.
     // in the caller.
@@ -1630,23 +1614,8 @@ bool CallBI(InterpState &S, CodePtr OpPC, const Function *Func,
   if (BuiltinID == Builtin::BI__builtin_operator_new &&
       S.checkingPotentialConstantExpression())
     return false;
-  auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC);
-
-  InterpFrame *FrameBefore = S.Current;
-  S.Current = NewFrame.get();
-
-  if (InterpretBuiltin(S, OpPC, Func, CE, BuiltinID)) {
-    // Release ownership of NewFrame to prevent it from being deleted.
-    NewFrame.release(); // Frame was deleted already.
-    // Ensure that S.Current is correctly reset to the previous frame.
-    assert(S.Current == FrameBefore);
-    return true;
-  }
 
-  // Interpreting the function failed somehow. Reset to
-  // previous state.
-  S.Current = FrameBefore;
-  return false;
+  return InterpretBuiltin(S, OpPC, Func, CE, BuiltinID);
 }
 
 bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize,

clang/lib/AST/ByteCode/InterpBuiltin.cpp

@@ -81,14 +81,41 @@ static void assignInteger(Pointer &Dest, PrimType ValueT, const APSInt &Value) {
       ValueT, { Dest.deref<T>() = T::from(static_cast<T>(Value)); });
 }
 
+template <PrimType Name, class V = typename PrimConv<Name>::T>
+static bool retBI(InterpState &S, const CallExpr *Call, unsigned BuiltinID) {
+  // The return value of the function is already on the stack.
+  // Remove it, get rid of all the arguments and add it back.
+  const V &Val = S.Stk.pop<V>();
+  if (!Context::isUnevaluatedBuiltin(BuiltinID)) {
+    for (int32_t I = Call->getNumArgs() - 1; I >= 0; --I) {
+      const Expr *A = Call->getArg(I);
+      PrimType Ty = S.getContext().classify(A).value_or(PT_Ptr);
+      TYPE_SWITCH(Ty, S.Stk.discard<T>());
+    }
+  }
+  S.Stk.push<V>(Val);
+  return true;
+}
+
 static bool retPrimValue(InterpState &S, CodePtr OpPC,
-                         std::optional<PrimType> &T) {
-  if (!T)
-    return RetVoid(S, OpPC);
+                         std::optional<PrimType> &T, const CallExpr *Call,
+                         unsigned BuiltinID) {
+
+  if (!T) {
+    if (!Context::isUnevaluatedBuiltin(BuiltinID)) {
+      for (int32_t I = Call->getNumArgs() - 1; I >= 0; --I) {
+        const Expr *A = Call->getArg(I);
+        PrimType Ty = S.getContext().classify(A).value_or(PT_Ptr);
+        TYPE_SWITCH(Ty, S.Stk.discard<T>());
+      }
+    }
+
+    return true;
+  }
 
 #define RET_CASE(X)                                                            \
   case X:                                                                      \
-    return Ret<X>(S, OpPC);
+    return retBI<X>(S, Call, BuiltinID);
   switch (*T) {
     RET_CASE(PT_Ptr);
     RET_CASE(PT_Float);
@@ -147,17 +174,16 @@ static bool interp__builtin_is_constant_evaluated(InterpState &S, CodePtr OpPC,
   // The one above that, potentially the one for std::is_constant_evaluated().
   if (S.inConstantContext() && !S.checkingPotentialConstantExpression() &&
       S.getEvalStatus().Diag &&
-      (Depth == 1 || (Depth == 2 && isStdCall(Caller->getCallee())))) {
-    if (Caller->Caller && isStdCall(Caller->getCallee())) {
-      const Expr *E = Caller->Caller->getExpr(Caller->getRetPC());
+      (Depth == 0 || (Depth == 1 && isStdCall(Frame->getCallee())))) {
+    if (Caller && isStdCall(Frame->getCallee())) {
+      const Expr *E = Caller->getExpr(Caller->getRetPC());
       S.report(E->getExprLoc(),
                diag::warn_is_constant_evaluated_always_true_constexpr)
           << "std::is_constant_evaluated" << E->getSourceRange();
     } else {
-      const Expr *E = Frame->Caller->getExpr(Frame->getRetPC());
-      S.report(E->getExprLoc(),
+      S.report(Call->getExprLoc(),
                diag::warn_is_constant_evaluated_always_true_constexpr)
-          << "__builtin_is_constant_evaluated" << E->getSourceRange();
+          << "__builtin_is_constant_evaluated" << Call->getSourceRange();
     }
   }
 
@@ -2675,7 +2701,7 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
     return false;
   }
 
-  return retPrimValue(S, OpPC, ReturnT);
+  return retPrimValue(S, OpPC, ReturnT, Call, BuiltinID);
 }
 
 bool InterpretOffsetOf(InterpState &S, CodePtr OpPC, const OffsetOfExpr *E,