Skip to content

Block inlines with different exception wrapping behavior #113849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,23 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO
MethodDesc callerMethod = HandleToObject(callerHnd);
MethodDesc calleeMethod = HandleToObject(calleeHnd);

EcmaModule rootModule = (MethodBeingCompiled.OwningType as MetadataType)?.Module as EcmaModule;
EcmaModule calleeModule = (calleeMethod.OwningType as MetadataType)?.Module as EcmaModule;

// If this inline crosses module boundaries, ensure the modules agree on exception wrapping behavior.
if ((rootModule != calleeModule) && (rootModule != null) && (calleeModule != null))
{
if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows)
{
var calleeIL = _compilation.GetMethodIL(calleeMethod);
if (calleeIL.GetExceptionRegions().Length != 0)
{
// Fail inlining if root method and callee have different exception wrapping behavior
return CorInfoInline.INLINE_FAIL;
}
}
}

if (_compilation.CanInline(callerMethod, calleeMethod))
{
// No restrictions on inlining
Expand All @@ -1266,6 +1283,10 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO
else
{
// Call may not be inlined
//
// Note despite returning INLINE_NEVER here, in compilations where jitting is possible
// the jit may still be able to inline this method. So we rely on reportInliningDecision
// to not propagate this into metadata to short-circuit future inlining attempts.
return CorInfoInline.INLINE_NEVER;
}
}
Expand Down
49 changes: 49 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public partial class EcmaModule : ModuleDesc
{
private readonly PEReader _peReader;
protected readonly MetadataReader _metadataReader;
private volatile bool _isWrapNonExceptionThrowsComputed;
private volatile bool _isWrapNonExceptionThrows;

internal interface IEntityHandleObject
{
Expand Down Expand Up @@ -690,5 +692,52 @@ public override string ToString()
ModuleDefinition moduleDefinition = _metadataReader.GetModuleDefinition();
return _metadataReader.GetString(moduleDefinition.Name);
}

public bool IsWrapNonExceptionThrows
{
get
{
if (!_isWrapNonExceptionThrowsComputed)
{
ComputeIsWrapNonExceptionThrows();
_isWrapNonExceptionThrowsComputed = true;
}
return _isWrapNonExceptionThrows;
}
}

private void ComputeIsWrapNonExceptionThrows()
{
var reader = MetadataReader;
var c = reader.StringComparer;
bool foundAttribute = false;
foreach (var attr in reader.GetAssemblyDefinition().GetCustomAttributes())
{
if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n))
{
if (c.Equals(ns, "System.Runtime.CompilerServices") && c.Equals(n, "RuntimeCompatibilityAttribute"))
{
var dec = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider(this));

foreach (var arg in dec.NamedArguments)
{
if (arg.Name == "WrapNonExceptionThrows")
{
if (!(arg.Value is bool))
ThrowHelper.ThrowBadImageFormatException();
_isWrapNonExceptionThrows = (bool)arg.Value;
foundAttribute = true;
break;
}
}
}
}

if (foundAttribute)
{
break;
}
}
}
}
}
91 changes: 66 additions & 25 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7668,6 +7668,37 @@ static void getMethodInfoHelper(
&methInfo->locals);
} // getMethodInfoHelper


void CEEInfo::getTransientMethodInfo(MethodDesc* pMD, CORINFO_METHOD_INFO* methInfo)
{
MethodInfoHelperContext cxt{ pMD };

// We will either find or create transient method details.
_ASSERTE(!cxt.HasTransientMethodDetails());

// IL methods with no RVA indicate there is no implementation defined in metadata.
// Check if we previously generated details/implementation for this method.
TransientMethodDetails* detailsMaybe = NULL;
if (FindTransientMethodDetails(pMD, &detailsMaybe))
cxt.UpdateWith(*detailsMaybe);

getMethodInfoHelper(cxt, methInfo);

// If we have transient method details we need to handle
// the lifetime of the details.
if (cxt.HasTransientMethodDetails())
{
// If we didn't find transient details, but we have them
// after getting method info, store them for cleanup.
if (detailsMaybe == NULL)
AddTransientMethodDetails(cxt.CreateTransientMethodDetails());

// Reset the context because ownership has either been
// transferred or was found in this instance.
cxt.UpdateWith({});
}
}

//---------------------------------------------------------------------------------------
//
bool
Expand Down Expand Up @@ -7704,31 +7735,7 @@ CEEInfo::getMethodInfo(
}
else if (ftn->IsIL() && ftn->GetRVA() == 0)
{
// We will either find or create transient method details.
_ASSERTE(!cxt.HasTransientMethodDetails());

// IL methods with no RVA indicate there is no implementation defined in metadata.
// Check if we previously generated details/implementation for this method.
TransientMethodDetails* detailsMaybe = NULL;
if (FindTransientMethodDetails(ftn, &detailsMaybe))
cxt.UpdateWith(*detailsMaybe);

getMethodInfoHelper(cxt, methInfo);

// If we have transient method details we need to handle
// the lifetime of the details.
if (cxt.HasTransientMethodDetails())
{
// If we didn't find transient details, but we have them
// after getting method info, store them for cleanup.
if (detailsMaybe == NULL)
AddTransientMethodDetails(cxt.CreateTransientMethodDetails());

// Reset the context because ownership has either been
// transferred or was found in this instance.
cxt.UpdateWith({});
}

getTransientMethodInfo(ftn, methInfo);
result = true;
}

Expand Down Expand Up @@ -7890,6 +7897,40 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller,
}
}

// If the root level caller and callee modules do not have the same runtime
// wrapped exception behavior, and the callee has EH, we cannot inline.
_ASSERTE(!pCallee->IsDynamicMethod());
{
Module* pCalleeModule = pCallee->GetModule();
Module* pRootModule = pOrigCaller->GetModule();

if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions())
{
if (pCallee->HasILHeader())
{
COR_ILMETHOD_DECODER header(pCallee->GetILHeader(), pCallee->GetMDImport(), NULL);
if (header.EHCount() > 0)
{
result = INLINE_FAIL;
szFailReason = "Inlinee and root method have different wrapped exception behavior";
goto exit;
}
}
else if (pCallee->IsIL() && pCallee->GetRVA() == 0)
{
CORINFO_METHOD_INFO methodInfo;
getTransientMethodInfo(pCallee, &methodInfo);

if (methodInfo.EHcount > 0)
{
result = INLINE_FAIL;
szFailReason = "Inlinee and root method have different wrapped exception behavior";
goto exit;
}
}
}
}

#ifdef PROFILING_SUPPORTED
if (pOrigCallerModule->IsInliningDisabledByProfiler())
{
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,9 @@ class CEEInfo : public ICorJitInfo
TransientMethodDetails RemoveTransientMethodDetails(MethodDesc* pMD);
bool FindTransientMethodDetails(MethodDesc* pMD, TransientMethodDetails** details);

// Get method info for a transient method
void getTransientMethodInfo(MethodDesc* pMD, CORINFO_METHOD_INFO* methInfo);

protected:
SArray<OBJECTHANDLE>* m_pJitHandles; // GC handles used by JIT
MethodDesc* m_pMethodBeingCompiled; // Top-level method being compiled
Expand Down
2 changes: 1 addition & 1 deletion src/tests/JIT/Directed/throwbox/filter.il
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
.class public auto ansi beforefieldinit Test
extends [mscorlib]System.Object
{
.method public hidebysig static int32 Main() cil managed
.method public hidebysig static int32 Main() cil managed aggressiveinlining
{
.custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = (
01 00 00 00
Expand Down
Loading