OSDN Git Service

Fix assert failure in interpreter after deoptimization.
authorMingyao Yang <mingyao@google.com>
Thu, 28 Apr 2016 23:23:01 +0000 (16:23 -0700)
committerMingyao Yang <mingyao@google.com>
Wed, 11 May 2016 17:55:11 +0000 (10:55 -0700)
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

(cherry picked from commit 504a69081f63818ca332ddaf54e8198448554538)

Change-Id: Idbb159b5aa450df2344cd93ae74fef5f55bdc534

runtime/interpreter/interpreter.cc
test/597-deopt-new-string/deopt.cc [new file with mode: 0644]
test/597-deopt-new-string/expected.txt [new file with mode: 0644]
test/597-deopt-new-string/info.txt [new file with mode: 0644]
test/597-deopt-new-string/run [new file with mode: 0644]
test/597-deopt-new-string/src/Main.java [new file with mode: 0644]
test/Android.libarttest.mk

index 81a396a..6c630cc 100644 (file)
@@ -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 (file)
index 0000000..844a786
--- /dev/null
@@ -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 (file)
index 0000000..f993efc
--- /dev/null
@@ -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 (file)
index 0000000..1bd1f79
--- /dev/null
@@ -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 (file)
index 0000000..9776ab3
--- /dev/null
@@ -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 (file)
index 0000000..1224e40
--- /dev/null
@@ -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();
+            }
+        }
+    }
+}
index 464da2e..0d9f282 100644 (file)
@@ -42,7 +42,8 @@ LIBARTTEST_COMMON_SRC_FILES := \
   543-env-long-ref/env_long_ref.cc \
   566-polymorphic-inlining/polymorphic_inline.cc \
   570-checker-osr/osr.cc \
-  595-profile-saving/profile-saving.cc
+  595-profile-saving/profile-saving.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