Skip to content

Commit 982912a

Browse files
authored
Block inlines with different exception wrapping behavior (#113849)
Fixes #113815
1 parent 8f8dad0 commit 982912a

File tree

5 files changed

+140
-26
lines changed

5 files changed

+140
-26
lines changed

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,6 +1258,23 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO
12581258
MethodDesc callerMethod = HandleToObject(callerHnd);
12591259
MethodDesc calleeMethod = HandleToObject(calleeHnd);
12601260

1261+
EcmaModule rootModule = (MethodBeingCompiled.OwningType as MetadataType)?.Module as EcmaModule;
1262+
EcmaModule calleeModule = (calleeMethod.OwningType as MetadataType)?.Module as EcmaModule;
1263+
1264+
// If this inline crosses module boundaries, ensure the modules agree on exception wrapping behavior.
1265+
if ((rootModule != calleeModule) && (rootModule != null) && (calleeModule != null))
1266+
{
1267+
if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows)
1268+
{
1269+
var calleeIL = _compilation.GetMethodIL(calleeMethod);
1270+
if (calleeIL.GetExceptionRegions().Length != 0)
1271+
{
1272+
// Fail inlining if root method and callee have different exception wrapping behavior
1273+
return CorInfoInline.INLINE_FAIL;
1274+
}
1275+
}
1276+
}
1277+
12611278
if (_compilation.CanInline(callerMethod, calleeMethod))
12621279
{
12631280
// No restrictions on inlining
@@ -1266,6 +1283,10 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO
12661283
else
12671284
{
12681285
// Call may not be inlined
1286+
//
1287+
// Note despite returning INLINE_NEVER here, in compilations where jitting is possible
1288+
// the jit may still be able to inline this method. So we rely on reportInliningDecision
1289+
// to not propagate this into metadata to short-circuit future inlining attempts.
12691290
return CorInfoInline.INLINE_NEVER;
12701291
}
12711292
}

src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ public partial class EcmaModule : ModuleDesc
1616
{
1717
private readonly PEReader _peReader;
1818
protected readonly MetadataReader _metadataReader;
19+
private volatile bool _isWrapNonExceptionThrowsComputed;
20+
private volatile bool _isWrapNonExceptionThrows;
1921

2022
internal interface IEntityHandleObject
2123
{
@@ -690,5 +692,52 @@ public override string ToString()
690692
ModuleDefinition moduleDefinition = _metadataReader.GetModuleDefinition();
691693
return _metadataReader.GetString(moduleDefinition.Name);
692694
}
695+
696+
public bool IsWrapNonExceptionThrows
697+
{
698+
get
699+
{
700+
if (!_isWrapNonExceptionThrowsComputed)
701+
{
702+
ComputeIsWrapNonExceptionThrows();
703+
_isWrapNonExceptionThrowsComputed = true;
704+
}
705+
return _isWrapNonExceptionThrows;
706+
}
707+
}
708+
709+
private void ComputeIsWrapNonExceptionThrows()
710+
{
711+
var reader = MetadataReader;
712+
var c = reader.StringComparer;
713+
bool foundAttribute = false;
714+
foreach (var attr in reader.GetAssemblyDefinition().GetCustomAttributes())
715+
{
716+
if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n))
717+
{
718+
if (c.Equals(ns, "System.Runtime.CompilerServices") && c.Equals(n, "RuntimeCompatibilityAttribute"))
719+
{
720+
var dec = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider(this));
721+
722+
foreach (var arg in dec.NamedArguments)
723+
{
724+
if (arg.Name == "WrapNonExceptionThrows")
725+
{
726+
if (!(arg.Value is bool))
727+
ThrowHelper.ThrowBadImageFormatException();
728+
_isWrapNonExceptionThrows = (bool)arg.Value;
729+
foundAttribute = true;
730+
break;
731+
}
732+
}
733+
}
734+
}
735+
736+
if (foundAttribute)
737+
{
738+
break;
739+
}
740+
}
741+
}
693742
}
694743
}

src/coreclr/vm/jitinterface.cpp

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7668,6 +7668,37 @@ static void getMethodInfoHelper(
76687668
&methInfo->locals);
76697669
} // getMethodInfoHelper
76707670

