Skip to content
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

[NativeAOT-LLVM] Fix EH with table-disjoint mutually protecting handlers #2652

Merged
merged 2 commits into from
Aug 12, 2024
Merged

[NativeAOT-LLVM] Fix EH with table-disjoint mutually protecting handlers #2652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0627f8
Add a test
SingleAccretion Aug 11, 2024
4250f9e
Account for disjoint mutually protecting handlers
SingleAccretion Aug 11, 2024
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
65 changes: 46 additions & 19 deletions src/coreclr/jit/llvmcodegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,8 @@ void Llvm::generateUnwindBlocks()
// Generate the unwind blocks used to catch native exceptions during the second pass.
// We generate these before the rest of the code because throwing calls need a certain
// amount of pieces filled in (in particular, "catchswitch"es in the Wasm EH model).
m_EHRegionsInfo = new (_compiler->getAllocator(CMK_Codegen)) EHRegionInfo[_compiler->compHndBBtabCount]();
CompAllocator alloc = _compiler->getAllocator(CMK_Codegen);
m_EHRegionsInfo = new (alloc) EHRegionInfo[_compiler->compHndBBtabCount]();

struct FunctionData
{
Expand All @@ -380,10 +381,35 @@ void Llvm::generateUnwindBlocks()
});
}

// The main loop will need to map an outermost mutually protecting region to the innermost. While usually
// mutually protecting handlers form a contiguous 'run' in the table, this is not guaranteed, and disjoint
// regions, or regions nested inside the respective handlers can 'break up' the run.
Comment on lines +384 to +386
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the EH table from the test:

***************  Exception Handling table
index  eTry, eHnd
  0  ::   6        - Try at BB02..BB02 [000..00C), Handler at BB03..BB03 [00C..015)
  1  ::   4     6  - Try at BB05..BB05 [016..022), Handler at BB06..BB06 [022..02B)
  2  ::   3     4  - Try at BB08..BB08 [02C..038), Handler at BB09..BB09 [038..041)
  3  ::   8     4  - Try at BB08..BB10 [02C..043), Finally at BB11..BB11 [043..04F)
  4  ::   5     6  - Try at BB05..BB05 [016..022), Handler at BB07..BB12 [02B..051)
  5  ::   8     6  - Try at BB05..BB05 [016..022), Handler at BB13..BB13 [051..05A)
  6  ::   7        - Try at BB02..BB02 [000..00C), Handler at BB04..BB14 [015..05C)
  7  ::   8        - Try at BB02..BB02 [000..00C), Handler at BB15..BB15 [05C..065)
  8  ::            - Try at BB02..BB15 [000..065), Finally at BB16..BB16 [065..06C)

There are two sets of mutually protecting handlers: {H0, H6, H7} and {H1, H4, H5}.

jitstd::vector<unsigned> innermostMutuallyProtectingRegionMap(_compiler->compHndBBtabCount, -1, alloc);
for (unsigned ehIndex = 0; ehIndex < _compiler->compHndBBtabCount; ehIndex++)
{
EHblkDsc* ehDsc = _compiler->ehGetDsc(ehIndex);
if (!ehDsc->HasCatchHandler())
{
continue; // Of no interest.
}

unsigned innermostEHIndex = innermostMutuallyProtectingRegionMap[ehIndex];
if (innermostEHIndex == -1)
{
innermostEHIndex = ehIndex;
innermostMutuallyProtectingRegionMap[ehIndex] = innermostEHIndex;
}

unsigned outerEHIndex = ehDsc->ebdEnclosingTryIndex;
if ((outerEHIndex != EHblkDsc::NO_ENCLOSING_INDEX) && ehDsc->ebdIsSameTry(_compiler, outerEHIndex))
{
innermostMutuallyProtectingRegionMap[outerEHIndex] = innermostEHIndex;
}
}

// There is no meaningful source location we can attach to the unwind blocks. None of them are "user" code.
llvm::DILocation* unwindBlocksDebugLoc = getArtificialDebugLocation();
jitstd::vector<FunctionData> functionData(
_compiler->compFuncCount(), FunctionData{}, _compiler->getAllocator(CMK_Codegen));
jitstd::vector<FunctionData> functionData(_compiler->compFuncCount(), FunctionData{}, alloc);

// Note the iteration order: outer -> inner.
for (unsigned ehIndex = _compiler->compHndBBtabCount - 1; ehIndex != -1; ehIndex--)
Expand All @@ -394,6 +420,13 @@ void Llvm::generateUnwindBlocks()
continue;
}

EHRegionInfo* pEHRegionInfo = &getEHRegionInfo(ehIndex);
if (pEHRegionInfo->UnwindBlock != nullptr)
{
// Already emitted this part of the unwind sequence for mutually protecting handlers.
continue;
}

