[mono][tests] Throw InvalidProgramException when ENDFILTER has invalid value type or invalid number of values

[kotlarmilos]

Fix for #54401

[ghost]

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fix for #54401

Author:	kotlarmilos
Assignees:	-
Labels:	area-Codegen-Interpreter-mono
Milestone:	-

[BrzVlad]

The title is not really related to the code change

[BrzVlad]

Why are we decreasing the stack pointer once more ? You would need to update the code below.

[kotlarmilos]

I followed the same approach as in mono jit where the error is thrown:

runtime/src/mono/mono/mini/method-to-ir.c

Lines 11786 to 11788 in a83c661

	--sp;
	if ((sp != stack_start) || (sp [0]->type != STACK_I4))
	UNVERIFIED;

I understood that on the top of stack is a result of filter and it should be the only value on the stack in new BB. So, if there is something else, just throw.

[BrzVlad]

You are modifying the value of the stack pointer, which breaks following code, when you should only be checking what it is at that location.

[kotlarmilos]

Just to check if I got it right, sp - 1 should be equal to stack_start and sp->type should be int32?

[BrzVlad]

td->sp points to the location where a new entry is allocated, so you are accessing junk here. You either access it with td->sp [-1], or better decrement td->sp and also update accordingly the code below that adds the MINT_ENDFILTER instruction

[kotlarmilos]

If we decrement td->sp and update accordingly the code that adds the MINT_ENDFILTER instruction, how td-sp is then rebased to the top of the stack in the next instruction?

[kotlarmilos]

Probably here?

runtime/src/mono/mono/mini/interp/transform.c

Lines 4595 to 4602 in a83c661

	if (new_bb->stack_height >= 0) {
	if (new_bb->stack_height > 0)
	memcpy (td->stack, new_bb->stack_state, new_bb->stack_height * sizeof(td->stack [0]));
	td->sp = td->stack + new_bb->stack_height;
	} else if (link_bblocks) {
	/* This bblock is not branched to. Initialize its stack state */
	init_bb_stack_state (td, new_bb);
	}

[BrzVlad]

Each instruction consumes some values from the top of the stack and pushes some result back. endfilter consumes one value from the top of the stack so td->sp is technically decreased by 1. In practice, it doesn't really matter since endifilter is the last instruction of the bblock, so the next bblock resets the pointer according to its stack height https://github.com/dotnet/runtime/blob/main/src/mono/mono/mini/interp/transform.c#L4603

[kotlarmilos]

Thanks for the explanation.