[mono][interp] Link try bblock with leave targets from catch block

[BrzVlad]

Otherwise SSA dominance algorithms may fail to detect certain code flow and incorrectly manage phi nodes.

public bool M() {
	bool ret = true;
	try {
		// throw NULL
	} catch (NullReferenceException) {
		goto Label;
	}
	ret = false;
Label:
	return ret;
}

Without this fix, given the SSA cfg doesn't include EH bblocks, we would see the return opcode always reachable through the fallthrough from ret = false.

[Copilot]

Pull Request Overview

This PR fixes the control flow issue in the interpreter by linking try basic blocks with corresponding leave targets from catch blocks to ensure correct SSA dominance and phi node management.

• Introduces a new flag "linked_try_bblocks" in the TransformData structure.
• Adds DFS logic to connect try blocks to leave targets when processing exception handling edges in the CFG.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/mono/mono/mini/interp/transform.h	Added a flag to track linking of try blocks.
src/mono/mono/mini/interp/transform-opt.c	Implemented DFS linking for try blocks and updated DFS visitation to invoke linking.

[dotnet-policy-service]

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

[jakobbotsch]

I'm curious – I thought Mono handled variable uses reachable from EH conservatively (from our discussion in #96315 (comment)). Is it not the case?

[BrzVlad]

Yes, that is how it works. If ret was used in the catch handler everything would work ok. However, in our case, ret is not such a variable since it is not used in exception handlers. So the problem is not really with variables used in EH handlers, but rather the control flow for the rest of the code, when jumping from catch handler. Before this change, the optimizations were not aware that there is a possible try -> catch -> Label flow, so code would have been optimized to always return false.

[jakobbotsch]

Ok -- so it's only for variables used directly in EH clauses, not for variable uses reachable from EH clauses.

[kotlarmilos]

I think the marked array should be reset between DFS runs.

[BrzVlad]

This wasn't necessary because we would never scan overlapping regions of code. However, this made me realize that if we have the same pattern inside a catch handler, then once again this fix is not enough. Having those nested catch blocks scanned also seems to be getting a bit too complex for what it's worth. I think I'll just end up disabling SSA optimizations for methods where a leave in a catch goes to a bblock that is not already a successor for the try bblock.