Move rewritten StringFactory call results into dex registers for deopt
Bug: 28555675
Change-Id: I9236df283f2e83ca5dcde01f73dc0522d745cd59
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 1d0e600..74a0ab6 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -484,6 +484,43 @@
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,
@@ -494,6 +531,7 @@
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.
@@ -519,6 +557,13 @@
// 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
@@ -529,12 +574,14 @@
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 {
- DCHECK(false) << "Unexpected instruction opcode " << instr->Opcode()
- << " at dex_pc " << dex_pc
- << " of method: " << PrettyMethod(shadow_frame->GetMethod(), false);
+ CHECK(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
@@ -544,6 +591,7 @@
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 12d70c5..53d5e43 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -540,6 +540,30 @@
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>
@@ -739,24 +763,7 @@
}
if (string_init && !self->IsExceptionPending()) {
- 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)));
- }
- }
- }
+ SetStringInitValueToAllAliases(&shadow_frame, string_init_vreg_this, *result);
}
return !self->IsExceptionPending();
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index 69376fd..cc470f3 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -1021,6 +1021,12 @@
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 1224e40..556ada1 100644
--- a/test/597-deopt-new-string/src/Main.java
+++ b/test/597-deopt-new-string/src/Main.java
@@ -48,7 +48,12 @@
throw new Error();
}
char[] arr = {'a', 'b', 'c'};
- return new String(arr, 0, arr.length);
+ String str = new String(arr, 0, arr.length);
+ if (!str.equals("abc")) {
+ System.out.println("Failure! " + str);
+ System.exit(0);
+ }
+ return str;
}
public void run() {
@@ -68,7 +73,11 @@
} else {
// This thread keeps doing new String() from a char array.
while (!done) {
- $noinline$run0();
+ String str = $noinline$run0();
+ if (!str.equals("abc")) {
+ System.out.println("Failure! " + str);
+ System.exit(0);
+ }
}
}
}
