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: roland
Backport-of: 844c504bc18fa8a79ceacc6b6f235650f5e643bf
  • Loading branch information
Alexey Pavlyutkin authored and Yuri Nesterenko committed Oct 14, 2022
1 parent 43cfe27 commit dfeeceb
Showing 1 changed file with 51 additions and 2 deletions.
53 changes: 51 additions & 2 deletions hotspot/src/share/vm/opto/stringopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ class StringConcat : public ResourceObj {
StringMode,
IntMode,
CharMode,
StringNullCheckMode
StringNullCheckMode,
NegativeIntCheckMode
};

StringConcat(PhaseStringOpts* stringopts, CallStaticJavaNode* end):
Expand Down Expand Up @@ -118,12 +119,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 @@ -484,13 +492,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 @@ -1477,6 +1507,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 @@ -1601,6 +1648,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: {
Node* end = __ AddI(start, string_sizes->in(argi));
// getChars words backwards so pass the ending point as well as the start
Expand Down

1 comment on commit dfeeceb

@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.