Skip to content

Commit

Permalink
[NativeAOT/x86] Fix funclet unwinding (#100732)
Browse files Browse the repository at this point in the history
* Guard ABI changes necessary for funclet unwinding with UsesFunclets()/FEATURE_EH_FUNCLETS instead of UNIX_X86_ABI

* Fix the emitFullArgInfo condition.

* Update comments, add test case
  • Loading branch information
filipnavara committed Apr 8, 2024
1 parent 8508806 commit 8e2655b
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 21 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/gcdump/i386/gcdumpx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,10 @@ size_t GCDump::DumpGCTable(PTR_CBYTE table,
/* non-ptr arg push */

curOffs += (val & 0x07);
#ifndef UNIX_X86_ABI
// For x86/Linux, non-ptr arg pushes can be reported even for EBP frames
#ifndef FEATURE_EH_FUNCLETS
// For funclets, non-ptr arg pushes can be reported even for EBP frames
_ASSERTE(!header.ebpFrame);
#endif // UNIX_X86_ABI
#endif // FEATURE_EH_FUNCLETS
argCnt++;

DumpEncoding(bp, table-bp); bp = table;
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6680,11 +6680,11 @@ unsigned emitter::emitEndCodeGen(Compiler* comp,

emitFullyInt = fullyInt;
emitFullGCinfo = fullPtrMap;

#ifndef UNIX_X86_ABI
emitFullArgInfo = !emitHasFramePtr;
#if TARGET_X86
// On x86 with funclets we emit full ptr map even for EBP frames
emitFullArgInfo = comp->UsesFunclets() ? fullPtrMap : !emitHasFramePtr;
#else
emitFullArgInfo = fullPtrMap;
emitFullArgInfo = !emitHasFramePtr;
#endif

#if EMITTER_STATS
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/jit/gcencode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2584,9 +2584,7 @@ size_t GCInfo::gcMakeRegPtrTable(BYTE* dest, int mask, const InfoHdr& header, un

assert((codeDelta & 0x7) == codeDelta);
*dest++ = 0xB0 | (BYTE)codeDelta;
#ifndef UNIX_X86_ABI
assert(!compiler->isFramePointerUsed());
#endif
assert(compiler->UsesFunclets() || !compiler->isFramePointerUsed());

/* Remember the new 'last' offset */

Expand Down
16 changes: 7 additions & 9 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14236,6 +14236,13 @@ void Compiler::fgSetOptions()
if (info.compXcptnsCount > 0)
{
codeGen->setFramePointerRequiredEH(true);

if (UsesFunclets())
{
assert(!codeGen->isGCTypeFixed());
// Enforce fully interruptible codegen for funclet unwinding
SetInterruptible(true);
}
}

#else // !TARGET_X86
Expand All @@ -14247,15 +14254,6 @@ void Compiler::fgSetOptions()

#endif // TARGET_X86

#ifdef UNIX_X86_ABI
if (info.compXcptnsCount > 0)
{
assert(!codeGen->isGCTypeFixed());
// Enforce fully interruptible codegen for funclet unwinding
SetInterruptible(true);
}
#endif // UNIX_X86_ABI

if (compMethodRequiresPInvokeFrame())
{
codeGen->setFramePointerRequired(true); // Setup of Pinvoke frame currently requires an EBP style frame
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/gc_unwind_x86.inl
Original file line number Diff line number Diff line change
Expand Up @@ -1519,10 +1519,10 @@ unsigned scanArgRegTableI(PTR_CBYTE table,

bool hasPartialArgInfo;

#ifndef UNIX_X86_ABI
#ifndef FEATURE_EH_FUNCLETS
hasPartialArgInfo = info->ebpFrame;
#else
// For x86/Linux, interruptible code always has full arg info
// For funclets, interruptible code always has full arg info
//
// This should be aligned with emitFullArgInfo setting at
// emitter::emitEndCodeGen (in JIT)
Expand Down
25 changes: 25 additions & 0 deletions src/tests/nativeaot/SmokeTests/Exceptions/Exceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.ExceptionServices;
using System.Text;

Expand Down Expand Up @@ -160,6 +161,8 @@ public static int Main()

TestFirstChanceExceptionEvent();

TestUnwindInFunclet();

throw new Exception("UnhandledException");

return Fail;
Expand Down Expand Up @@ -300,5 +303,27 @@ static bool FilterWithGC()
CreateSomeGarbage();
return true;
}

static void TestUnwindInFunclet()
{
try
{
throw new Exception();
}
catch (Exception e)
{
// x86 pushes the call arguments on the stack and moves the stack pointer.
// We use a non-inlined call with enough parameters to force this to happen,
// so we can verify that the unwinder can walk through this funclet. The
// unwinding is triggered by the GC.Collect call.
MultiparameterCallWithGC(0, 1, 2, 3, 4);
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void MultiparameterCallWithGC(nint a, nint b, nint c, nint d, nint f)
{
GC.Collect();
}
}

0 comments on commit 8e2655b

Please sign in to comment.