Mercurial > hg > openjdk > jdk9 > hotspot

changeset 9795:372fcb921e99

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
8144246: adding lots of directives via jcmd may produce OOM crash Summary: Add a limit to the number of directives Reviewed-by: kvn
author neliasso
date Wed, 16 Dec 2015 15:38:28 +0100
parents 4650b087ab55
children 5225495dfa66
files src/share/vm/compiler/compileBroker.cpp src/share/vm/compiler/compilerDirectives.cpp src/share/vm/compiler/compilerDirectives.hpp src/share/vm/compiler/directivesParser.cpp src/share/vm/compiler/directivesParser.hpp src/share/vm/runtime/globals.hpp src/share/vm/utilities/json.cpp test/compiler/compilercontrol/parser/DirectiveStressTest.java test/compiler/compilercontrol/parser/HugeDirectiveUtil.java
diffstat 9 files changed, 46 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/compiler/compileBroker.cpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/compiler/compileBroker.cpp	Wed Dec 16 15:38:28 2015 +0100
@@ -215,7 +215,7 @@
 
   if (DirectivesParser::has_file()) {
     return DirectivesParser::parse_from_flag();
-  } else if (PrintCompilerDirectives) {
+  } else if (CompilerDirectivesPrint) {
     // Print default directive even when no other was added
     DirectivesStack::print(tty);
   }
--- a/src/share/vm/compiler/compilerDirectives.cpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/compiler/compilerDirectives.cpp	Wed Dec 16 15:38:28 2015 +0100
@@ -487,6 +487,14 @@
   DirectivesStack::release(tmp);
 }
 
+bool DirectivesStack::check_capacity(int request_size, outputStream* st) {
+  if ((request_size + _depth) > CompilerDirectivesLimit) {
+    st->print_cr("Could not add %i more directives. Currently %i/%i directives.", request_size, _depth, CompilerDirectivesLimit);
+    return false;
+  }
+  return true;
+}
+
 void DirectivesStack::clear() {
   // holding the lock during the whole operation ensuring consistent result
   MutexLockerEx locker(DirectivesStack_lock, Mutex::_no_safepoint_check_flag);
--- a/src/share/vm/compiler/compilerDirectives.hpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/compiler/compilerDirectives.hpp	Wed Dec 16 15:38:28 2015 +0100
@@ -89,6 +89,7 @@
   static DirectiveSet* getDefaultDirective(AbstractCompiler* comp);
   static void push(CompilerDirectives* directive);
   static void pop();
+  static bool check_capacity(int request_size, outputStream* st);
   static void clear();
   static void print(outputStream* st);
   static void release(DirectiveSet* set);
--- a/src/share/vm/compiler/directivesParser.cpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/compiler/directivesParser.cpp	Wed Dec 16 15:38:28 2015 +0100
@@ -30,6 +30,7 @@
 #include <string.h>
 
 void DirectivesParser::push_tmp(CompilerDirectives* dir) {
+  _tmp_depth++;
   dir->set_next(_tmp_top);
   _tmp_top = dir;
 }
@@ -41,17 +42,29 @@
   CompilerDirectives* tmp = _tmp_top;
   _tmp_top = _tmp_top->next();
   tmp->set_next(NULL);
+  _tmp_depth--;
   return tmp;
 }
 
