[mono][llvm] Support nested EH clauses

[imhameed]

StoreNonTemporal_r.dll bails out of LLVM compilation of IntelHardwareIntrinsicTest:Main because of this:

runtime/src/mono/mono/mini/mini-llvm.c

Lines 10910 to 10929 in d95bfea

	/*
	* Nested clauses where one of the clauses is a finally clause is
	* not supported, because LLVM can't figure out the control flow,
	* probably because we resume exception handling by calling our
	* own function instead of using the 'resume' llvm instruction.
	*/
	for (i = 0; i < cfg->header->num_clauses; ++i) {
	for (j = 0; j < cfg->header->num_clauses; ++j) {
	MonoExceptionClause *clause1 = &cfg->header->clauses [i];
	MonoExceptionClause *clause2 = &cfg->header->clauses [j];
	// FIXME: Nested try clauses fail in some cases too, i.e. #37273
	if (i != j && clause1->try_offset >= clause2->try_offset && clause1->handler_offset <= clause2->handler_offset) {
	//(clause1->flags == MONO_EXCEPTION_CLAUSE_FINALLY || clause2->flags == MONO_EXCEPTION_CLAUSE_FINALLY)) {
	cfg->exception_message = g_strdup ("nested clauses");
	cfg->disable_llvm = TRUE;
	break;
	}
	}
	}

The archived referenced bug is: https://bugzilla.xamarin.com/37/37273/bug.html

Some EH info from compiling this test:

Method int IntelHardwareIntrinsicTest.Program:Main (string[]) emitted at 0x41d516a0 to 0x41d51f1d (code length 2173)
IL clause: try 0x15e-0x194 handler 0x194-0x199 filter 0x0
looking for end of try [350, 54] -> 0x55d2206dde78 (code size 550)
IL clause: try 0x15e-0x194 handler 0x199-0x1bd filter 0x0
looking for end of try [350, 54] -> 0x55d2206dde78 (code size 550)
IL clause: try 0x1be-0x1f4 handler 0x1f4-0x1f9 filter 0x0
looking for end of try [446, 54] -> 0x55d2206de3b8 (code size 550)
IL clause: try 0x1be-0x1f4 handler 0x1f9-0x21d filter 0x0
looking for end of try [446, 54] -> 0x55d2206de3b8 (code size 550)

Also enable JIT/HardwareIntrinsics/X86/Sse2.X64/StoreNonTemporal_{r,ro} with the fix

[imhameed]

The changes we'll need for exception filters (#54185) will be closely related to the changes needed to make nested EH clauses work (e.g. use of catchswitch, catchret, etc.)

Moving this to 7.0.

[imhameed]

A few notes on playing around with catchswitch, catchpad, and catchret:

The LLVM EH documentation claims that these instructions can be used to model C++ Itanium-style exception handling. This is true in theory but as of today LLVM makes several assumptions about the EH "personality" function and about the target's calling convention when these IR instructions are encountered while targeting arm64:

• https://github.com/dotnet/llvm-project/blob/release/11.x/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp#L104-L124
• https://github.com/dotnet/llvm-project/blob/release/11.x/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp#L1944-L1946
• https://github.com/dotnet/llvm-project/blob/release/11.x/llvm/lib/CodeGen/WinEHPrepare.cpp#L149-L164

I didn't encounter any of this on amd64, even when targeting Linux, so maybe this would be easy to fix. Also landingpad allows the runtime EH unwinder to communicate values back to the generated object code via registers. catchpad does not.

EDIT: This won't matter to us for a while, but GlobalISel doesn't support these new EH instructions

SEH filters are associated with individual catchpad instructions like so:

define dso_local i32 @testguy() #1 personality i8* bitcast (i32 (...)* @__C_specific_handler to i8*) {
  ; ...
  %7 = catchpad within %5 [i8* bitcast (i32 (i8*, i8*)* @"?filt$0@0@testguy@@" to i8*)]
  catchret from %7 to label %8
  ; ...
}

define internal i32 @"?filt$0@0@testguy@@"(i8* nocapture readnone %0, i8* %1) #3 {
  %3 = tail call i8* @llvm.eh.recoverfp(i8* bitcast (i32 ()* @testguy to i8*), i8* %1)
  %4 = tail call i8* @llvm.localrecover(i8* bitcast (i32 ()* @testguy to i8*), i8* %3, i32 0)
  %5 = bitcast i8* %4 to i32*
  %6 = load i32, i32* %5, align 4
  %7 = add nsw i32 %6, 2
  store i32 %7, i32* %5, align 4
  ret i32 15
}

This constant function pointer is associated with a BasicBlock here: https://github.com/dotnet/llvm-project/blob/release/11.x/llvm/lib/CodeGen/WinEHPrepare.cpp#L378-L383

[jandupej]

Bringing back the support for nested EH clauses would also mean fixing the original flow control bug. The original repro can be simplified to the following:

[MethodImpl(MethodImplOptions.NoInlining)]
public static void CatchDivideByZeroException() => Console.WriteLine("***FAIL*** Caught DivideByZeroException"); 

[MethodImpl(MethodImplOptions.NoInlining)]
public static void FinallyInner() => Console.WriteLine("Finally inner"); 

[MethodImpl(MethodImplOptions.NoInlining)]
public static void CatchException() => Console.WriteLine("***FAIL*** Caught Exception"); 

[MethodImpl(MethodImplOptions.NoInlining)]
public static void FinallyOuter() => Console.WriteLine("Finally outer"); 

public static void Demo()
{
    try
    {
        try
        { 
            throw new Exception();
        }
        catch(DivideByZeroException)
        {
            CatchDivideByZeroException();
        }
        finally
        {
            FinallyInner();
            throw new Exception();
        }
    } 
    catch(ArgumentException)
    {
        CatchException();
    }
    finally
    {
        FinallyOuter();
    }
}

Both catch blocks are executed, while none should be.

The inside of the inner-most try block translates to LLVM invokes with the exception path leading to this landing pad:

EH_CLAUSE0_BB3:                                   ; preds = %NOEX_BB5, %BB2
  %17 = landingpad { i8*, i32 }
          catch i32* @type_info_0, !dbg !45
  ...
  %ex_selector = extractvalue { i8*, i32 } %17, 1, !dbg !45
  switch i32 %ex_selector, label %BB3_CALL_HANDLER_TARGET [
    i32 3, label %BB7_CALL_HANDLER_TARGET
    i32 2, label %BB6_CALL_HANDLER_TARGET
    i32 1, label %BB4_CALL_HANDLER_TARGET
  ], !dbg !45

The label BB3_CALL_HANDLER_TARGET is the default branch and also the inside of the catch(DivideByZeroException) block. So if an unexpected exception is raised, the incorrect block is executed. That default is set in mini-llvm.c:5504.

[jandupej]

Simulations show that the performance and binary size benefits would be minimal. Closing.