Merge pull request #16423 from ChengJin01/fix_bcv_init_flag_stackmaps

Rectify the flag setting for uninitializedThis in the stackmaps
eclipse-openj9 · Dec 8, 2022 · 6863489 · 6863489
2 parents e42a04b + 31ebe05
commit 6863489
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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;