+void DirectivesParser::clean_tmp() {
+  CompilerDirectives* tmp = pop_tmp();
+  while (tmp != NULL) {
+    delete tmp;
+    tmp = pop_tmp();
+  }
+  assert(_tmp_depth == 0, "Consistency");
+}
+
 bool DirectivesParser::parse_string(const char* text, outputStream* st) {
   DirectivesParser cd(text, st);
   if (cd.valid()) {
     return cd.install_directives();
+  } else {
+    cd.clean_tmp();
+    st->flush();
+    st->print_cr("Parsing of compiler directives failed");
+    return false;
   }
-  st->flush();
-  st->print_cr("Parsing of compiler directives failed");
-  return false;
 }
 
 bool DirectivesParser::has_file() {
@@ -91,6 +104,12 @@
 }
 
 bool DirectivesParser::install_directives() {
+  // Check limit
+  if (!DirectivesStack::check_capacity(_tmp_depth, _st)) {
+    clean_tmp();
+    return false;
+  }
+
   // Pop from internal temporary stack and push to compileBroker.
   CompilerDirectives* tmp = pop_tmp();
   int i = 0;
@@ -104,7 +123,7 @@
     return false;
   } else {
     _st->print_cr("%i compiler directives added", i);
-    if (PrintCompilerDirectives) {
+    if (CompilerDirectivesPrint) {
       // Print entire directives stack after new has been pushed.
       DirectivesStack::print(_st);
     }
@@ -113,7 +132,7 @@
 }
 
 DirectivesParser::DirectivesParser(const char* text, outputStream* st)
-: JSON(text, false, st), depth(0), current_directive(NULL), current_directiveset(NULL), _tmp_top(NULL) {
+: JSON(text, false, st), depth(0), current_directive(NULL), current_directiveset(NULL), _tmp_top(NULL), _tmp_depth(0) {
 #ifndef PRODUCT
   memset(stack, 0, MAX_DEPTH * sizeof(stack[0]));
 #endif
@@ -121,6 +140,8 @@
 }
 
 DirectivesParser::~DirectivesParser() {
+  assert(_tmp_top == NULL, "Consistency");
+  assert(_tmp_depth == 0, "Consistency");
 }
 
 const DirectivesParser::key DirectivesParser::keys[] = {
@@ -584,6 +605,7 @@
       tty->print("-- DirectivesParser test failed as expected --\n");
     }
   }
+  cd.clean_tmp();
 }
 
 bool DirectivesParser::test() {
--- a/src/share/vm/compiler/directivesParser.hpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/compiler/directivesParser.hpp	Wed Dec 16 15:38:28 2015 +0100
@@ -126,8 +126,10 @@
   DirectiveSet*       current_directiveset;
 
   void push_tmp(CompilerDirectives* dir);
+  void clean_tmp();
   CompilerDirectives* pop_tmp();
   CompilerDirectives* _tmp_top; // temporary storage for dirs while parsing
+  int _tmp_depth;               // Number of directives that has been parsed but not installed.
 
   static uint mask(keytype kt);
 
--- a/src/share/vm/runtime/globals.hpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/runtime/globals.hpp	Wed Dec 16 15:38:28 2015 +0100
@@ -4268,8 +4268,11 @@
   diagnostic(bool, CompilerDirectivesIgnoreCompileCommands, false,          \
              "Disable backwards compatibility for compile commands.")       \
                                                                             \
-  diagnostic(bool, PrintCompilerDirectives, false,                          \
-             "Print compiler directives on installation.")
+  diagnostic(bool, CompilerDirectivesPrint, false,                          \
+             "Print compiler directives on installation.")                  \
+  diagnostic(int,  CompilerDirectivesLimit, 50,                             \
+             "Limit on number of compiler directives.")
+
 
 /*
  *  Macros for factoring of globals
--- a/src/share/vm/utilities/json.cpp	Wed Dec 16 18:38:02 2015 +0300
+++ b/src/share/vm/utilities/json.cpp	Wed Dec 16 15:38:28 2015 +0100
@@ -750,7 +750,6 @@
 
   JSONTest::test("{ key : 1 }", true);
   JSONTest::test("{ key : 1, }", true);
-  JSONTest::test("{ key : 1.2 }", true);
   JSONTest::test("{ key : true }", true);
   JSONTest::test("{ key : true, }", true);
   JSONTest::test("{ key : false }", true);
--- a/test/compiler/compilercontrol/parser/DirectiveStressTest.java	Wed Dec 16 18:38:02 2015 +0300
+++ b/test/compiler/compilercontrol/parser/DirectiveStressTest.java	Wed Dec 16 15:38:28 2015 +0100
@@ -44,7 +44,7 @@
 public class DirectiveStressTest {
     private static final int AMOUNT = Integer.getInteger(
             "compiler.compilercontrol.parser.DirectiveStressTest.amount",
-            Short.MAX_VALUE * 2 + 2);
+            999);
     private static final List<MethodDescriptor> DESCRIPTORS
             = new PoolHelper().getAllMethods().stream()
                     .map(pair -> AbstractTestBase.getValidMethodDescriptor(
--- a/test/compiler/compilercontrol/parser/HugeDirectiveUtil.java	Wed Dec 16 18:38:02 2015 +0300
+++ b/test/compiler/compilercontrol/parser/HugeDirectiveUtil.java	Wed Dec 16 15:38:28 2015 +0100
@@ -117,6 +117,7 @@
         try {
             output = ProcessTools.executeTestJvm(
                     "-XX:+UnlockDiagnosticVMOptions",
+                    "-XX:CompilerDirectivesLimit=1000",
                     "-XX:CompilerDirectivesFile=" + fileName,
                     "-version");
         } catch (Throwable thr) {