[clang][Interp] Explicitly handle RVO Pointer

The calling convention is:

[RVO pointer]
[instance pointer]
[... args ...]

We handle the instance pointer ourselves, BUT for the RVO pointer, we
just assumed in visitReturnStmt() that it is on top of the stack. Which
isn't true if there are other args present (and a this pointer, maybe).

Fix this by recording the RVO pointer explicitly when creating an
InterpFrame, just like we do with the instance/This pointer.

There is already a "RVOAndParams()" test in test/AST/Inter/records.cpp,
that was supposed to test this, however, it didn't trigger any
problematic behavior because the parameter and the return value have the
same type.

Differential Revision: https://reviews.llvm.org/D137392

diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 19cdc2a..69b2232 100644 (file)
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -233,8 +233,13 @@ bool ByteCodeStmtGen<Emitter>::visitReturnStmt(const ReturnStmt *RS) {
       return this->emitRet(*ReturnType, RS);
     } else {
       // RVO - construct the value in the return location.
+      if (!this->emitRVOPtr(RE))
+        return false;
       if (!this->visitInitializer(RE))
         return false;
+      if (!this->emitPopPtr(RE))
+        return false;
+
       this->emitCleanup();
       return this->emitRetVoid(RS);
     }

diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 5a6c3f1..e252c54 100644 (file)
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1148,6 +1148,12 @@ inline bool This(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+inline bool RVOPtr(InterpState &S, CodePtr OpPC) {
+  assert(S.Current->getFunction()->hasRVO());
+  S.Stk.push<Pointer>(S.Current->getRVOPtr());
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Shr, Shl
 //===----------------------------------------------------------------------===//

diff --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp
index 781b6d8..9bdb712 100644 (file)
--- a/clang/lib/AST/Interp/InterpFrame.cpp
+++ b/clang/lib/AST/Interp/InterpFrame.cpp
@@ -49,6 +49,9 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC)
   // If the fuction has a This pointer, that one is next.
   // Then follow the actual arguments (but those are handled
   // in getParamPointer()).
+  if (Func->hasRVO())
+    RVOPtr = stackRef<Pointer>(0);
+
   if (Func->hasThisPointer()) {
     if (Func->hasRVO())
       This = stackRef<Pointer>(sizeof(Pointer));

diff --git a/clang/lib/AST/Interp/InterpFrame.h b/clang/lib/AST/Interp/InterpFrame.h
index ecbe697..bcb45a1 100644 (file)
--- a/clang/lib/AST/Interp/InterpFrame.h
+++ b/clang/lib/AST/Interp/InterpFrame.h
@@ -37,7 +37,8 @@ public:
 
   /// Creates a new frame with the values that make sense.
   /// I.e., the caller is the current frame of S,
-  /// and the This() pointer is the current Pointer on the top of S's stack,
+  /// the This() pointer is the current Pointer on the top of S's stack,
+  /// and the RVO pointer is before that.
   InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC);
 
   /// Destroys the frame, killing all live pointers to stack slots.
@@ -101,6 +102,9 @@ public:
   /// Returns the 'this' pointer.
   const Pointer &getThis() const { return This; }
 
+  /// Returns the RVO pointer, if the Function has one.
+  const Pointer &getRVOPtr() const { return RVOPtr; }
+
   /// Checks if the frame is a root frame - return should quit the interpreter.
   bool isRoot() const { return !Func; }
 
@@ -139,6 +143,8 @@ private:
   const Function *Func;
   /// Current object pointer for methods.
   Pointer This;
+  /// Pointer the non-primitive return value gets constructed in.
+  Pointer RVOPtr;
   /// Return address.
   CodePtr RetPC;
   /// The size of all the arguments.

diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index ebb0f49..058475b 100644 (file)
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -269,6 +269,9 @@ def GetPtrThisVirtBase : Opcode {
 // [] -> [Pointer]
 def This : Opcode;
 
+// [] -> [Pointer]
+def RVOPtr : Opcode;
+
 // [Pointer] -> [Pointer]
 def NarrowPtr : Opcode;
 // [Pointer] -> [Pointer]

diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 963a3f1..d556ad0 100644 (file)
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -131,6 +131,12 @@ constexpr C RVOAndParams(const C *c) {
   return C();
 }
 constexpr C RVOAndParamsResult = RVOAndParams(&c);
+
+/// Parameter and return value have different types.
+constexpr C RVOAndParams(int a) {
+  return C();
+}
+constexpr C RVOAndParamsResult2 = RVOAndParams(12);
 #endif
 
 class Bar { // expected-note {{definition of 'Bar' is not complete}} \