[clang][Interp] Check instance pointers before calling functions on them

Remove the double Call() implementation to reduce code duplication. Then fix Function::getSource() so we can diagnose instance pointers being null. Differential Revision: https://reviews.llvm.org/D135513
llvm · Oct 22, 2022 · cc79ddb · cc79ddb
1 parent 8013ab4
commit cc79ddb
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 27 deletions.
diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -102,14 +102,6 @@ template <PrimType OpType> bool EvalEmitter::emitRet(const SourceInfo &Info) {
   return ReturnValue<T>(S.Stk.pop<T>(), Result);
 }
 
-template <PrimType OpType>
-bool EvalEmitter::emitCall(const Function *Func, const SourceInfo &Info) {
-
-  S.Current = new InterpFrame(S, Func, {});
-  // Result of call will be on the stack and needs to be handled by the caller.
-  return Interpret(S, Result);
-}
-
 bool EvalEmitter::emitCallVoid(const Function *Func, const SourceInfo &Info) {
   APValue VoidResult;
   InterpFrame *before = S.Current;

diff --git a/clang/lib/AST/Interp/Function.cpp b/clang/lib/AST/Interp/Function.cpp
@@ -30,11 +30,12 @@ Function::ParamDescriptor Function::getParamDescriptor(unsigned Offset) const {
 }
 
 SourceInfo Function::getSource(CodePtr PC) const {
+  assert(PC >= getCodeBegin() && "PC does not belong to this function");
+  assert(PC <= getCodeEnd() && "PC Does not belong to this function");
   unsigned Offset = PC - getCodeBegin();
   using Elem = std::pair<unsigned, SourceInfo>;
   auto It = llvm::lower_bound(SrcMap, Elem{Offset, {}}, llvm::less_first());
-  if (It == SrcMap.end() || It->first != Offset)
-    llvm::report_fatal_error("missing source location");
+  assert(It != SrcMap.end());
   return It->second;
 }
 

diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
@@ -53,16 +53,6 @@ static bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
   return true;
 }
 
-template <PrimType Name, class T = typename PrimConv<Name>::T>
-static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
-  S.Current = new InterpFrame(S, const_cast<Function *>(Func), PC);
-  APValue CallResult;
-  // Note that we cannot assert(CallResult.hasValue()) here since
-  // Ret() above only sets the APValue if the curent frame doesn't
-  // have a caller set.
-  return Interpret(S, CallResult);
-}
-
 static bool CallVoid(InterpState &S, CodePtr &PC, const Function *Func) {
   APValue VoidResult;
   S.Current = new InterpFrame(S, const_cast<Function *>(Func), PC);

diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
@@ -114,6 +114,9 @@ bool CheckDivRem(InterpState &S, CodePtr OpPC, const T &LHS, const T &RHS) {
 
 template <typename T> inline bool IsTrue(const T &V) { return !V.isZero(); }
 
+/// Interpreter entry point.
+bool Interpret(InterpState &S, APValue &Result);
+
 //===----------------------------------------------------------------------===//
 // Add, Sub, Mul
 //===----------------------------------------------------------------------===//
@@ -1103,6 +1106,35 @@ inline bool ExpandPtr(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
+  auto NewFrame = std::make_unique<InterpFrame>(S, Func, PC);
+  if (Func->hasThisPointer()) {
+    if (!CheckInvoke(S, PC, NewFrame->getThis())) {
+      return false;
+    }
+    // TODO: CheckCallable
+  }
+
+  InterpFrame *FrameBefore = S.Current;
+  S.Current = NewFrame.get();
+
+  APValue CallResult;
+  // Note that we cannot assert(CallResult.hasValue()) here since
+  // Ret() above only sets the APValue if the curent frame doesn't
+  // have a caller set.
+  if (Interpret(S, CallResult)) {
+    NewFrame.release(); // Frame was delete'd already.
+    assert(S.Current == FrameBefore);
+    return true;
+  }
+
+  // Interpreting the function failed somehow. Reset to
+  // previous state.
+  S.Current = FrameBefore;
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 // Read opcode arguments
 //===----------------------------------------------------------------------===//
@@ -1116,9 +1148,6 @@ template <typename T> inline T ReadArg(InterpState &S, CodePtr &OpPC) {
   }
 }
 
-/// Interpreter entry point.
-bool Interpret(InterpState &S, APValue &Result);
-
 } // namespace interp
 } // namespace clang
 

diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
@@ -162,7 +162,6 @@ def Call : Opcode {
   let Args = [ArgFunction];
   let Types = [AllTypeClass];
   let ChangesPC = 1;
-  let HasCustomEval = 1;
   let HasGroup = 1;
 }
 

diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
@@ -156,11 +156,14 @@ namespace thisPointer {
 
   constexpr int foo() { // ref-error {{never produces a constant expression}}
     S *s = nullptr;
-    return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+    return s->get12(); // ref-note 2{{member call on dereferenced null pointer}} \
+                       // expected-note {{member call on dereferenced null pointer}}
+
   }
-  // FIXME: The new interpreter doesn't reject this currently.
   static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
-                                  // ref-note {{in call to 'foo()'}}
+                                  // ref-note {{in call to 'foo()'}} \
+                                  // expected-error {{not an integral constant expression}} \
+                                  // expected-note {{in call to 'foo()'}}
 };
 
 struct FourBoolPairs {