Merge "Fix assert failure in interpreter after deoptimization."

7 files changed, 175 insertions, 3 deletions

diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 81a396a925..6c630cc48f 100644
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -515,8 +515,24 @@ void EnterInterpreterFromDeoptimize(Thread* self,
       // 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 0000000000..844a786e1e
--- /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 0000000000..f993efcdad
--- /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 0000000000..1bd1f79c04
--- /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 0000000000..9776ab3eb4
--- /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 0000000000..1224e407b0
--- /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 d6f5d372a9..1550cc4403 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -43,7 +43,8 @@ LIBARTTEST_COMMON_SRC_FILES := \
   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