Skip to content

Commit 983fdc8

Browse files
authored
Merge pull request #22889 from theresa-m/fix_22812
Check stack map if type changed in the exception table range
2 parents 802f606 + 3a6e4b3 commit 983fdc8

2 files changed

Lines changed: 51 additions & 97 deletions

File tree

runtime/bcverify/rtverify.c

Lines changed: 51 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static IDATA matchStack (J9BytecodeVerificationData * verifyData, J9BranchTarget
4545
static IDATA findAndMatchStack (J9BytecodeVerificationData *verifyData, IDATA targetPC, IDATA currentPC);
4646
static IDATA verifyExceptions (J9BytecodeVerificationData *verifyData);
4747
static J9BranchTargetStack * nextStack (J9BytecodeVerificationData *verifyData, UDATA *nextMapIndex, IDATA *nextStackPC);
48-
static IDATA nextExceptionStart (J9BytecodeVerificationData *verifyData, J9ROMMethod *romMethod, IDATA lastPC);
48+
static void nextExceptionPC (J9BytecodeVerificationData *verifyData, J9ROMMethod *romMethod, IDATA *exceptionStartPC, IDATA *exceptionEndPC);
4949
static void storeArgumentErrorData (J9BytecodeVerificationData * verifyData, U_32 errorCurrentFramePosition, U_16 errorArgumentIndex);
5050
static void storeMethodInfo (J9BytecodeVerificationData * verifyData, J9UTF8* errorClassString, J9UTF8* errorMethodString, J9UTF8* errorSignatureString, IDATA currentPC);
5151

@@ -447,16 +447,16 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
447447
UDATA classIndex, maxStack;
448448
UDATA wideIndex = FALSE;
449449
IDATA nextStackPC;
450-
IDATA nextExceptionStartPC;
450+
IDATA exceptionStartPC = -1;
451+
IDATA exceptionEndPC = -1;
451452
IDATA rc = 0;
452453
U_8 returnBytecode;
453454
UDATA errorModule = J9NLS_BCV_ERR_BYTECODES_INVALID__MODULE; /* defaults to BCV NLS catalog */
454455
U_16 errorType;
455456
I_16 offset16;
456457
I_32 offset32;
457458
UDATA argCount;
458-
UDATA checkIfInsideException = romMethod->modifiers & J9AccMethodHasExceptionInfo;
459-
UDATA tempStoreChange;
459+
BOOLEAN tempStoreChange = FALSE;
460460
J9ExceptionInfo *exceptionInfo = J9_EXCEPTION_DATA_FROM_ROM_METHOD(romMethod);
461461
J9ExceptionHandler *handler;
462462
J9UTF8 *catchName;
@@ -470,7 +470,6 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
470470
UDATA errorTargetType = (UDATA)-1;
471471
UDATA errorStackIndex = (UDATA)-1;
472472
UDATA errorTempData = (UDATA)-1;
473-
BOOLEAN isNextStack = FALSE;
474473
#if defined(J9VM_OPT_VALHALLA_STRICT_FIELDS)
475474
BOOLEAN anotherInstanceInitCalled = FALSE;
476475
#endif /* defined(J9VM_OPT_VALHALLA_STRICT_FIELDS) */
@@ -524,7 +523,7 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
524523
currentMapData = nextStack (verifyData, &nextMapIndex, &nextStackPC);
525524

526525
/* Determine where the first region of bytecodes covered by an exception handler is */
527-
nextExceptionStartPC = nextExceptionStart (verifyData, romMethod, -1);
526+
nextExceptionPC (verifyData, romMethod, &exceptionStartPC, &exceptionEndPC);
528527

529528
/* walk the bytecodes linearly */
530529
while (pc < length) {
@@ -571,8 +570,19 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
571570
currentMapData = nextStack (verifyData, &nextMapIndex, &nextStackPC);
572571
}
573572

574-
/* Check the stack against the exception handler stack */
575-
if (pc == (UDATA) nextExceptionStartPC) {
573+
/* Check the current state against the exception handlers if:
574+
* - This is the first bytecode of the try/catch block
575+
* - The temps (locals) type state has changed due to a store
576+
* instruction. If it is within the same exception range ensure
577+
* that the types are still compatible.
578+
*/
579+
if ((pc == (UDATA)exceptionStartPC)
580+
|| (tempStoreChange && ((exceptionStartPC < pc) && (pc < exceptionEndPC)))
581+
) {
582+
/* If this is the first pc of an exception tempStoreChange does not apply. */
583+
if ((pc == (UDATA)exceptionStartPC) && tempStoreChange) {
584+
tempStoreChange = FALSE;
585+
}
576586
handler = J9EXCEPTIONINFO_HANDLERS(exceptionInfo);
577587
SAVE_STACKTOP(liveStack, stackTop);
578588

@@ -583,7 +593,9 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
583593

584594
/* Find all exception handlers from here */
585595
for (i = exceptionInfo->catchCount; i; i--, handler++) {
586-
if (handler->startPC == pc) {
596+
if ((handler->startPC == pc)
597+
|| (tempStoreChange && ((UDATA)pc >= handler->startPC) && ((UDATA)pc < handler->endPC))
598+
) {
587599
/* Check the maps at the handler PC */
588600
/* Modify the liveStack temporarily to contain the handler exception */
589601
if (handler->exceptionClassIndex) {
@@ -613,8 +625,13 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
613625
liveStack->stackElements[liveStack->stackBaseIndex] = originalStackZeroEntry;
614626
stackTop = originalStackTop;
615627

616-
/* Get next exception start PC of interest */
617-
nextExceptionStartPC = nextExceptionStart (verifyData, romMethod, nextExceptionStartPC);
628+
if (!tempStoreChange) {
629+
/* Get next exception start PC of interest */
630+
nextExceptionPC (verifyData, romMethod, &exceptionStartPC, &exceptionEndPC);
631+
}
632+
}
633+
if (tempStoreChange) {
634+
tempStoreChange = FALSE;
618635
}
619636

620637
bcIndex = code + pc;
@@ -646,8 +663,6 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
646663
((U_32)(stackBase - liveStack->stackElements) - 1) : 0;
647664
goto _miscError;
648665
}
649-
/* Reset as the flag is only used for the current bytecode */
650-
isNextStack = FALSE;
651666

652667
/* Format: 8bits action, 4bits type Y, 4bits type X */
653668
type1 = (UDATA) J9JavaBytecodeVerificationTable[bc];
@@ -812,8 +827,6 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
812827
goto _inconsistentStack;
813828
}
814829

815-
tempStoreChange = FALSE;
816-
817830
if (type != type1) {
818831
if ((type1 != BCV_GENERIC_OBJECT) || (type & BCV_TAG_BASE_TYPE_OR_TOP)) {
819832
inconsistentStack = TRUE;
@@ -833,56 +846,6 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
833846
}
834847
tempStoreChange |= (type != temps[index]);
835848
STORE_TEMP(index, type);
836-
837-
if (checkIfInsideException && tempStoreChange) {
838-
/* If we've stored a value into an arg/local, and it's of a different type than was
839-
* originally there, we need to ensure that we are still compatible with all our
840-
* exception handlers.
841-
*
842-
* For all exception handlers covering this instruction
843-
*/
844-
handler = J9EXCEPTIONINFO_HANDLERS(exceptionInfo);
845-
SAVE_STACKTOP(liveStack, stackTop);
846-
847-
/* Save the current liveStack element zero */
848-
/* Reset the stack pointer to push the exception on the empty stack */
849-
originalStackTop = stackTop;
850-
originalStackZeroEntry = liveStack->stackElements[liveStack->stackBaseIndex];
851-
852-
/* Find all exception handlers from here */
853-
for (i = exceptionInfo->catchCount; i; i--, handler++) {
854-
if (((UDATA) start >= handler->startPC) && ((UDATA) start < handler->endPC)) {
855-
#ifdef DEBUG_BCV
856-
printf("exception map change check at startPC: %d\n", handler->startPC);
857-
#endif
858-
/* Check the maps at the handler PC */
859-
/* Modify the liveStack temporarily to contain the handler exception */
860-
if (handler->exceptionClassIndex) {
861-
catchName = J9ROMSTRINGREF_UTF8DATA((J9ROMStringRef *)(&constantPool [handler->exceptionClassIndex]));
862-
catchClass = convertClassNameToStackMapType(verifyData, J9UTF8_DATA(catchName), J9UTF8_LENGTH(catchName), 0, 0);
863-
} else {
864-
catchClass = BCV_JAVA_LANG_THROWABLE_INDEX;
865-
catchClass <<= BCV_CLASS_INDEX_SHIFT;
866-
}
867-
868-
stackTop = &(liveStack->stackElements[liveStack->stackBaseIndex]);
869-
PUSH(catchClass);
870-
SAVE_STACKTOP(liveStack, stackTop);
871-
872-
rc = findAndMatchStack (verifyData, handler->handlerPC, start);
873-
if (BCV_SUCCESS != rc) {
874-
if (BCV_ERR_INSUFFICIENT_MEMORY == rc) {
875-
goto _outOfMemoryError;
876-
}
877-
goto _mapError;
878-
}
879-
}
880-
}
881-
882-
/* Restore liveStack */
883-
liveStack->stackElements[liveStack->stackBaseIndex] = originalStackZeroEntry;
884-
stackTop = originalStackTop;
885-
}
886849
break;
887850

888851
case RTV_ARRAY_STORE:
@@ -2389,7 +2352,6 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
23892352
/* Load the next stack into the live stack */
23902353
SAVE_STACKTOP(liveStack, stackTop);
23912354
memcpy((UDATA *) liveStack, (UDATA *) currentMapData, verifyData->stackSize);
2392-
isNextStack = TRUE;
23932355
RELOAD_LIVESTACK;
23942356

23952357
/* Start with uninitialized_this flag for the current map data */
@@ -2866,23 +2828,34 @@ nextStack (J9BytecodeVerificationData *verifyData, UDATA *nextMapIndex, IDATA *n
28662828
return returnStack;
28672829
}
28682830

2869-
/* Return the next pc in the method that is an exception handler start address */
2870-
2871-
static IDATA
2872-
nextExceptionStart (J9BytecodeVerificationData *verifyData, J9ROMMethod *romMethod, IDATA lastPC)
2831+
/**
2832+
* Return the pc range for the next exception handler in romMethod.
2833+
* @param verifyData pointer to J9BytecodeVerificationData
2834+
* @param romMethod the method whose exception handling information is searched
2835+
* @param exceptionStartPC contains the starting pc of the last exception. It will
2836+
* be set to the next exception start pc if one exists otherwise will be set to
2837+
* the last possible pc for the method.
2838+
* @param exceptionEndPC will be set to the next exception end pc if one exists
2839+
* otherwise will be set to the last possible pc for the method.
2840+
*/
2841+
static void
2842+
nextExceptionPC(J9BytecodeVerificationData *verifyData, J9ROMMethod *romMethod, IDATA *exceptionStartPC, IDATA *exceptionEndPC)
28732843
{
28742844
/* Method size */
2875-
IDATA nextPC = J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod);
2845+
IDATA nextStartPC = J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod);
2846+
IDATA nextEndPC = nextStartPC;
28762847

28772848
if (romMethod->modifiers & CFR_ACC_HAS_EXCEPTION_INFO) {
28782849
J9ExceptionInfo *exceptionInfo = J9_EXCEPTION_DATA_FROM_ROM_METHOD(romMethod);
28792850
J9ExceptionHandler *handler = J9EXCEPTIONINFO_HANDLERS(exceptionInfo);
28802851
UDATA i;
28812852

28822853
for (i = exceptionInfo->catchCount; i; i--, handler++) {
2883-
if (((IDATA) handler->startPC) > lastPC) {
2884-
if (handler->startPC < (UDATA) nextPC) {
2885-
nextPC = handler->startPC;
2854+
/* exceptionStartPC has the last exception start. */
2855+
if (((IDATA)handler->startPC) > *exceptionStartPC) {
2856+
if (handler->startPC < (UDATA) nextStartPC) {
2857+
nextStartPC = handler->startPC;
2858+
nextEndPC = handler->endPC;
28862859
}
28872860
}
28882861
}
@@ -2893,10 +2866,12 @@ nextExceptionStart (J9BytecodeVerificationData *verifyData, J9ROMMethod *romMeth
28932866
J9UTF8_DATA(J9ROMMETHOD_NAME(verifyData->romMethod)),
28942867
(UDATA) J9UTF8_LENGTH(J9ROMMETHOD_SIGNATURE(verifyData->romMethod)),
28952868
J9UTF8_DATA(J9ROMMETHOD_SIGNATURE(verifyData->romMethod)),
2896-
exceptionInfo->catchCount, lastPC, nextPC,
2869+
exceptionInfo->catchCount, *exceptionStartPC, nextStartPC,
28972870
J9_BYTECODE_SIZE_FROM_ROM_METHOD(romMethod));
28982871
}
2899-
return nextPC;
2872+
2873+
*exceptionStartPC = nextStartPC;
2874+
*exceptionEndPC = nextEndPC;
29002875
}
29012876

29022877

runtime/oti/bcverify.h

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -281,27 +281,6 @@ typedef struct J9BCVAlloc {
281281
temps[ ( tempIndex - 1 ) ] = BCV_BASE_TYPE_TOP; \
282282
}
283283

284-
#define STORE_TEMP_OBJECT( tempIndex, tempType ) \
285-
STORE_TEMP( tempIndex, tempType );
286-
287-
#define STORE_TEMP_INTEGER( tempIndex ) \
288-
STORE_TEMP( tempIndex, (UDATA) (BCV_BASE_TYPE_INT) );
289-
290-
#define STORE_TEMP_FLOAT( tempIndex ) \
291-
STORE_TEMP( tempIndex, (UDATA) (BCV_BASE_TYPE_FLOAT) );
292-
293-
/* direct write of the TOP avoids the pre-index check */
294-
295-
#define STORE_TEMP_PAIR( tempIndex, tempType ) \
296-
STORE_TEMP( tempIndex, tempType ); \
297-
temps[ ( tempIndex + 1 ) ] = (BCV_BASE_TYPE_TOP);
298-
299-
#define STORE_TEMP_LONG( tempIndex ) \
300-
STORE_TEMP_PAIR( tempIndex, (UDATA) (BCV_BASE_TYPE_LONG) );
301-
302-
#define STORE_TEMP_DOUBLE( tempIndex ) \
303-
STORE_TEMP_PAIR( tempIndex, (UDATA) (BCV_BASE_TYPE_DOUBLE) );
304-
305284
#define RELOAD_LIVESTACK \
306285
stackBase = &(liveStack->stackElements[liveStack->stackBaseIndex]); \
307286
stackTop = &(liveStack->stackElements[liveStack->stackTopIndex]); \

0 commit comments

Comments
 (0)