8218201: Failures when vmIntrinsics::_getClass is not inlined

Summary: Fix BCEscapeAnalyzer to correctly handle _getClass intrinsic.
Reviewed-by: kvn, dlong, redestad, neliasso

3 files changed, 73 insertions, 25 deletions

diff --git a/src/share/vm/ci/bcEscapeAnalyzer.cpp b/src/share/vm/ci/bcEscapeAnalyzer.cpp
index 4c4db3e85..2b9e0e514 100644
--- a/src/share/vm/ci/bcEscapeAnalyzer.cpp
+++ b/src/share/vm/ci/bcEscapeAnalyzer.cpp
@@ -1170,45 +1170,43 @@ void BCEscapeAnalyzer::iterate_blocks(Arena *arena) {
   }
 }
 
-bool BCEscapeAnalyzer::do_analysis() {
+void BCEscapeAnalyzer::do_analysis() {
   Arena* arena = CURRENT_ENV->arena();
   // identify basic blocks
   _methodBlocks = _method->get_method_blocks();
 
   iterate_blocks(arena);
-  // TEMPORARY
-  return true;
 }
 
 vmIntrinsics::ID BCEscapeAnalyzer::known_intrinsic() {
   vmIntrinsics::ID iid = method()->intrinsic_id();
-
   if (iid == vmIntrinsics::_getClass ||
       iid ==  vmIntrinsics::_fillInStackTrace ||
-      iid == vmIntrinsics::_hashCode)
+      iid == vmIntrinsics::_hashCode) {
     return iid;
-  else
+  } else {
     return vmIntrinsics::_none;
+  }
 }
 
-bool BCEscapeAnalyzer::compute_escape_for_intrinsic(vmIntrinsics::ID iid) {
+void BCEscapeAnalyzer::compute_escape_for_intrinsic(vmIntrinsics::ID iid) {
   ArgumentMap arg;
   arg.clear();
   switch (iid) {
-  case vmIntrinsics::_getClass:
-    _return_local = false;
-    break;
-  case vmIntrinsics::_fillInStackTrace:
-    arg.set(0); // 'this'
-    set_returned(arg);
-    break;
-  case vmIntrinsics::_hashCode:
-    // initialized state is correct
-    break;
+    case vmIntrinsics::_getClass:
+      _return_local = false;
+      _return_allocated = false;
+      break;
+    case vmIntrinsics::_fillInStackTrace:
+      arg.set(0); // 'this'
+      set_returned(arg);
+      break;
+    case vmIntrinsics::_hashCode:
+      // initialized state is correct
+      break;
   default:
     assert(false, "unexpected intrinsic");
   }
-  return true;
 }
 
 void BCEscapeAnalyzer::initialize() {
@@ -1279,7 +1277,7 @@ void BCEscapeAnalyzer::compute_escape_info() {
   vmIntrinsics::ID iid = known_intrinsic();
 
   // check if method can be analyzed
-  if (iid ==  vmIntrinsics::_none && (method()->is_abstract() || method()->is_native() || !method()->holder()->is_initialized()
+  if (iid == vmIntrinsics::_none && (method()->is_abstract() || method()->is_native() || !method()->holder()->is_initialized()
       || _level > MaxBCEAEstimateLevel
       || method()->code_size() > MaxBCEAEstimateSize)) {
     if (BCEATraceLevel >= 1) {
@@ -1312,8 +1310,6 @@ void BCEscapeAnalyzer::compute_escape_info() {
     tty->print_cr(" (%d bytes)", method()->code_size());
   }
 
-  bool success;
-
   initialize();
 
   // Do not scan method if it has no object parameters and
@@ -1329,9 +1325,9 @@ void BCEscapeAnalyzer::compute_escape_info() {
   }
 
   if (iid != vmIntrinsics::_none)
-    success = compute_escape_for_intrinsic(iid);
+    compute_escape_for_intrinsic(iid);
   else {
-    success = do_analysis();
+    do_analysis();
   }
 
   // don't store interprocedural escape information if it introduces
diff --git a/src/share/vm/ci/bcEscapeAnalyzer.hpp b/src/share/vm/ci/bcEscapeAnalyzer.hpp
index 74a0d3106..acca5a210 100644
--- a/src/share/vm/ci/bcEscapeAnalyzer.hpp
+++ b/src/share/vm/ci/bcEscapeAnalyzer.hpp
@@ -101,8 +101,8 @@ class BCEscapeAnalyzer : public ResourceObj {
   void clear_escape_info();
   void compute_escape_info();
   vmIntrinsics::ID known_intrinsic();
-  bool compute_escape_for_intrinsic(vmIntrinsics::ID iid);
-  bool do_analysis();
+  void compute_escape_for_intrinsic(vmIntrinsics::ID iid);
+  void do_analysis();
 
   void read_escape_info();
 
diff --git a/test/compiler/escapeAnalysis/TestGetClass.java b/test/compiler/escapeAnalysis/TestGetClass.java
new file mode 100644
index 000000000..7b2b587b2
--- /dev/null
+++ b/test/compiler/escapeAnalysis/TestGetClass.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8218201
+ * @summary BCEscapeAnalyzer assigns wrong escape state to getClass return value.
+ * @run main/othervm -XX:-TieredCompilation -Xcomp -XX:+UnlockDiagnosticVMOptions -XX:DisableIntrinsic=_getClass
+ *                   -XX:CompileCommand=quiet -XX:CompileCommand=compileonly,compiler.escapeAnalysis.TestGetClass::test
+ *                   -XX:+PrintCompilation compiler.escapeAnalysis.TestGetClass
+ */
+
+package compiler.escapeAnalysis;
+
+public class TestGetClass {
+    static Object obj = new Object();
+
+    public static boolean test() {
+        if (obj.getClass() == Object.class) {
+            synchronized (obj) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public static void main(String[] args) {
+        if (!test()) {
+            throw new RuntimeException("Test failed");
+        }
+    }
+}