Fix the crash issue with the error message framework in Verifier

[ChengJin01]

The change is to resolve the crash issue specific to a stackmap
frame without any element in 'locals' and 'stack' when allocating
the memory of stackmap frame in the error message framework
during the runtime verification.

Fixes: #15639

Signed-off-by: Cheng Jin jincheng@ca.ibm.com

[ChengJin01]

The PR was verified in internal & open builds.

Reviewer: @DanHeidinga
FYI: @gacholio, @tajila, @pshipton

[gacholio]

Is there also a missing check for allocation failure (due to the huge size) later on?

[ChengJin01]

Is there also a missing check for allocation failure (due to the huge size) later on?

We already had the check in the existing code for the allocation failure in such case in allocateMemoryToVerificationTypeBuffer at https://github.com/eclipse-openj9/openj9/blob/f3caaa782a6f639aba1c1aac43942d416c443d34/runtime/verbose/errormessagehelper.c:

tatic VerificationTypeInfo*
allocateMemoryToVerificationTypeBuffer(MethodContextInfo* methodInfo, StackMapFrame* stackMapFrame, VerificationTypeInfo* currentVerificationTypeEntry, UDATA slotCount)
{
	PORT_ACCESS_FROM_PORT(methodInfo->portLib);
	UDATA numberOfSlots = stackMapFrame->numberOfSlots;
	UDATA slotIndex = 0;

	Assert_VRB_notNull(currentVerificationTypeEntry);

	/* Increase the buffer size of entries if full or the existing buffer size is less than the required slot count */
	slotIndex = currentVerificationTypeEntry - stackMapFrame->entries;
	if ((numberOfSlots - slotIndex) <= slotCount) {
		VerificationTypeInfo* newEntries = NULL;
		numberOfSlots = slotIndex + slotCount + 1;

		newEntries = (VerificationTypeInfo *)j9mem_reallocate_memory(stackMapFrame->entries, (sizeof(VerificationTypeInfo) * numberOfSlots), OMRMEM_CATEGORY_VM);
		if (NULL == newEntries) { <----------------
			Trc_VRB_Reallocate_Memory_Failed(slotIndex, numberOfSlots);
			currentVerificationTypeEntry = NULL;
			goto exit;
		}

[gacholio]

OK, but I'm wondering why we crashed before - if the huge allocation were properly checked, wouldn't that result in an out of memory error being reported?

[ChengJin01]

OK, but I'm wondering why we crashed before - if the huge allocation were properly checked, wouldn't that result in an out of memory error being reported?

The crash occurred in releaseVerificationTypeBuffer (called by generateJ9RtvExceptionDetails) when attempting to release an invalid/incomplete stackmap frame (which points to the memory which was partially allocated & released) as follows at https://github.com/eclipse-openj9/openj9/blob/master/runtime/verbose/errormessageframeworkrtv.c

U_8*
generateJ9RtvExceptionDetails(J9BytecodeVerificationData* verifyData, U_8* initMsgBuffer, UDATA* msgBufferLength)
{

case BCV_ERR_STACK_SIZE_MISMATCH:
		printMessage(&msgBuf, "Current frame's stack size doesn't match stackmap.");
returned false due to out-of-memory ---> printStackFrame = setStackMapFrameWithIndex(verifyData, &methodInfo, &stackMapFrameTarget);
		break;
...
/* Stackmap Frame: */
	if (printStackFrame) { <-------- false in which case stackMapFrameTarget had no entry
		/* Specify if the stack frame is from classfile or internal */
		if (verifyData->createdStackMap) {
			printMessage(&msgBuf, "\n%*sStackmap Frame (FallBack):", INDENT(2));
		} else {
			printMessage(&msgBuf, "\n%*sStackmap Frame:", INDENT(2));
		}
		printTheStackMapFrame(&msgBuf, &stackMapFrameTarget, &methodInfo);
	}

	/* Release memory allocated for the stackmap frame */
	releaseVerificationTypeBuffer(&stackMapFrameTarget, &methodInfo); <-------


void
releaseVerificationTypeBuffer(StackMapFrame* stackMapFrame, MethodContextInfo* methodInfo)
{
	if (NULL != stackMapFrame->entries) {
		PORT_ACCESS_FROM_PORT(methodInfo->portLib);
		j9mem_free_memory(stackMapFrame->entries); <------- crashed as the pointer was invalid
	}
}

in which case I just set stackMapFrame->entries to null after releasing the memory to avoid wild pointer in this case.

In addition, we skip the stackmap frames in huge size in the case of out-of-memory in our error message framework to ensure the rest of stackmap frames in an appropriate size are correctly allocated & outputted to the error message so as to provide users with sufficient information of what is really happening in the problematic class.

[ChengJin01]

Please note the issue detected at #15639 had nothing to do with the out-of-memory case but an incorrect last locals index that was wrongly set & converted to a huge size in allocating the stackmap frame, which misleadingly ended up with an out-of-memory case.

[gacholio]

Right, the original issue was corruption from an overrun.

[gacholio]

jenkins test sanity,extended win jdk8