Skip to content

NativeAOT-LLVM: Fix order of catch and second pass + calling finally twice #1991

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.

Sign up for GitHub

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 5 commits into from
Sep 14, 2022
Merged

NativeAOT-LLVM: Fix order of catch and second pass + calling finally twice #1991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4ec8c7e
fix order of catch and second pass. Fix for a calling the finally tw…
yowl Sep 14, 2022
f6222a6
bump the mac pool to 11
yowl Sep 14, 2022
ef3692e
delete deprecated function
yowl Sep 14, 2022
7525c0d
Apply https://github.com/dotnet/runtime/pull/75294/files manually...
yowl Sep 14, 2022
22e29ce
remove useless assignment
yowl Sep 14, 2022
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
2 changes: 1 addition & 1 deletion eng/pipelines/common/xplat-setup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ jobs:

# OSX Build Pool (we don't have on-prem OSX BuildPool
${{ if in(parameters.osGroup, 'OSX', 'MacCatalyst', 'iOS', 'iOSSimulator', 'tvOS', 'tvOSSimulator') }}:
vmImage: 'macOS-10.15'
vmImage: 'macOS-11'

# Official Build Windows Pool
${{ if and(or(eq(parameters.osGroup, 'windows'), eq(parameters.hostedOs, 'windows')), ne(variables['System.TeamProject'], 'public')) }}:
Expand Down
29 changes: 5 additions & 24 deletions src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.wasm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ public bool ContainsCodeOffset(uint idxTryLandingStart)
}
else
{
tryRegionIdx = 0;
bool shouldInvokeHandler = InternalCalls.RhpCallFilterFunclet(exception, ehClause._filterAddress, shadowStack);
if (shouldInvokeHandler)
{
Expand All @@ -106,40 +105,22 @@ public bool ContainsCodeOffset(uint idxTryLandingStart)
return false;
}

private static void InvokeSecondPassWasm(uint idxStart, uint idxTryLandingStart, ref EHClauseIterator clauseIter, uint idxLimit, void* shadowStack)
private static void InvokeSecondPassWasm(uint idxStart, uint idxTryLandingStart /* we do dont have the PC, so use the start of the block */, ref EHClauseIterator clauseIter, uint idxLimit, void* shadowStack)
{
uint lastTryStart = 0, lastTryEnd = 0;
// Search the clauses for one that contains the current offset.
RhEHClauseWasm ehClause = new RhEHClauseWasm();
for (uint curIdx = 0; clauseIter.Next(ref ehClause) && curIdx < idxLimit; curIdx++)
{
//
// Skip to the starting try region. This is used by collided unwinds and rethrows to pickup where
// the previous dispatch left off.
//
if (idxStart != MaxTryRegionIdx)
{
if (curIdx <= idxStart)
{
lastTryStart = ehClause._tryStartOffset;
lastTryEnd = ehClause._tryEndOffset;
continue;
}

// Now, we continue skipping while the try region is identical to the one that invoked the
// previous dispatch.
if ((ehClause._tryStartOffset == lastTryStart) && (ehClause._tryEndOffset == lastTryEnd))
continue;

// We are done skipping. This is required to handle empty finally block markers that are used
// to separate runs of different try blocks with same native code offsets.
idxStart = MaxTryRegionIdx;
if (curIdx > idxStart)
{
break; // these blocks are after the catch
}

EHClauseIterator.RhEHClauseKindWasm clauseKind = ehClause._clauseKind;

if ((clauseKind != EHClauseIterator.RhEHClauseKindWasm.RH_EH_CLAUSE_FAULT)
|| !ehClause.TryStartsAt(idxTryLandingStart))
|| !ehClause.ContainsCodeOffset(idxTryLandingStart))
{
continue;
}
Expand Down
115 changes: 60 additions & 55 deletions src/coreclr/tools/aot/ILCompiler.LLVM/CodeGen/ILToLLVMImporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public enum LocalVarKind
private class ExceptionRegion
{
public ILExceptionRegion ILRegion;
public bool Marked;
}

DependencyNodeCore<NodeFactory>.DependencyList _dependencies = new DependencyNodeCore<NodeFactory>.DependencyList();
Expand Down Expand Up @@ -104,6 +105,7 @@ public enum ImportState : byte
}

private ExceptionRegion[] _exceptionRegions;
private ExceptionRegion[] _handlerRegionsForOffsetLookup;
public ILImporter(LLVMCodegenCompilation compilation, MethodDesc method, MethodIL methodIL, string mangledName, bool isUnboxingThunk)
{
Module = LLVMCodegenCompilation.Module;
Expand Down Expand Up @@ -136,6 +138,7 @@ public ILImporter(LLVMCodegenCompilation compilation, MethodDesc method, MethodI
_thisType = method.OwningType;
var ilExceptionRegions = methodIL.GetExceptionRegions();
_exceptionRegions = new ExceptionRegion[ilExceptionRegions.Length];
_handlerRegionsForOffsetLookup = new ExceptionRegion[ilExceptionRegions.Length];
if (ilExceptionRegions.Length != 0)
{
_exceptionFunclets = new List<LLVMValueRef>(_exceptionRegions.Length);
Expand All @@ -147,12 +150,20 @@ public ILImporter(LLVMCodegenCompilation compilation, MethodDesc method, MethodI
.ThenByDescending(region => region.TryLength) // outer regions with the same try offset as inner region first - they will have longer lengths, // WASMTODO, except maybe an inner of try {} catch {} which could still be a problem
.ThenBy(region => region.HandlerOffset))
{
_exceptionRegions[curRegion++] = new ExceptionRegion
_handlerRegionsForOffsetLookup[curRegion++] = new ExceptionRegion
{
ILRegion = region
};
}

for(curRegion = 0; curRegion < ilExceptionRegions.Length; curRegion++)
{
_exceptionRegions[curRegion] = new ExceptionRegion
{
ILRegion = ilExceptionRegions[curRegion]
};
}

_llvmFunction = GetOrCreateLLVMFunction(mangledName, method.Signature, method.RequiresInstArg());
_currentFunclet = _llvmFunction;
_pointerSize = compilation.NodeFactory.Target.PointerSize;
Expand Down Expand Up @@ -569,12 +580,12 @@ private ExceptionRegion GetCurrentTryRegion()
private ExceptionRegion GetTryRegion(int offset)
{
// Iterate backwards to find the most nested region
for (int i = _exceptionRegions.Length - 1; i >= 0; i--)
for (int i = _handlerRegionsForOffsetLookup.Length - 1; i >= 0; i--)
{
ILExceptionRegion region = _exceptionRegions[i].ILRegion;
ILExceptionRegion region = _handlerRegionsForOffsetLookup[i].ILRegion;
if (IsOffsetContained(offset - 1, region.TryOffset, region.TryLength))
{
return _exceptionRegions[i];
return _handlerRegionsForOffsetLookup[i];
}
}

Expand All @@ -588,13 +599,13 @@ private ExceptionRegion GetTryRegion(int offset)
private ExceptionRegion GetHandlerRegion(int offset)
{
// Iterate backwards to find the most nested region
for (int i = _exceptionRegions.Length - 1; i >= 0; i--)
for (int i = _handlerRegionsForOffsetLookup.Length - 1; i >= 0; i--)
{
ExceptionRegion exceptionRegion = _exceptionRegions[i];
ExceptionRegion exceptionRegion = _handlerRegionsForOffsetLookup[i];
if (IsOffsetContained(offset, exceptionRegion.ILRegion.HandlerOffset, exceptionRegion.ILRegion.HandlerLength) ||
(exceptionRegion.ILRegion.Kind == ILExceptionRegionKind.Filter && IsOffsetContained(offset, exceptionRegion.ILRegion.FilterOffset, exceptionRegion.ILRegion.HandlerOffset - exceptionRegion.ILRegion.FilterOffset)))
{
return _exceptionRegions[i];
return _handlerRegionsForOffsetLookup[i];
}
}

Expand Down Expand Up @@ -640,7 +651,7 @@ private void StartImportingBasicBlock(BasicBlock basicBlock)
if (basicBlock.HandlerStart || basicBlock.FilterStart)
{
_funcletAddrCacheCtx = null;
foreach (ExceptionRegion ehRegion in _exceptionRegions)
foreach (ExceptionRegion ehRegion in _handlerRegionsForOffsetLookup)
{
if (ehRegion.ILRegion.HandlerOffset == basicBlock.StartOffset ||
ehRegion.ILRegion.FilterOffset == basicBlock.StartOffset)
Expand All @@ -657,7 +668,7 @@ private void StartImportingBasicBlock(BasicBlock basicBlock)

if (basicBlock.TryStart)
{
foreach (ExceptionRegion ehRegion in _exceptionRegions)
foreach (ExceptionRegion ehRegion in _handlerRegionsForOffsetLookup)
{
if(ehRegion.ILRegion.TryOffset == basicBlock.StartOffset)
{
Expand Down Expand Up @@ -2960,12 +2971,26 @@ private LLVMBasicBlockRef GetOrCreateLandingPad(ExceptionRegion tryRegion)

var leaveDestination = landingPadBuilder.BuildAlloca(LLVMTypeRef.Int32, "leaveDest"); // create a variable to store the operand of the leave as we can't use the result of the call directly due to domination/branches
landingPadBuilder.BuildStore(LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, 0, false), leaveDestination);

// second pass -
// TODO-LLVM: note that we will call the finally blocks even if there is no catch, whereas really we should fail fast
// reinitialise the iterator
CallRuntime(_compilation.TypeSystemContext, "EHClauseIterator", "InitFromEhInfo", iteratorInitArgs, null, fromLandingPad: true, builder: landingPadBuilder);

var secondPassArgs = new StackEntry[] { new ExpressionEntry(StackValueKind.Int32, "idxStart", landingPadBuilder.BuildLoad(tryRegionIdx)),
new ExpressionEntry(StackValueKind.Int32, "idxTryLandingStart", LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, (ulong)tryRegion.ILRegion.TryOffset, false)),
new ExpressionEntry(StackValueKind.ByRef, "refFrameIter", ehInfoIterator),
new ExpressionEntry(StackValueKind.Int32, "idxLimit", LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, 0xFFFFFFFFu, false)),
new ExpressionEntry(StackValueKind.NativeInt, "shadowStack", _currentFunclet.GetParam(0))
};
CallRuntime(_compilation.TypeSystemContext, "EH", "InvokeSecondPassWasm", secondPassArgs, null, true, builder: landingPadBuilder);

var foundCatchBlock = _currentFunclet.AppendBasicBlock("LPFoundCatch");
// If it didn't find a catch block, we can rethrow (resume in LLVM) the C++ exception to continue the stack walk.
var noCatch = landingPadBuilder.BuildICmp(LLVMIntPredicate.LLVMIntEQ, LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, 0, false),
handler.ValueAsInt32(landingPadBuilder, false), "testCatch");
var secondPassBlock = _currentFunclet.AppendBasicBlock("SecondPass");
landingPadBuilder.BuildCondBr(noCatch, secondPassBlock, foundCatchBlock);
var resumeBlock = GetOrCreateResumeBlock(pad, tryRegion.ILRegion.TryOffset.ToString());
landingPadBuilder.BuildCondBr(noCatch, resumeBlock, foundCatchBlock);

landingPadBuilder.PositionAtEnd(foundCatchBlock);
// finished with the c++ exception
Expand All @@ -2981,23 +3006,9 @@ private LLVMBasicBlockRef GetOrCreateLandingPad(ExceptionRegion tryRegion)
LLVMValueRef leaveReturnValue = landingPadBuilder.BuildCall(RhpCallCatchFunclet, callCatchArgs, "");

landingPadBuilder.BuildStore(leaveReturnValue, leaveDestination);
landingPadBuilder.BuildBr(secondPassBlock);

landingPadBuilder.PositionAtEnd(secondPassBlock);

// reinitialise the iterator
CallRuntime(_compilation.TypeSystemContext, "EHClauseIterator", "InitFromEhInfo", iteratorInitArgs, null, fromLandingPad: true, builder: landingPadBuilder);

var secondPassArgs = new StackEntry[] { new ExpressionEntry(StackValueKind.Int32, "idxStart", LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, 0xFFFFFFFFu, false)),
new ExpressionEntry(StackValueKind.Int32, "idxTryLandingStart", LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, (ulong)tryRegion.ILRegion.TryOffset, false)),
new ExpressionEntry(StackValueKind.ByRef, "refFrameIter", ehInfoIterator),
new ExpressionEntry(StackValueKind.Int32, "idxLimit", LLVMValueRef.CreateConstInt(LLVMTypeRef.Int32, 0xFFFFFFFFu, false)),
new ExpressionEntry(StackValueKind.NativeInt, "shadowStack", _currentFunclet.GetParam(0))
};
CallRuntime(_compilation.TypeSystemContext, "EH", "InvokeSecondPassWasm", secondPassArgs, null, true, builder: landingPadBuilder);

var catchLeaveBlock = _currentFunclet.AppendBasicBlock("CatchLeave");
landingPadBuilder.BuildCondBr(noCatch, GetOrCreateResumeBlock(pad, tryRegion.ILRegion.TryOffset.ToString()), catchLeaveBlock);
landingPadBuilder.BuildCondBr(noCatch, resumeBlock, catchLeaveBlock);
landingPadBuilder.PositionAtEnd(catchLeaveBlock);

// Use the else as the path for no exception handler found for this exception
Expand Down Expand Up @@ -4435,9 +4446,9 @@ void ThrowOrRethrow(StackEntry exceptionObject)
_builder.BuildInvoke(RhpThrowEx, args, GetOrCreateUnreachableBlock(), GetOrCreateLandingPad(currentExceptionRegion), "");
}

for (int i = 0; i < _exceptionRegions.Length; i++)
for (int i = 0; i < _handlerRegionsForOffsetLookup.Length; i++)
{
var r = _exceptionRegions[i];
var r = _handlerRegionsForOffsetLookup[i];

if (IsOffsetContained(_currentOffset - 1, r.ILRegion.TryOffset, r.ILRegion.TryLength))
{
Expand Down Expand Up @@ -4943,9 +4954,9 @@ private void ImportBox(int token)

private void ImportLeave(BasicBlock target)
{
for (int i = _exceptionRegions.Length - 1; i >= 0; i--)
for (int i = _handlerRegionsForOffsetLookup.Length - 1; i >= 0; i--)
{
var r = _exceptionRegions[i];
var r = _handlerRegionsForOffsetLookup[i];

if (r.ILRegion.Kind == ILExceptionRegionKind.Finally &&
IsOffsetContained(_currentOffset - 1, r.ILRegion.TryOffset, r.ILRegion.TryLength) &&
Expand Down Expand Up @@ -5382,35 +5393,21 @@ private ObjectNode.ObjectData EncodeEHInfo()
builder.RequireInitialAlignment(1);
int totalClauses = _exceptionRegions.Length;

// Count the number of special markers that will be needed
// for (int i = 1; i < _exceptionRegions.Length; i++)
// {
// ExceptionRegion clause = _exceptionRegions[i];
// ExceptionRegion previousClause = _exceptionRegions[i - 1];

// WASMTODO : do we need these special markers and if so how do we detect and set CORINFO_EH_CLAUSE_SAMETRY?
// if ((previousClause.ILRegion.TryOffset == clause.ILRegion.TryOffset) &&
// (previousClause.ILRegion.TryLength == clause.ILRegion.TryLength) &&
// ((clause.Flags & CORINFO_EH_CLAUSE_FLAGS.CORINFO_EH_CLAUSE_SAMETRY) == 0))
// {
// totalClauses++;
// }
// }

builder.EmitCompressedUInt((uint)totalClauses);
// Iterate backwards to emit the innermost first, but within a try region go forwards to get the first matching catch type
int i = _exceptionRegions.Length - 1;
while (i >= 0)

// Iterate forwards to emit the innermost first (as they appear in the IL), but within a try region go forwards to get the first matching catch type
for(int i = 0; i < _exceptionRegions.Length; i++)
{
int tryStart = _exceptionRegions[i].ILRegion.TryOffset;
int tryLength = _exceptionRegions[i].ILRegion.TryLength;
ExceptionRegion tryRegion = _exceptionRegions[i];

if (tryRegion.Marked) continue;

int tryStart = tryRegion.ILRegion.TryOffset;
int tryLength = tryRegion.ILRegion.TryLength;
for (var j = 0; j < _exceptionRegions.Length; j++)
{
ExceptionRegion exceptionRegion = _exceptionRegions[j];
if (exceptionRegion.ILRegion.TryOffset != tryStart || exceptionRegion.ILRegion.TryLength != tryLength) continue;
// if (i > 0)
// {
// ExceptionRegion previousClause = _exceptionRegions[i - 1];

// If the previous clause has same try offset and length as the current clause,
// but belongs to a different try block (CORINFO_EH_CLAUSE_SAMETRY is not set),
Expand Down Expand Up @@ -5476,11 +5473,19 @@ private ObjectNode.ObjectData EncodeEHInfo()
builder.EmitReloc(new LLVMBlockRefNode(filterFuncletName), rel);
break;
}
i--;

exceptionRegion.Marked = true;
}
}

return builder.ToObjectData();
#if DEBUG
for (int i = 0; i < _exceptionRegions.Length; i++)
{
Debug.Assert(_exceptionRegions[i].Marked);
}
#endif

return builder.ToObjectData();
}

private string GetFuncletName(ExceptionRegion exceptionRegion, int regionOffset, ILExceptionRegionKind ilExceptionRegionKind)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/aot/ILCompiler.LLVM/CodeGen/LLVMObjectWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1364,6 +1364,7 @@ public ILImporter(LLVMBuilderRef builder, LLVMCodegenCompilation compilation, LL
_thisType = GetWellKnownType(WellKnownType.Void);
_pointerSize = compilation.NodeFactory.Target.PointerSize;
_exceptionRegions = new ExceptionRegion[0];
_handlerRegionsForOffsetLookup = new ExceptionRegion[0];
}

internal void OutputCodeForTriggerCctor(TypeDesc type, LLVMValueRef staticBaseValueRef)
Expand Down
5 changes: 5 additions & 0 deletions src/libraries/Native/Unix/System.Globalization.Native/configure.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ else()
"unicode/ucol.h"
HAVE_SET_MAX_VARIABLE)

check_symbol_exists(
ucol_clone
"unicode/ucol.h"
HAVE_UCOL_CLONE)

unset(CMAKE_REQUIRED_LIBRARIES)
unset(CMAKE_REQUIRED_INCLUDES)
endif()
Expand Down
17 changes: 17 additions & 0 deletions src/libraries/Native/Unix/System.Globalization.Native/pal_collation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,24 @@ static UCollator* CloneCollatorWithOptions(const UCollator* pCollator, int32_t o

if (customRuleLength == 0)
{
#if !defined(STATIC_ICU)
if (ucol_clone_ptr != NULL)
{
pClonedCollator = ucol_clone(pCollator, pErr);
}
else
{
pClonedCollator = ucol_safeClone_ptr(pCollator, NULL, NULL, pErr);
}
#else // !defined(STATIC_ICU)

#if HAVE_UCOL_CLONE
pClonedCollator = ucol_clone(pCollator, pErr);
#else
pClonedCollator = ucol_safeClone(pCollator, NULL, NULL, pErr);
#endif // HAVE_UCOL_CLONE

#endif // !defined(STATIC_ICU)
}
else
{
Expand Down
Loading