7671+
7672+
void CEEInfo::getTransientMethodInfo(MethodDesc* pMD, CORINFO_METHOD_INFO* methInfo)
7673+
{
7674+
MethodInfoHelperContext cxt{ pMD };
7675+
7676+
// We will either find or create transient method details.
7677+
_ASSERTE(!cxt.HasTransientMethodDetails());
7678+
7679+
// IL methods with no RVA indicate there is no implementation defined in metadata.
7680+
// Check if we previously generated details/implementation for this method.
7681+
TransientMethodDetails* detailsMaybe = NULL;
7682+
if (FindTransientMethodDetails(pMD, &detailsMaybe))
7683+
cxt.UpdateWith(*detailsMaybe);
7684+
7685+
getMethodInfoHelper(cxt, methInfo);
7686+
7687+
// If we have transient method details we need to handle
7688+
// the lifetime of the details.
7689+
if (cxt.HasTransientMethodDetails())
7690+
{
7691+
// If we didn't find transient details, but we have them
7692+
// after getting method info, store them for cleanup.
7693+
if (detailsMaybe == NULL)
7694+
AddTransientMethodDetails(cxt.CreateTransientMethodDetails());
7695+
7696+
// Reset the context because ownership has either been
7697+
// transferred or was found in this instance.
7698+
cxt.UpdateWith({});
7699+
}
7700+
}
7701+
76717702
//---------------------------------------------------------------------------------------
76727703
//
76737704
bool
@@ -7704,31 +7735,7 @@ CEEInfo::getMethodInfo(
77047735
}
77057736
else if (ftn->IsIL() && ftn->GetRVA() == 0)
77067737
{
7707-
// We will either find or create transient method details.
7708-
_ASSERTE(!cxt.HasTransientMethodDetails());
7709-
7710-
// IL methods with no RVA indicate there is no implementation defined in metadata.
7711-
// Check if we previously generated details/implementation for this method.
7712-
TransientMethodDetails* detailsMaybe = NULL;
7713-
if (FindTransientMethodDetails(ftn, &detailsMaybe))
7714-
cxt.UpdateWith(*detailsMaybe);
7715-
7716-
getMethodInfoHelper(cxt, methInfo);
7717-
7718-
// If we have transient method details we need to handle
7719-
// the lifetime of the details.
7720-
if (cxt.HasTransientMethodDetails())
7721-
{
7722-
// If we didn't find transient details, but we have them
7723-
// after getting method info, store them for cleanup.
7724-
if (detailsMaybe == NULL)
7725-
AddTransientMethodDetails(cxt.CreateTransientMethodDetails());
7726-
7727-
// Reset the context because ownership has either been
7728-
// transferred or was found in this instance.
7729-
cxt.UpdateWith({});
7730-
}
7731-
7738+
getTransientMethodInfo(ftn, methInfo);
77327739
result = true;
77337740
}
77347741

@@ -7890,6 +7897,40 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller,
78907897
}
78917898
}
78927899

7900+
// If the root level caller and callee modules do not have the same runtime
7901+
// wrapped exception behavior, and the callee has EH, we cannot inline.
7902+
_ASSERTE(!pCallee->IsDynamicMethod());
7903+
{
7904+
Module* pCalleeModule = pCallee->GetModule();
7905+
Module* pRootModule = pOrigCaller->GetModule();
7906+
7907+
if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions())
7908+
{
7909+
if (pCallee->HasILHeader())
7910+
{
7911+
COR_ILMETHOD_DECODER header(pCallee->GetILHeader(), pCallee->GetMDImport(), NULL);
7912+
if (header.EHCount() > 0)
7913+
{
7914+
result = INLINE_FAIL;
7915+
szFailReason = "Inlinee and root method have different wrapped exception behavior";
7916+
goto exit;
7917+
}
7918+
}
7919+
else if (pCallee->IsIL() && pCallee->GetRVA() == 0)
7920+
{
7921+
CORINFO_METHOD_INFO methodInfo;
7922+
getTransientMethodInfo(pCallee, &methodInfo);
7923+
7924+
if (methodInfo.EHcount > 0)
7925+
{
7926+
result = INLINE_FAIL;
7927+
szFailReason = "Inlinee and root method have different wrapped exception behavior";
7928+
goto exit;
7929+
}
7930+
}
7931+
}
7932+
}
7933+
78937934
#ifdef PROFILING_SUPPORTED
78947935
if (pOrigCallerModule->IsInliningDisabledByProfiler())
78957936
{

src/coreclr/vm/jitinterface.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,9 @@ class CEEInfo : public ICorJitInfo
472472
TransientMethodDetails RemoveTransientMethodDetails(MethodDesc* pMD);
473473
bool FindTransientMethodDetails(MethodDesc* pMD, TransientMethodDetails** details);
474474

475+
// Get method info for a transient method
476+
void getTransientMethodInfo(MethodDesc* pMD, CORINFO_METHOD_INFO* methInfo);
477+
475478
protected:
476479
SArray<OBJECTHANDLE>* m_pJitHandles; // GC handles used by JIT
477480
MethodDesc* m_pMethodBeingCompiled; // Top-level method being compiled

src/tests/JIT/Directed/throwbox/filter.il

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
.class public auto ansi beforefieldinit Test
1515
extends [mscorlib]System.Object
1616
{
17-
.method public hidebysig static int32 Main() cil managed
17+
.method public hidebysig static int32 Main() cil managed aggressiveinlining
1818
{
1919
.custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = (
2020
01 00 00 00

0 commit comments

Comments
 (0)