From 31ebe057325bef36b90a433b61b547981b1a4d37 Mon Sep 17 00:00:00 2001 From: ChengJin01 Date: Sun, 4 Dec 2022 23:54:11 -0500 Subject: [PATCH] Rectify the flag setting for uninitializedThis in the stackmaps The changes are to fix the issue with the flags setting intended for uninitializedThis in the stackmaps so as to match the JVM Spec and the RI's behavior by explicitly outputting the detailed error messages for the mismatch captured in the stackmaps. Internal ref 147737 Signed-off-by: ChengJin01 --- runtime/bcverify/bcverify.c | 17 ++++++++++++++--- runtime/bcverify/rtverify.c | 1 + runtime/oti/j9consts.h | 1 + runtime/verbose/errormessageframeworkrtv.c | 17 +++++++++-------- 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/runtime/bcverify/bcverify.c b/runtime/bcverify/bcverify.c index e6c7fd1cae3..adbb5759ad9 100644 --- a/runtime/bcverify/bcverify.c +++ b/runtime/bcverify/bcverify.c @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 1991, 2021 IBM Corp. and others + * Copyright (c) 1991, 2022 IBM Corp. and others * * This program and the accompanying materials are made available under * the terms of the Eclipse Public License 2.0 which accompanies this @@ -107,8 +107,19 @@ setInitializedThisStatus(J9BytecodeVerificationData *verifyData) if (currentStack->stackBaseIndex != -1) { BOOLEAN flag_uninitialized = FALSE; IDATA i = 0; - for (; i < currentStack->stackTopIndex; i++) { - if ((currentStack->stackElements[i] & BCV_SPECIAL_INIT) == BCV_SPECIAL_INIT) { + /* The operation stack frame in the Verifier is as follows: + * |-------- Locals --------|-------- Stacks ----------| + * 0 stackBaseIndex stackTopIndex + * which means 'Locals' starts from 0 to stackBaseIndex while 'Stacks' starts from stackBaseIndex + * to stackTopIndex. + * + * The JVM Spec at 4.10.1.4 Stack Map Frame Representation says that if any local variable in 'Locals' + * has the type 'uninitializedThis', then 'Flags' has the single element 'flagThisUninit'; otherwise + * 'Flags' is an empty list. That is to say, we only check the elements of 'Locals' rather than the + * whole operation stack frame to determine whether to set the flags. + */ + for (; i < currentStack->stackBaseIndex; i++) { + if (J9_ARE_ALL_BITS_SET(currentStack->stackElements[i], BCV_SPECIAL_INIT)) { flag_uninitialized = TRUE; break; } diff --git a/runtime/bcverify/rtverify.c b/runtime/bcverify/rtverify.c index e46e90de0bf..fd67138786b 100644 --- a/runtime/bcverify/rtverify.c +++ b/runtime/bcverify/rtverify.c @@ -312,6 +312,7 @@ matchStack(J9BytecodeVerificationData * verifyData, J9BranchTargetStack *liveSta /* Note: Target stack frame flag needs to be subset of ours. See JVM sepc 4.10.1.4 */ if (liveStack->uninitializedThis && !targetStack->uninitializedThis) { rc = BCV_FAIL; + verifyData->errorDetailCode = BCV_ERR_INIT_FLAGS_MISMATCH; goto _finished; } diff --git a/runtime/oti/j9consts.h b/runtime/oti/j9consts.h index 1acecee0883..c2adc576811 100644 --- a/runtime/oti/j9consts.h +++ b/runtime/oti/j9consts.h @@ -567,6 +567,7 @@ extern "C" { #define BCV_ERR_INACCESSIBLE_CLASS -33 #define BCV_ERR_BYTECODE_ERROR -34 #define BCV_ERR_NEW_OJBECT_MISMATCH -35 +#define BCV_ERR_INIT_FLAGS_MISMATCH -36 #define J9_GC_OBJ_HEAP_HOLE 0x1 #define J9_GC_MULTI_SLOT_HOLE 0x1 diff --git a/runtime/verbose/errormessageframeworkrtv.c b/runtime/verbose/errormessageframeworkrtv.c index ec8c7914b3c..021d1b7b7eb 100644 --- a/runtime/verbose/errormessageframeworkrtv.c +++ b/runtime/verbose/errormessageframeworkrtv.c @@ -504,6 +504,7 @@ adjustStackTopForWrongDataType(J9BytecodeVerificationData *verifyData) case BCV_ERR_STACKMAP_FRAME_LOCALS_UNDERFLOW: /* FALLTHROUGH */ case BCV_ERR_STACKMAP_FRAME_LOCALS_OVERFLOW: /* FALLTHROUGH */ case BCV_ERR_STACKMAP_FRAME_STACK_OVERFLOW: /* FALLTHROUGH */ + case BCV_ERR_INIT_FLAGS_MISMATCH: /* FALLTHROUGH */ case BCV_ERR_INIT_NOT_CALL_INIT: /* FALLTHROUGH */ case BCV_ERR_WRONG_TOP_TYPE: /* FALLTHROUGH */ case BCV_ERR_INVALID_ARRAY_REFERENCE: /* FALLTHROUGH */ @@ -772,14 +773,13 @@ printReasonForFlagMismatch(MessageBuffer *msgBuf, J9BytecodeVerificationData *ve goto exit; } - /* Compare the flag value if the bci value matches */ - if (currentFrame->bci == targetFrame->bci) { - BOOLEAN matchFlag = compareStackMapFrameFlag(currentFrame, targetFrame); - if (!matchFlag) { - printMessage(msgBuf, "Current frame's flags are not assignable to stack map frame's."); - result = TRUE; - goto exit; - } + /* Compare the flag value regardless of the bci value as the comparison + * might happen to the branch bytecode specified in the stackmap frame. + */ + if (FALSE == compareStackMapFrameFlag(currentFrame, targetFrame)) { + printMessage(msgBuf, "Current frame's flags are not assignable to stack map frame's."); + result = TRUE; + goto exit; } } } @@ -871,6 +871,7 @@ generateJ9RtvExceptionDetails(J9BytecodeVerificationData* verifyData, U_8* initM printMessage(&msgBuf, "Current frame's stack size doesn't match stackmap."); printStackFrame = setStackMapFrameWithIndex(verifyData, &methodInfo, &stackMapFrameTarget); break; + case BCV_ERR_INIT_FLAGS_MISMATCH: /* FALLTHROUGH */ case BCV_ERR_INIT_NOT_CALL_INIT: printStackFrame = printReasonForFlagMismatch(&msgBuf, verifyData, &methodInfo, &stackMapFrameCurrent, &stackMapFrameTarget); printCurrentStack = printStackFrame;