Skip to content

Commit

Permalink
8271459: C2: Missing NegativeArraySizeException when creating StringB…
Browse files Browse the repository at this point in the history
…uilder with negative capacity

Reviewed-by: goetz
Backport-of: 3953e0774c59c5e936e752aa08b6b6778e232994
  • Loading branch information
TheRealMDoerr committed Oct 13, 2021
1 parent 4940e2e commit 844c504
Showing 1 changed file with 51 additions and 2 deletions.
53 changes: 51 additions & 2 deletions src/hotspot/share/opto/stringopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ class StringConcat : public ResourceObj {
StringMode,
IntMode,
CharMode,
StringNullCheckMode
StringNullCheckMode,
NegativeIntCheckMode
};

StringConcat(PhaseStringOpts* stringopts, CallStaticJavaNode* end):
Expand Down Expand Up @@ -119,12 +120,19 @@ class StringConcat : public ResourceObj {
void push_string(Node* value) {
push(value, StringMode);
}

void push_string_null_check(Node* value) {
push(value, StringNullCheckMode);
}

void push_negative_int_check(Node* value) {
push(value, NegativeIntCheckMode);
}

void push_int(Node* value) {
push(value, IntMode);
}

void push_char(Node* value) {
push(value, CharMode);
}
Expand Down Expand Up @@ -499,13 +507,35 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) {
#ifndef PRODUCT
if (PrintOptimizeStringConcat) {
tty->print("giving up because StringBuilder(null) throws exception");
alloc->jvms()->dump_spec(tty); tty->cr();
alloc->jvms()->dump_spec(tty);
tty->cr();
}
#endif
return NULL;
}
// StringBuilder(str) argument needs null check.
sc->push_string_null_check(use->in(TypeFunc::Parms + 1));
} else if (sig == ciSymbol::int_void_signature()) {
// StringBuilder(int) case.
Node* parm = use->in(TypeFunc::Parms + 1);
assert(parm != NULL, "must exist");
const TypeInt* type = _gvn->type(parm)->is_int();
if (type->_hi < 0) {
// Initial capacity argument is always negative in which case StringBuilder(int) throws
// a NegativeArraySizeException. Bail out from string opts.
#ifndef PRODUCT
if (PrintOptimizeStringConcat) {
tty->print("giving up because a negative argument is passed to StringBuilder(int) which "
"throws a NegativeArraySizeException");
alloc->jvms()->dump_spec(tty);
tty->cr();
}
#endif
return NULL;
} else if (type->_lo < 0) {
// Argument could be negative: We need a runtime check to throw NegativeArraySizeException in that case.
sc->push_negative_int_check(parm);
}
}
// The int variant takes an initial size for the backing
// array so just treat it like the void version.
Expand Down Expand Up @@ -1792,6 +1822,23 @@ void PhaseStringOpts::replace_string_concat(StringConcat* sc) {
for (int argi = 0; argi < sc->num_arguments(); argi++) {
Node* arg = sc->argument(argi);
switch (sc->mode(argi)) {
case StringConcat::NegativeIntCheckMode: {
// Initial capacity argument might be negative in which case StringBuilder(int) throws
// a NegativeArraySizeException. Insert a runtime check with an uncommon trap.
const TypeInt* type = kit.gvn().type(arg)->is_int();
assert(type->_hi >= 0 && type->_lo < 0, "no runtime int check needed");
Node* p = __ Bool(__ CmpI(arg, kit.intcon(0)), BoolTest::ge);
IfNode* iff = kit.create_and_map_if(kit.control(), p, PROB_MIN, COUNT_UNKNOWN);
{
// Negative int -> uncommon trap.
PreserveJVMState pjvms(&kit);
kit.set_control(__ IfFalse(iff));
kit.uncommon_trap(Deoptimization::Reason_intrinsic,
Deoptimization::Action_maybe_recompile);
}
kit.set_control(__ IfTrue(iff));
break;
}
case StringConcat::IntMode: {
Node* string_size = int_stringSize(kit, arg);

Expand Down Expand Up @@ -1961,6 +2008,8 @@ void PhaseStringOpts::replace_string_concat(StringConcat* sc) {
for (int argi = 0; argi < sc->num_arguments(); argi++) {
Node* arg = sc->argument(argi);
switch (sc->mode(argi)) {
case StringConcat::NegativeIntCheckMode:
break; // Nothing to do, was only needed to add a runtime check earlier.
case StringConcat::IntMode: {
start = int_getChars(kit, arg, dst_array, coder, start, string_sizes->in(argi));
break;
Expand Down

1 comment on commit 844c504

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.