// The unwind block is part of the function with the protected region.
unsigned funcIdx = getLlvmFunctionIndexForProtectedRegion(ehIndex);
Function* llvmFunc = getLlvmFunctionForIndex(funcIdx);
Expand Down Expand Up @@ -552,23 +585,16 @@ void Llvm::generateUnwindBlocks()
_builder.CreateIntrinsic(llvm::Intrinsic::wasm_get_exception, {}, catchPadInst);
}

// Eagerly initialize the unwind block to emit calls below.
getEHRegionInfo(ehIndex).UnwindBlock = unwindLlvmBlock;
// Eagerly initialize the unwind block to emit the calls below.
pEHRegionInfo->UnwindBlock = unwindLlvmBlock;

if (ehDsc->HasCatchHandler())
{
// Find the full set of mutually protecting handlers we have. Since we are generating things outer-to-inner,
// we are guaranteed to capture them all here.
unsigned innerEHIndex = ehIndex;
while ((innerEHIndex > 0) && ehDsc->ebdIsSameTry(_compiler, innerEHIndex - 1))
// Generate the full set of mutually protecting handlers we have, inner (first) to outer (last).
unsigned hndEHIndex = innermostMutuallyProtectingRegionMap[ehIndex];
while (hndEHIndex <= ehIndex)
{
innerEHIndex--;
getEHRegionInfo(innerEHIndex).UnwindBlock = unwindLlvmBlock;
}

for (unsigned hndEHIndex = innerEHIndex; hndEHIndex <= ehIndex; hndEHIndex++)
{
EHblkDsc* hndDsc = _compiler->ehGetDsc(hndEHIndex);
EHRegionInfo* pHndRegionInfo = &getEHRegionInfo(hndEHIndex);

// Call the runtime to determine whether this catch should handle the exception. Note how we must do so
// even if we know the catch handler is statically unreachable. This is both because the runtime assumes
Expand All @@ -579,7 +605,7 @@ void Llvm::generateUnwindBlocks()
unsigned hndUnwindIndex = (m_unwindIndexMap != nullptr) ? m_unwindIndexMap->Bottom(hndEHIndex)
: UNWIND_INDEX_NOT_IN_TRY;
Value* caughtValue = emitHelperCall(CORINFO_HELP_LLVM_EH_CATCH, getIntPtrConst(hndUnwindIndex));
getEHRegionInfo(hndEHIndex).CatchArgValue = caughtValue;
pHndRegionInfo->CatchArgValue = caughtValue;

// Yes if we get not-"null" back, otherwise continue unwinding.
Value* callCatchValue = _builder.CreateIsNotNull(caughtValue);
Expand All @@ -596,9 +622,11 @@ void Llvm::generateUnwindBlocks()
}
else
{
pHndRegionInfo->UnwindBlock = unwindLlvmBlock;
continueUnwindLlvmBlock = createInlineLlvmBlock();
}

EHblkDsc* hndDsc = _compiler->ehGetDsc(hndEHIndex);
if (isReachable(hndDsc->ebdHndBeg))
{
llvm::BasicBlock* catchLlvmBlock = getFirstLlvmBlockForBlock(hndDsc->ebdHndBeg);
Expand All @@ -610,9 +638,8 @@ void Llvm::generateUnwindBlocks()
}

_builder.SetInsertPoint(continueUnwindLlvmBlock);
hndEHIndex = hndDsc->ebdEnclosingTryIndex;
}

ehIndex = innerEHIndex;
}
else if (ehDsc->HasFinallyHandler())
{
Expand Down
62 changes: 62 additions & 0 deletions src/tests/nativeaot/SmokeTests/HelloWasm/ExceptionHandlingTests.Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ private static bool TestTryCatch()

TestThrowInMutuallyProtectingHandlers();

TestDisjointMutuallyProtectingHandlers();

TestExceptionInGvmCall();

TestCatchHandlerNeedsGenericContext();
Expand Down Expand Up @@ -427,6 +429,66 @@ private static void TestThrowInMutuallyProtectingHandlers()
PassTest();
}

private static void TestDisjointMutuallyProtectingHandlers()
{
int index = 0;
bool result = true;
[MethodImpl(MethodImplOptions.NoInlining)] void At(int expected) => result &= index++ == expected;

StartTest("TestDisjointMutuallyProtectingHandlers");
try
{
At(0);
throw new InvalidOperationException();
}
catch (NotSupportedException)
{
At(-1);
}
catch (InvalidOperationException)
{
try
{
At(1);
throw new NullReferenceException();
}
catch (NotSupportedException)
{
At(-1);
}
catch (NullReferenceException)
{
try
{
At(2);
throw new Exception();
}
catch
{
At(3);
}
finally
{
At(4);
}
}
catch
{
At(-1);
}
}
catch (Exception)
{
At(-1);
}
finally
{
At(5);
}

EndTest(result);
}

private static void TestExceptionInGvmCall()
{
StartTest("TestExceptionInGvmCall");
Expand Down