Revert "Move rewritten StringFactory call results into dex registers for deopt"

This reverts commit 0ea451b4d74e8ddc28ea5ce87ef6d21113a3cdd5.

597-deopt-new-string is failing.

Bug: 28846692
Bug: 28555675
Change-Id: I8274e5ae1d065dedc562132882b39e7f3fe5baf4
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 74a0ab6..1d0e600 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -484,43 +484,6 @@
   self->PopShadowFrame();
 }
 
-static bool IsStringInit(Thread* self,
-                         const Instruction* instr,
-                         ArtMethod* caller,
-                         ArtMethod* callee)
-    SHARED_REQUIRES(Locks::mutator_lock_) {
-  if (instr->Opcode() == Instruction::INVOKE_DIRECT ||
-      instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE) {
-    if (callee == nullptr) {
-      // Don't know the callee. Resolve and find out.
-      uint16_t callee_method_idx = (instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE) ?
-          instr->VRegB_3rc() : instr->VRegB_35c();
-      ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
-      // We just returned from callee, the method should resolve.
-      // If it's a String constructor, the resolved method should not be null.
-      callee = class_linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
-          self, callee_method_idx, caller, kDirect);
-    }
-    if (callee == nullptr) {
-      // In case null is returned from method resolution, just return false
-      // since a string init (just called) must not resolve to null.
-      return false;
-    }
-    if (callee->GetDeclaringClass()->IsStringClass() &&
-        callee->IsConstructor()) {
-      return true;
-    }
-  }
-  return false;
-}
-
-static int16_t GetReceiverRegisterForStringInit(const Instruction* instr) {
-  DCHECK(instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE ||
-         instr->Opcode() == Instruction::INVOKE_DIRECT);
-  return (instr->Opcode() == Instruction::INVOKE_DIRECT_RANGE) ?
-      instr->VRegC_3rc() : instr->VRegC_35c();
-}
-
 void EnterInterpreterFromDeoptimize(Thread* self,
                                     ShadowFrame* shadow_frame,
                                     bool from_code,
@@ -531,7 +494,6 @@
   value.SetJ(ret_val->GetJ());
   // Are we executing the first shadow frame?
   bool first = true;
-  ArtMethod* callee_method = nullptr;
   while (shadow_frame != nullptr) {
     // We do not want to recover lock state for lock counting when deoptimizing. Currently,
     // the compiler should not have compiled a method that failed structured-locking checks.
@@ -557,13 +519,6 @@
       // TODO: should be tested more once b/17586779 is fixed.
       const Instruction* instr = Instruction::At(&code_item->insns_[dex_pc]);
       if (instr->IsInvoke()) {
-        if (IsStringInit(self, instr, shadow_frame->GetMethod(), callee_method)) {
-          uint16_t this_obj_vreg = GetReceiverRegisterForStringInit(instr);
-          // Move the StringFactory.newStringFromChars() result into the register representing
-          // "this object" when invoking the string constructor in the original dex instruction.
-          // Also move the result into all aliases.
-          SetStringInitValueToAllAliases(shadow_frame, this_obj_vreg, value);
-        }
         new_dex_pc = dex_pc + instr->SizeInCodeUnits();
       } else if (instr->Opcode() == Instruction::NEW_INSTANCE) {
         // It's possible to deoptimize at a NEW_INSTANCE dex instruciton that's for a
@@ -574,14 +529,12 @@
               instr->VRegB_21c(), shadow_frame->GetMethod());
           DCHECK(klass->IsStringClass());
         }
-        // Move the StringFactory.newEmptyString() result into the destination register.
-        shadow_frame->SetVRegReference(instr->VRegA_21c(), value.GetL());
         // Skip the dex instruction since we essentially come back from an invocation.
         new_dex_pc = dex_pc + instr->SizeInCodeUnits();
       } else {
-        CHECK(false) << "Unexpected instruction opcode " << instr->Opcode()
-                     << " at dex_pc " << dex_pc
-                     << " of method: " << PrettyMethod(shadow_frame->GetMethod(), false);
+        DCHECK(false) << "Unexpected instruction opcode " << instr->Opcode()
+                      << " at dex_pc " << dex_pc
+                      << " of method: " << PrettyMethod(shadow_frame->GetMethod(), false);
       }
     } else {
       // Nothing to do, the dex_pc is the one at which the code requested
@@ -591,7 +544,6 @@
       shadow_frame->SetDexPC(new_dex_pc);
       value = Execute(self, code_item, *shadow_frame, value);
     }
