8144246: adding lots of directives via jcmd may produce OOM crash
Summary: Add a limit to the number of directives
Reviewed-by: kvn
--- a/hotspot/src/share/vm/compiler/compileBroker.cpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/src/share/vm/compiler/compilerDirectives.cpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/src/share/vm/compiler/compilerDirectives.hpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/src/share/vm/compiler/directivesParser.cpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/src/share/vm/compiler/directivesParser.hpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/src/share/vm/runtime/globals.hpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/src/share/vm/utilities/json.cpp Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/test/compiler/compilercontrol/parser/DirectiveStressTest.java Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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/hotspot/test/compiler/compilercontrol/parser/HugeDirectiveUtil.java Wed Dec 16 18:38:02 2015 +0300
+++ b/hotspot/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) {