Skip to content

Commit 5ad9b06

Browse files
committed
Guard against overflow of String Builder Transformer estimate
String Builder Transformer uses the result of getStringUTF8Length to estimate the StringBuilder buffer size needed to accommodate appending a constant String to a StringBuilder. That could overestimate the space required. This has been changed to use getStringLength instead, to use the actual lengths of constant String objects. A test has also been added to detect integer overflow of the capacity estimate, aborting the transformation, as StringBuilder.<init> will throw a NegativeArraySizeException if the specified capacity is negative. Signed-off-by: Henry Zongaro <zongaro@ca.ibm.com>
1 parent b6b5bb6 commit 5ad9b06

File tree

1 file changed

+25
-4
lines changed

1 file changed

+25
-4
lines changed

runtime/compiler/optimizer/StringBuilderTransformer.cpp

+25-4
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,15 @@ int32_t TR_StringBuilderTransformer::performOnBlock(TR::Block* block)
142142
{
143143
int32_t capacity = computeHeuristicStringBuilderInitCapacity(appendArguments);
144144

145+
// Guard against the possibility that the computed capacity has overflowed,
146+
// as StringBuilder.<init>(I) will throw a NegativeArraySizeException if the
147+
// capacity argument is negative. It is extremely unlikely that the capacity
148+
// calculation will overflow, but possible.
149+
if (capacity < 0)
150+
{
151+
return 1;
152+
}
153+
145154
if (performTransformation(comp(), "%sTransforming java/lang/StringBuilder.<init>()V call at node [0x%p] to java/lang/StringBuilder.<init>(I)V with capacity = %d\n", OPT_DETAILS, initNode, capacity))
146155
{
147156
static const bool collectAppendStatistics = feGetEnv("TR_StringBuilderTransformerCollectAppendStatistics") != NULL;
@@ -465,14 +474,23 @@ TR::Node* TR_StringBuilderTransformer::findStringBuilderChainedAppendArguments(T
465474
* The heuristic used in the non-constant case is hard coded per object type and was determined using the
466475
* statistics collection mechanisms implemented by this optimization. See TR_StringBuilderTransformer Environment
467476
* Variables section for more details.
477+
*
478+
* \returns A non-negative computed capacity or a negative value if the capacity estimate resulted in an integer overflow
468479
*/
469480
int32_t TR_StringBuilderTransformer::computeHeuristicStringBuilderInitCapacity(List<TR_Pair<TR::Node*, TR::RecognizedMethod> >& appendArguments)
470481
{
471-
int32_t capacity = 0;
482+
uint32_t capacity = 0;
472483

473484
ListIterator<TR_Pair<TR::Node*, TR::RecognizedMethod> > iter(&appendArguments);
474485

475-
for (TR_Pair<TR::Node*, TR::RecognizedMethod>* pair = iter.getFirst(); pair != NULL; pair = iter.getNext())
486+
// Iterate over pairs of recognized methods and arguments that can be used
487+
// to estimate the size of the StringBuilder buffer that will be needed.
488+
// If the estimated capacity ever exceeds the maximum int32_t value, the
489+
// calculation has overflowed, so halt early.
490+
//
491+
for (TR_Pair<TR::Node*, TR::RecognizedMethod>* pair = iter.getFirst();
492+
(pair != NULL) && (capacity <= std::numeric_limits<int32_t>::max());
493+
pair = iter.getNext())
476494
{
477495
TR::Node* argument = pair->getKey();
478496

@@ -595,7 +613,7 @@ int32_t TR_StringBuilderTransformer::computeHeuristicStringBuilderInitCapacity(L
595613
{
596614
uintptr_t stringObjectLocation = (uintptr_t)symbol->castToStaticSymbol()->getStaticAddress();
597615
uintptr_t stringObject = comp()->fej9()->getStaticReferenceFieldAtAddress(stringObjectLocation);
598-
capacity += comp()->fe()->getStringUTF8Length(stringObject);
616+
capacity += comp()->fej9()->getStringLength(stringObject);
599617

600618
break;
601619
}
@@ -625,5 +643,8 @@ int32_t TR_StringBuilderTransformer::computeHeuristicStringBuilderInitCapacity(L
625643
}
626644
}
627645

628-
return capacity;
646+
// If the loop has halted early because the value of capacity is greater than the
647+
// int32_t value, casting the capacity to int32_t will yield a negative result,
648+
// signalling that the capacity calculation has failed.
649+
return (int32_t) capacity;
629650
}

0 commit comments

Comments
 (0)