-    callee_method = shadow_frame->GetMethod();
     ShadowFrame* old_frame = shadow_frame;
     shadow_frame = shadow_frame->GetLink();
     ShadowFrame::DeleteDeoptimizedFrame(old_frame);
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 53d5e43..12d70c5 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -540,30 +540,6 @@
                  result, method->GetInterfaceMethodIfProxy(sizeof(void*))->GetShorty());
 }
 
-void SetStringInitValueToAllAliases(ShadowFrame* shadow_frame,
-                                    uint16_t this_obj_vreg,
-                                    JValue result)
-    SHARED_REQUIRES(Locks::mutator_lock_) {
-  Object* existing = shadow_frame->GetVRegReference(this_obj_vreg);
-  if (existing == nullptr) {
-    // If it's null, we come from compiled code that was deoptimized. Nothing to do,
-    // as the compiler verified there was no alias.
-    // Set the new string result of the StringFactory.
-    shadow_frame->SetVRegReference(this_obj_vreg, result.GetL());
-    return;
-  }
-  // Set the string init result into all aliases.
-  for (uint32_t i = 0, e = shadow_frame->NumberOfVRegs(); i < e; ++i) {
-    if (shadow_frame->GetVRegReference(i) == existing) {
-      DCHECK_EQ(shadow_frame->GetVRegReference(i),
-                reinterpret_cast<mirror::Object*>(shadow_frame->GetVReg(i)));
-      shadow_frame->SetVRegReference(i, result.GetL());
-      DCHECK_EQ(shadow_frame->GetVRegReference(i),
-                reinterpret_cast<mirror::Object*>(shadow_frame->GetVReg(i)));
-    }
-  }
-}
-
 template <bool is_range,
           bool do_assignability_check,
           size_t kVarArgMax>
@@ -763,7 +739,24 @@
   }
 
   if (string_init && !self->IsExceptionPending()) {
-    SetStringInitValueToAllAliases(&shadow_frame, string_init_vreg_this, *result);
+    mirror::Object* existing = shadow_frame.GetVRegReference(string_init_vreg_this);
+    if (existing == nullptr) {
+      // If it's null, we come from compiled code that was deoptimized. Nothing to do,
+      // as the compiler verified there was no alias.
+      // Set the new string result of the StringFactory.
+      shadow_frame.SetVRegReference(string_init_vreg_this, result->GetL());
+    } else {
+      // Replace the fake string that was allocated with the StringFactory result.
+      for (uint32_t i = 0; i < shadow_frame.NumberOfVRegs(); ++i) {
+        if (shadow_frame.GetVRegReference(i) == existing) {
+          DCHECK_EQ(shadow_frame.GetVRegReference(i),
+                    reinterpret_cast<mirror::Object*>(shadow_frame.GetVReg(i)));
+          shadow_frame.SetVRegReference(i, result->GetL());
+          DCHECK_EQ(shadow_frame.GetVRegReference(i),
+                    reinterpret_cast<mirror::Object*>(shadow_frame.GetVReg(i)));
+        }
+      }
+    }
   }
 
   return !self->IsExceptionPending();
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index cc470f3..69376fd 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -1021,12 +1021,6 @@
                                         ShadowFrame* shadow_frame,
                                         JValue* result);
 
-// Set string value created from StringFactory.newStringFromXXX() into all aliases of
-// StringFactory.newEmptyString().
-void SetStringInitValueToAllAliases(ShadowFrame* shadow_frame,
-                                    uint16_t this_obj_vreg,
-                                    JValue result);
-
 // Explicitly instantiate all DoInvoke functions.
 #define EXPLICIT_DO_INVOKE_TEMPLATE_DECL(_type, _is_range, _do_check)                      \
   template SHARED_REQUIRES(Locks::mutator_lock_)                                     \
diff --git a/test/597-deopt-new-string/src/Main.java b/test/597-deopt-new-string/src/Main.java
index 556ada1..1224e40 100644
--- a/test/597-deopt-new-string/src/Main.java
+++ b/test/597-deopt-new-string/src/Main.java
@@ -48,12 +48,7 @@
             throw new Error();
         }
         char[] arr = {'a', 'b', 'c'};
-        String str = new String(arr, 0, arr.length);
-        if (!str.equals("abc")) {
-            System.out.println("Failure! " + str);
-            System.exit(0);
-        }
-        return str;
+        return new String(arr, 0, arr.length);
     }
 
     public void run() {
@@ -73,11 +68,7 @@
         } else {
             // This thread keeps doing new String() from a char array.
             while (!done) {
-                String str = $noinline$run0();
-                if (!str.equals("abc")) {
-                    System.out.println("Failure! " + str);
-                    System.exit(0);
-                }
+                $noinline$run0();
             }
         }
     }