Skip to content

Commit

Permalink
Disallow reuse of PInvoke transition frame by marshalling helper calls (
Browse files Browse the repository at this point in the history
#96047)

* Disallow reuse of PInvoke transition frame by marshalling helper calls

Fixes #95887

* Delete redundant check
  • Loading branch information
jkotas authored Dec 16, 2023
1 parent 09536ba commit 5d2c901
Showing 1 changed file with 23 additions and 28 deletions.
51 changes: 23 additions & 28 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5291,6 +5291,14 @@ bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block)
return true;
}

// The VM assumes that the PInvoke frame in IL Stub is only going to be used
// for the PInvoke target call. The PInvoke frame cannot be reused by marshalling helper
// calls (see InlinedCallFrame::GetActualInteropMethodDesc and related stackwalking code).
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB))
{
return false;
}

#ifdef USE_PER_FRAME_PINVOKE_INIT
// For platforms that use per-P/Invoke InlinedCallFrame initialization,
// we can't inline P/Invokes inside of try blocks where we can resume execution in the same function.
Expand All @@ -5301,18 +5309,6 @@ bool Compiler::impCanPInvokeInlineCallSite(BasicBlock* block)
// re-entering the same method frame as the InlinedCallFrame after an exception in unmanaged code.
if (block->hasTryIndex())
{
// This does not apply to the raw pinvoke call that is inside the pinvoke
// ILStub. In this case, we have to inline the raw pinvoke call into the stub,
// otherwise we would end up with a stub that recursively calls itself, and end
// up with a stack overflow.
// This works correctly because the runtime never emits a catch block in a managed-to-native
// IL stub. If the runtime ever emits a catch block into a managed-to-native stub when using
// P/Invoke helpers, this condition will need to be revisited.
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && opts.ShouldUsePInvokeHelpers())
{
return true;
}

// Check if this block's try block or any containing try blocks have catch handlers.
// If any of the containing try blocks have catch handlers,
// we cannot inline a P/Invoke for reasons above. If the handler is a fault or finally handler,
Expand Down Expand Up @@ -5415,6 +5411,13 @@ void Compiler::impCheckForPInvokeCall(

// PInvoke CALLI in IL stubs must be inlined
}
else if (!IsTargetAbi(CORINFO_NATIVEAOT_ABI) && opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) &&
opts.IsReadyToRun())
{
// The raw PInvoke call that is inside the no marshalling R2R compiled pinvoke ILStub must
// be inlined into the stub, otherwise we would end up with a stub that recursively calls
// itself, and end up with a stack overflow.
}
else
{
// Check legality
Expand All @@ -5427,25 +5430,17 @@ void Compiler::impCheckForPInvokeCall(
// inlining in NativeAOT. Skip the ambient conditions checks and profitability checks.
if (!IsTargetAbi(CORINFO_NATIVEAOT_ABI) || (info.compFlags & CORINFO_FLG_PINVOKE) == 0)
{
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_IL_STUB) && opts.ShouldUsePInvokeHelpers())
if (!impCanPInvokeInline())
{
// Raw PInvoke call in PInvoke IL stub generated must be inlined to avoid infinite
// recursive calls to the stub.
return;
}
else
{
if (!impCanPInvokeInline())
{
return;
}

// Size-speed tradeoff: don't use inline pinvoke at rarely
// executed call sites. The non-inline version is more
// compact.
if (block->isRunRarely())
{
return;
}
// Size-speed tradeoff: don't use inline pinvoke at rarely
// executed call sites. The non-inline version is more
// compact.
if (block->isRunRarely())
{
return;
}
}

Expand Down

0 comments on commit 5d2c901

Please sign in to comment.