Fix incorrect assumption around the presence of ICF frames in EH codebase for 64-bit targets

[fadimounir]

Fix for #34524.

The main issue here is a disconnect between what the JIT produces, and the assumption we had around it in the EH code:
https://github.com/dotnet/runtime/blob/master/src/coreclr/src/vm/exceptionhandling.cpp#L1824

Clearly this is not what the JIT does:
https://github.com/dotnet/runtime/blob/master/src/coreclr/src/jit/lower.cpp#L3497

This means that it is not correct to limit this define to only 32-bit targets:
https://github.com/dotnet/runtime/blob/master/src/coreclr/src/vm/exceptionhandling.h#L16

To fix this issue, we should have one standard for all targets (which is the case today anyways): The ICF is always initialized and linked in the prolog of any method that contains PInvokes, and unlinked at the method epilog for jitted code (R2R is a slightly different story)

[jkotas]

The ICF is always initialized and linked in the prolog of any method that contains PInvokes, and unlinked at the method epilog for jitted code (R2R is a slightly different story)

I thought that we agree on that link / unlink around PInvoke is the right thing to standardize on (ie same as R2R).

We can initialize the frame in the prolog or right before the PInvoke, but that is just a local detail - it does not affect how exception handling works.

[jkotas]

This will need matching changes in the JIT as well. You should see a functional test failures without matching changes in the JIT.

[fadimounir]

The JIT today links/unlinks the ICF in the method prolog/epilog, and activates/deactivates the ICF around the pinvoke call. It does this for both 32-bits and 64-bits. The fix here is to match the behavior in the EH codebase.

[janvorli]

I thought that we agree on that link / unlink around PInvoke is the right thing to standardize on (ie same as R2R).

I would also prefer that. The presence of linked but inactive InlinedCallFrame is just confusing.

[fadimounir]

As a second step after this fix is in, I can look into merging the linking+activation in one step around the pinvoke call (this will match the behavior of R2R code as well), but that would be a slightly bigger change I believe. At this point, I want to fix the bug first (to also unblock my other PR that is failing with gcstress runs because of this).

Should be an easy JIT change, but there's a lot of places on the runtime side I've seen where the assumption of disconnected linking/activation is baked in, and that will be a bigger change

[fadimounir]

gcstress results are comparable to a baseline I ran a few days ago: https://dev.azure.com/dnceng/public/_build/results?buildId=582616&view=ms.vss-test-web.build-test-results-tab

We can merge these changes at this point to unblock #33733, then I'll make the changes to activate+link the ICF around the pinvoke calls.

@jkotas @janvorli Does this plan sound good?

[jkotas]

Sounds good