Fix assert failure in interpreter after deoptimization.
There is code that does
new String(chararr, 0, chararr_count);
which under debuggable turns into a call into pNewEmptyString() plus a
call into pNewStringFromChars_CII().
Even though we currently don't patch return pc in a runtime method,
calling into pNewEmptyString() is special since it's hacked such
that it's acting as if the caller calls into the java method
StringFactory.newEmptyString() directly. So deoptimization can now
happen at the NewEmptyString site and the assert is triggered since
it's a new instance instead of an invoke instruction.
The fix relaxes the assert to allow the special case.
Bug: 28555675
Change-Id: Idbb159b5aa450df2344cd93ae74fef5f55bdc534
diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 81a396a..6c630cc 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -515,8 +515,24 @@
// instruction, as it already executed.
// TODO: should be tested more once b/17586779 is fixed.
const Instruction* instr = Instruction::At(&code_item->insns_[dex_pc]);
- DCHECK(instr->IsInvoke());
- new_dex_pc = dex_pc + instr->SizeInCodeUnits();
+ if (instr->IsInvoke()) {
+ 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
+ // java string, which is turned into a call into StringFactory.newEmptyString();
+ if (kIsDebugBuild) {
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ mirror::Class* klass = class_linker->ResolveType(
+ instr->VRegB_21c(), shadow_frame->GetMethod());
+ DCHECK(klass->IsStringClass());
+ }
+ // 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);
+ }
} else {
// Nothing to do, the dex_pc is the one at which the code requested
// the deoptimization.
diff --git a/test/597-deopt-new-string/deopt.cc b/test/597-deopt-new-string/deopt.cc
new file mode 100644
index 0000000..844a786
--- /dev/null
+++ b/test/597-deopt-new-string/deopt.cc
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "jni.h"
+#include "mirror/class-inl.h"
+#include "runtime.h"
+#include "thread_list.h"
+#include "thread_state.h"
+#include "gc/gc_cause.h"
+#include "gc/scoped_gc_critical_section.h"
+
+namespace art {
+
+extern "C" JNIEXPORT void JNICALL Java_Main_deoptimizeAll(
+ JNIEnv* env,
+ jclass cls ATTRIBUTE_UNUSED) {
+ ScopedObjectAccess soa(env);
+ ScopedThreadSuspension sts(Thread::Current(), kWaitingForDeoptimization);
+ gc::ScopedGCCriticalSection gcs(Thread::Current(),
+ gc::kGcCauseInstrumentation,
+ gc::kCollectorTypeInstrumentation);
+ // We need to suspend mutator threads first.
+ ScopedSuspendAll ssa(__FUNCTION__);
+ static bool first = true;
+ if (first) {
+ // We need to enable deoptimization once in order to call DeoptimizeEverything().
+ Runtime::Current()->GetInstrumentation()->EnableDeoptimization();
+ first = false;
+ }
+ Runtime::Current()->GetInstrumentation()->DeoptimizeEverything("test");
+}
+
+extern "C" JNIEXPORT void JNICALL Java_Main_undeoptimizeAll(
+ JNIEnv* env,
+ jclass cls ATTRIBUTE_UNUSED) {
+ ScopedObjectAccess soa(env);
+ ScopedThreadSuspension sts(Thread::Current(), kWaitingForDeoptimization);
+ gc::ScopedGCCriticalSection gcs(Thread::Current(),
+ gc::kGcCauseInstrumentation,
+ gc::kCollectorTypeInstrumentation);
+ // We need to suspend mutator threads first.
+ ScopedSuspendAll ssa(__FUNCTION__);
+ Runtime::Current()->GetInstrumentation()->UndeoptimizeEverything("test");
+}
+
+} // namespace art
diff --git a/test/597-deopt-new-string/expected.txt b/test/597-deopt-new-string/expected.txt
new file mode 100644
index 0000000..f993efc
--- /dev/null
+++ b/test/597-deopt-new-string/expected.txt
@@ -0,0 +1,2 @@
+JNI_OnLoad called
+Finishing
diff --git a/test/597-deopt-new-string/info.txt b/test/597-deopt-new-string/info.txt
new file mode 100644
index 0000000..1bd1f79
--- /dev/null
+++ b/test/597-deopt-new-string/info.txt
@@ -0,0 +1 @@
+Regression test for b/28555675
diff --git a/test/597-deopt-new-string/run b/test/597-deopt-new-string/run
new file mode 100644
index 0000000..9776ab3
--- /dev/null
+++ b/test/597-deopt-new-string/run
@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Copyright (C) 2016 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# We want to run in debuggable mode which keeps the call into StringFactory.newEmptyString().
+exec ${RUN} -Xcompiler-option --debuggable "${@}"
diff --git a/test/597-deopt-new-string/src/Main.java b/test/597-deopt-new-string/src/Main.java
new file mode 100644
index 0000000..1224e40
--- /dev/null
+++ b/test/597-deopt-new-string/src/Main.java
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main implements Runnable {
+ static final int numberOfThreads = 2;
+ static final int totalOperations = 40000;
+ static boolean sFlag = false;
+ static volatile boolean done = false;
+ int threadIndex;
+
+ public static native void deoptimizeAll();
+ public static native void undeoptimizeAll();
+
+ Main(int index) {
+ threadIndex = index;
+ }
+
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+
+ final Thread[] threads = new Thread[numberOfThreads];
+ for (int t = 0; t < threads.length; t++) {
+ threads[t] = new Thread(new Main(t));
+ threads[t].start();
+ }
+ for (Thread t : threads) {
+ t.join();
+ }
+ System.out.println("Finishing");
+ }
+
+ public String $noinline$run0() {
+ // Prevent inlining.
+ if (sFlag) {
+ throw new Error();
+ }
+ char[] arr = {'a', 'b', 'c'};
+ return new String(arr, 0, arr.length);
+ }
+
+ public void run() {
+ if (threadIndex == 0) {
+ // This thread keeps doing deoptimization of all threads.
+ // Hopefully that will trigger one deoptimization when returning from
+ // StringFactory.newEmptyString() in one of the other threads.
+ for (int i = 0; i < totalOperations; ++i) {
+ if (i % 50 == 0) {
+ deoptimizeAll();
+ }
+ if (i % 50 == 25) {
+ undeoptimizeAll();
+ }
+ }
+ done = true;
+ } else {
+ // This thread keeps doing new String() from a char array.
+ while (!done) {
+ $noinline$run0();
+ }
+ }
+ }
+}
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index d6f5d37..1550cc4 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -43,7 +43,8 @@
566-polymorphic-inlining/polymorphic_inline.cc \
570-checker-osr/osr.cc \
595-profile-saving/profile-saving.cc \
- 596-app-images/app_images.cc
+ 596-app-images/app_images.cc \
+ 597-deopt-new-string/deopt.cc
ART_TARGET_LIBARTTEST_$(ART_PHONY_TEST_TARGET_SUFFIX) += $(ART_TARGET_TEST_OUT)/$(TARGET_ARCH)/libarttest.so
ART_TARGET_LIBARTTEST_$(ART_PHONY_TEST_TARGET_SUFFIX) += $(ART_TARGET_TEST_OUT)/$(TARGET_ARCH)/libarttestd.so