8145331: SEGV in DirectivesStack::release(DirectiveSet*)
authorneliasso
Thu, 14 Jan 2016 13:24:03 +0100
changeset 35563 d5ac28780cda
parent 35562 16a8e33e995c
child 35564 3485bf43b924
8145331: SEGV in DirectivesStack::release(DirectiveSet*) Summary: getDefaultDirective was not updated in 8144873 Reviewed-by: twisti, kvn
hotspot/src/share/vm/compiler/compilerDirectives.cpp
hotspot/src/share/vm/prims/whitebox.cpp
hotspot/test/compiler/intrinsics/IntrinsicAvailableTest.java
--- a/hotspot/src/share/vm/compiler/compilerDirectives.cpp	Thu Jan 14 09:30:31 2016 +0100
+++ b/hotspot/src/share/vm/compiler/compilerDirectives.cpp	Thu Jan 14 13:24:03 2016 +0100
@@ -164,20 +164,15 @@
 
 DirectiveSet* CompilerDirectives::get_for(AbstractCompiler *comp) {
   assert(DirectivesStack_lock->owned_by_self(), "");
-  inc_refcount(); // The compiling thread is responsible to decrement this when finished.
   if (comp == NULL) { // Xint
     return _c1_store;
-  } else if (comp->is_c2()) {
+  } else  if (comp->is_c2()) {
     return _c2_store;
-  } else if (comp->is_c1()) {
+  } else {
+    // use c1_store as default
+    assert(comp->is_c1() || comp->is_jvmci() || comp->is_shark(), "");
     return _c1_store;
-  } else if (comp->is_shark()) {
-    return NULL;
-  } else if (comp->is_jvmci()) {
-    return NULL;
   }
-  ShouldNotReachHere();
-  return NULL;
 }
 
 // In the list of disabled intrinsics, the ID of the disabled intrinsics can separated:
@@ -459,6 +454,7 @@
   MutexLockerEx locker(DirectivesStack_lock, Mutex::_no_safepoint_check_flag);
 
   assert(_bottom != NULL, "Must never be empty");
+  _bottom->inc_refcount();
   return _bottom->get_for(comp);
 }
 
@@ -521,12 +517,13 @@
 }
 
 void DirectivesStack::release(DirectiveSet* set) {
+  assert(set != NULL, "Never NULL");
   MutexLockerEx locker(DirectivesStack_lock, Mutex::_no_safepoint_check_flag);
   if (set->is_exclusive_copy()) {
     // Old CompilecCmmands forced us to create an exclusive copy
     delete set;
   } else {
-    assert(set->directive() != NULL, "");
+    assert(set->directive() != NULL, "Never NULL");
     release(set->directive());
   }
 }
@@ -553,26 +550,18 @@
     while (dir != NULL) {
       if (dir->is_default_directive() || dir->match(method)) {
         match = dir->get_for(comp);
-        if (match == NULL) {
-          // temporary workaround for compilers without directives.
-          if (dir->is_default_directive()) {
-            // default dir is always enabled
-            // match c1 store - it contains all common flags even if C1 is unavailable
-            match = dir->_c1_store;
-            break;
-          }
-        } else {
-          if (match->EnableOption) {
-            // The directiveSet for this compile is also enabled -> success
-            break;
-          }
+        assert(match != NULL, "Consistency");
+        if (match->EnableOption) {
+          // The directiveSet for this compile is also enabled -> success
+          dir->inc_refcount();
+          break;
         }
       }
       dir = dir->next();
     }
   }
+  guarantee(match != NULL, "There should always be a default directive that matches");
 
-  guarantee(match != NULL, "There should always be a default directive that matches");
   // Check for legacy compile commands update, without DirectivesStack_lock
   return match->compilecommand_compatibility_init(method);
 }
--- a/hotspot/src/share/vm/prims/whitebox.cpp	Thu Jan 14 09:30:31 2016 +0100
+++ b/hotspot/src/share/vm/prims/whitebox.cpp	Thu Jan 14 13:24:03 2016 +0100
@@ -565,14 +565,15 @@
   methodHandle mh(THREAD, Method::checked_resolve_jmethod_id(method_id));
 
   DirectiveSet* directive;
+  AbstractCompiler* comp = CompileBroker::compiler((int)compLevel);
   if (compilation_context != NULL) {
     compilation_context_id = reflected_method_to_jmid(thread, env, compilation_context);
     CHECK_JNI_EXCEPTION_(env, JNI_FALSE);
     methodHandle cch(THREAD, Method::checked_resolve_jmethod_id(compilation_context_id));
-    directive = DirectivesStack::getMatchingDirective(cch, CompileBroker::compiler((int)compLevel));
+    directive = DirectivesStack::getMatchingDirective(cch, comp);
   } else {
     // Calling with NULL matches default directive
-    directive = DirectivesStack::getDefaultDirective(CompileBroker::compiler((int)compLevel));
+    directive = DirectivesStack::getDefaultDirective(comp);
   }
   bool result = CompileBroker::compiler(compLevel)->is_intrinsic_available(mh, directive);
   DirectivesStack::release(directive);
--- a/hotspot/test/compiler/intrinsics/IntrinsicAvailableTest.java	Thu Jan 14 09:30:31 2016 +0100
+++ b/hotspot/test/compiler/intrinsics/IntrinsicAvailableTest.java	Thu Jan 14 13:24:03 2016 +0100
@@ -23,6 +23,8 @@
 import java.lang.reflect.Executable;
 import java.util.concurrent.Callable;
 import java.util.Objects;
+
+import jdk.test.lib.*;
 import compiler.whitebox.CompilerWhiteBoxTest;
 /*
  * @test
@@ -105,17 +107,16 @@
         }
     }
 
-    protected boolean isServerVM() {
-        return VMName.toLowerCase().contains("server");
-    }
-
     public void test() throws Exception {
         Executable intrinsicMethod = testCase.getExecutable();
-        if (isServerVM()) {
+        if (Platform.isServer()) {
             if (TIERED_COMPILATION) {
                 checkIntrinsicForCompilationLevel(intrinsicMethod, COMP_LEVEL_SIMPLE);
             }
-            checkIntrinsicForCompilationLevel(intrinsicMethod, COMP_LEVEL_FULL_OPTIMIZATION);
+            // Dont bother check JVMCI compiler - returns false on all intrinsics.
+            if (!Boolean.valueOf(getVMOption("UseJVMCICompiler"))) {
+                checkIntrinsicForCompilationLevel(intrinsicMethod, COMP_LEVEL_FULL_OPTIMIZATION);
+            }
         } else {
             checkIntrinsicForCompilationLevel(intrinsicMethod, COMP_LEVEL_SIMPLE);
         }