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] Initial version of the managed stack traces on Browser #2677

Merged
merged 5 commits into from
Sep 13, 2024
Merged

[NativeAOT-LLVM] Initial version of the managed stack traces on Browser #2677

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
c660b4e
Initial runtime support for stack traces on Browser
SingleAccretion Sep 4, 2024
6f17f6c
Forbid LLVM inlining of EH frames with virtual unwind frames
SingleAccretion Sep 9, 2024
1849f5a
Respect AggressiveInlining
SingleAccretion Sep 9, 2024
fa226d9
Add a test
SingleAccretion Sep 4, 2024
309b95b
Add System.Diagnostics.StackTrace.Tests to CI
SingleAccretion Sep 4, 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
15 changes: 15 additions & 0 deletions src/coreclr/jit/llvmcodegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,15 @@ void Llvm::annotateFunctions()
{
llvmFunc->addFnAttr(llvm::Attribute::NoInline);
}

// The runtime stack trace logic relies on a 1-to-1 correspondence between WASM (and therefore LLVM)
// frames and virtual unwind frames; it uses this to determine when to stop appending stack frames.
// We therefore cannot let LLVM inline methods with virtual unwind frames (~= with catch handlers).
// TODO-LLVM-StackTrace: do we need to apply this to (finally) funclets too?
if (m_unwindFrameLclNum != BAD_VAR_NUM)
{
llvmFunc->addFnAttr(llvm::Attribute::NoInline);
}
}

if (_compiler->opts.OptimizationEnabled())
Expand All @@ -147,6 +156,12 @@ void Llvm::annotateFunctions()
llvmFunc->addFnAttr(llvm::Attribute::OptimizeForSize);
}

if ((_compiler->info.compFlags & CORINFO_FLG_FORCEINLINE) != 0)
{
// LLVM's "alwaysinline" is stronger than the CLR's 'AggressiveInlining'; use "inlinehint" instead.
llvmFunc->addFnAttr(llvm::Attribute::InlineHint);
}

// Mark the shadow stack dereferenceable.
if ((funcIdx != ROOT_FUNC_IDX) || _compiler->lvaGetDesc(_shadowStackLclNum)->lvIsParam)
{
Expand Down
19 changes: 11 additions & 8 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,21 +546,24 @@ The .NET Foundation licenses this file to you under the MIT license.
</ItemGroup>

<ItemGroup>
<_WasiComponentImports Include="$(IlcFrameworkNativePath)*.wit" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required for this PR, or just general goodness?

Copy link
Author

Choose a reason for hiding this comment

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

The latter. I tried changing a few things while chasing down the bug that would end up being references below (dotnet/msbuild#5937).


<NativeLibrary Include="$(IlcFrameworkNativePath)libSystem.Native.a" />
<NativeLibrary Include="$(IlcFrameworkNativePath)libSystem.Native.TimeZoneData.a" Condition="'$(InvariantTimezone)' != 'true'" />
<NativeLibrary Include="$(IlcFrameworkNativePath)libSystem.Native.TimeZoneData.Disabled.a" Condition="'$(InvariantTimezone)' == 'true'" />
<NativeLibrary Condition="'$(IlcIsLibLikeApp)' != 'true'" Include="$(IlcSdkPath)libbootstrapper.o" />
<NativeLibrary Condition="'$(IlcIsLibLikeApp)' == 'true'" Include="$(IlcSdkPath)libbootstrapperdll.o" />
<NativeLibrary Include="$(IlcSdkPath)libbootstrapper.o" Condition="'$(IlcIsLibLikeApp)' != 'true'" />
<NativeLibrary Include="$(IlcSdkPath)libbootstrapperdll.o" Condition="'$(IlcIsLibLikeApp)' == 'true'" />
<NativeLibrary Include="$(IlcSdkPath)libPortableRuntime.a" />
<NativeLibrary Include="$(IlcSdkPath)libstandalonegc-disabled$(LibFileExt)" />
<NativeLibrary Condition="'$(IlcLlvmExceptionHandlingModel)' == 'cpp'" Include="$(IlcSdkPath)libCppExceptionHandling.a" />
<NativeLibrary Condition="'$(IlcLlvmExceptionHandlingModel)' == 'wasm'" Include="$(IlcSdkPath)libWasmExceptionHandling.a" />
<NativeLibrary Condition="'$(IlcLlvmExceptionHandlingModel)' == 'emulated'" Include="$(IlcSdkPath)libEmulatedExceptionHandling.a" />
<_WasiComponentImports Include="$(IlcFrameworkNativePath)*.wit" />
<NativeLibrary Include="$(IlcSdkPath)libCppExceptionHandling.a" Condition="'$(IlcLlvmExceptionHandlingModel)' == 'cpp'" />
<NativeLibrary Include="$(IlcSdkPath)libWasmExceptionHandling.a" Condition="'$(IlcLlvmExceptionHandlingModel)' == 'wasm'" />
<NativeLibrary Include="$(IlcSdkPath)libEmulatedExceptionHandling.a" Condition="'$(IlcLlvmExceptionHandlingModel)' == 'emulated'" />

<!-- wasi-sdk clang wants forward slashes, emscripten doesn't care -->
<CustomLinkerArg Include="@(NativeLibrary->'&quot;%(Identity)&quot;'->Replace(&quot;\&quot;, &quot;/&quot;))" />
<CustomLinkerArg Include="@(NativeObjects->'&quot;%(Identity)&quot;'->Replace(&quot;\&quot;, &quot;/&quot;))" />
<!-- The canary function should be last in the linked output. But adding it to Native[Library|Object] runs into https://github.com/dotnet/msbuild/issues/5937. -->
<CustomLinkerArg Include="&quot;$(IlcSdkPath.Replace('\', '/'))libStackTraceIpCanary.o&quot;" Condition="'$(_targetOS)' == 'browser'" />
<CustomLinkerArg Include="@(_WasiComponentImports->Replace('\', '/')->'-Wl,--component-type,&quot;%(Identity)&quot;')" />
<CustomLinkerArg Include="-o &quot;$(NativeBinary.Replace(&quot;\&quot;, &quot;/&quot;))&quot;" />
<CustomLinkerArg Condition="'$(NativeDebugSymbols)' == 'true'" Include="-g3" />
Expand All @@ -571,7 +574,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<CustomLinkerArg Condition="'$(IlcLlvmTarget)' != ''" Include="-target $(IlcLlvmTarget)" />
</ItemGroup>

<ItemGroup Condition = "'$(_targetOS)' == 'browser'" >
<ItemGroup Condition="'$(_targetOS)' == 'browser'" >
<CustomLinkerArg Include="--js-library &quot;$(IlcFrameworkNativePath)pal_random.lib.js&quot;" Condition="'$(DotNetJsApi)' != 'true'" />
<CustomLinkerArg Condition="'$(WasmHtmlTemplate)' != ''" Include="--shell-file &quot;$(WasmHtmlTemplate)&quot;" />
<CustomLinkerArg Condition="'$(ExportsFile)' != '' and '$([System.IO.File]::ReadAllText($(ExportsFile)))' != ''" Include="-s EXPORTED_FUNCTIONS=@&quot;$(ExportsFile)&quot;" />
Expand All @@ -593,7 +596,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<Output TaskParameter="Lines" ItemName="CustomLinkerArgExport" />
</ReadLinesFromFile>

<ItemGroup Condition ="'$(_targetOS)' == 'wasi'" >
<ItemGroup Condition="'$(_targetOS)' == 'wasi'" >
<CustomLinkerArg Include="--sysroot=&quot;$(WASI_SDK_PATH.Replace(&quot;\&quot;, &quot;/&quot;))/share/wasi-sysroot&quot;" />
<CustomLinkerArg Include="@(CustomLinkerArgExport->'-Wl,--export=%(Identity)')" />
<!-- TODO-LLVM: change to 'warn-unresolved-symbols' once we have a WASI SDK with https://github.com/llvm/llvm-project/pull/78643. -->
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/nativeaot/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@
<WasmAbi>true</WasmAbi>
<PlatformTarget>AnyCPU</PlatformTarget>
<DefineConstants>TARGET_32BIT;TARGET_WASM;$(DefineConstants)</DefineConstants>
<DefineConstants Condition="'$(TargetsBrowser)' == 'true'">TARGET_BROWSER;$(DefineConstants)</DefineConstants>
<DefineConstants Condition="'$(TargetsWasi)' == 'true'">TARGET_WASI;$(DefineConstants)</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition="'$(Platform)' == 'x64'">
<DefineConstants>TARGET_64BIT;TARGET_AMD64;$(DefineConstants)</DefineConstants>
Expand Down
116 changes: 60 additions & 56 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 @@ -9,9 +9,12 @@
// Disable: Filter expression is a constant. We know. We just can't do an unfiltered catch.
#pragma warning disable 7095

//
// WASM exception handling.
// See: https://github.com/dotnet/runtimelab/blob/feature/NativeAOT-LLVM/docs/design/coreclr/botr/nativeaot-wasm-exception-handling.md.
//
namespace System.Runtime
{
// TODO-LLVM-EH: write and link a design document for this EH scheme. It is not terribly simple...
internal static unsafe partial class EH
{
private const nuint UnwindIndexNotInTry = 0;
Expand All @@ -21,6 +24,7 @@ internal static unsafe partial class EH
private static ExceptionDispatchData? t_lastDispatchedException;

[RuntimeExport("RhpThrowEx")]
[MethodImpl(MethodImplOptions.NoInlining)]
private static void RhpThrowEx(object exception)
{
#if INPLACE_RUNTIME
Expand All @@ -29,30 +33,26 @@ private static void RhpThrowEx(object exception)
#else
#error Implement "throw null" in non-INPLACE_RUNTIME builds
#endif
DispatchException(exception, 0);
DispatchException(exception, RhEHFrameType.RH_EH_FIRST_FRAME);
}

[RuntimeExport("RhpRethrow")]
private static void RhpRethrow(object pException)
[MethodImpl(MethodImplOptions.NoInlining)]
private static void RhpRethrow(object exception)
{
DispatchException(pException, RhEHFrameType.RH_EH_FIRST_RETHROW_FRAME);
DispatchException(exception, RhEHFrameType.RH_EH_FIRST_FRAME | RhEHFrameType.RH_EH_FIRST_RETHROW_FRAME);
}

// Note that this method cannot have any catch handlers as it manipulates the virtual unwind frames directly
// and exits via native unwind (it would not pop the frame it would push). This is accomplished by calling
// all user code via separate noinline methods. It also cannot throw any exceptions as that would lead to
// infinite recursion.
//
[MethodImpl(MethodImplOptions.NoInlining)]
private static void DispatchException(object exception, RhEHFrameType flags)
{
WasmEHLogFirstPassEnter(exception, flags);

WasmEHLogFirstPassEnter(exception, (flags & RhEHFrameType.RH_EH_FIRST_RETHROW_FRAME) != 0);
OnFirstChanceExceptionNoInline(exception);
#if INPLACE_RUNTIME
Exception.InitializeExceptionStackFrameLLVM(exception, (int)flags);
#else
#error Make InitializeExceptionStackFrameLLVM into a classlib export
#endif

// Find the handler for this exception by virtually unwinding the stack of active protected regions.
VirtualUnwindFrame** pLastFrameRef = (VirtualUnwindFrame**)InternalCalls.RhpGetRawLastVirtualUnwindFrameRef();
Expand All @@ -61,11 +61,14 @@ private static void DispatchException(object exception, RhEHFrameType flags)
nuint unwindCount = 0;
while (pFrame != null)
{
EHClause clause;
EHTable table = new EHTable(pFrame->UnwindTable);
GetAppendStackFrame(exception)(exception, table.GetStackTraceIP(), (int)flags);
flags = 0;

nuint index = pFrame->UnwindIndex;
while (IsCatchUnwindIndex(index))
{
EHClause clause;
nuint enclosingIndex = table.GetClauseInfo(index, &clause);
WasmEHLogEHTableEntry(pFrame, index, &clause);

Expand Down Expand Up @@ -310,6 +313,9 @@ private static void RhpPopUnwoundVirtualFrames()
[RuntimeExport("RhpHandleUnhandledException")]
private static void HandleUnhandledException(object exception)
{
// Append JS frames to this unhandled exception for better diagnostics.
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens in Wasi here, or does it never get here?

Copy link
Author

@SingleAccretion SingleAccretion Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This eventually calls down into the WASI version of AppendStack, which is just a no-op for now.

Edit: in the eventual work that will bring stack traces to WASI, it will be just filtered out with a null IP check, or I'll ifdef it out for browser only. Not sure yet what will look better.

GetAppendStackFrame(exception)(exception, 0, 0);

OnUnhandledExceptionViaClassLib(exception);

// We have to duplicate "UnhandledExceptionFailFastViaClasslib" because we cannot use code addresses to get helpers.
Expand All @@ -334,6 +340,13 @@ private static void HandleUnhandledException(object exception)
FallbackFailFast(RhFailFastReason.UnhandledException, exception);
}

private static delegate*<object, IntPtr, int, void> GetAppendStackFrame(object exception)
{
// We use this more direct way of invoking "AppendExceptionStackFrame" to preserve more user frames in truncated traces.
nint pAppendStackFrame = exception.GetMethodTable()->GetClasslibFunction(ClassLibFunctionId.AppendExceptionStackFrame);
return (delegate*<object, IntPtr, int, void>)pAppendStackFrame;
}

// These are pushed by codegen on the shadow stack for frames that have at least one region protected by a catch.
//
private struct VirtualUnwindFrame
Expand All @@ -360,71 +373,62 @@ private unsafe struct EHClause

private unsafe struct EHTable
{
private const nuint MetadataFilter = 1;
private const nuint MetadataClauseTypeFormat = 2;
private const int MetadataShift = 1;
private const nuint MetadataMask = ~(1u << MetadataShift);

private const nuint FormatClauseType = 0;
private const nuint FormatSmall = 1;
private const nuint FormatLarge = 2;
private const nuint FormatMask = 3;
private const nuint MetadataLargeFormat = 1;
private const nuint MetadataFilter = 1 << 1;
private const int MetadataShift = 2;

private readonly void* _pEHTable;
private readonly nuint _format;
private readonly byte* _pEHTable;
private readonly bool _isLargeFormat;

public EHTable(void* pUnwindTable)
{
_pEHTable = (void*)((nuint)pUnwindTable & ~FormatMask);
_format = (nuint)pUnwindTable & FormatMask;
_pEHTable = (byte*)pUnwindTable;
_isLargeFormat = (*(byte*)pUnwindTable & MetadataLargeFormat) != 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose using _pEHTable would remove a cast?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but at the cost of a field load (which is likely to get removed, but I like 'efficiency by construction' in system code).

}

public readonly nuint GetClauseInfo(nuint index, EHClause* pClause = null)
{
nuint metadata;
nuint enclosingIndex = GetMetadata(index, &metadata);
Debug.Assert(IsCatchUnwindIndex(index));
nuint largeFormatEntrySize = (nuint)(sizeof(uint) + sizeof(void*));
nuint smallFormatEntrySize = (nuint)(sizeof(byte) + sizeof(void*));

nuint zeroBasedIndex = index - UnwindIndexBase;
nuint metadata = _isLargeFormat
? Unsafe.ReadUnaligned<uint>(_pEHTable + zeroBasedIndex * largeFormatEntrySize)
: Unsafe.ReadUnaligned<byte>(_pEHTable + zeroBasedIndex * smallFormatEntrySize);

if (pClause != null)
{
if (metadata == MetadataClauseTypeFormat)
{
pClause->Filter = null;
pClause->ClauseType = (MethodTable*)_pEHTable;
}
else if ((metadata & MetadataFilter) != 0)
nuint value = _isLargeFormat
? Unsafe.ReadUnaligned<nuint>(_pEHTable + zeroBasedIndex * largeFormatEntrySize + sizeof(uint))
: Unsafe.ReadUnaligned<nuint>(_pEHTable + zeroBasedIndex * smallFormatEntrySize + sizeof(byte));

if ((metadata & MetadataFilter) != 0)
{
pClause->Filter = ((void**)_pEHTable)[index - UnwindIndexBase];
pClause->Filter = (void*)value;
pClause->ClauseType = null;
}
else
{
pClause->Filter = null;
pClause->ClauseType = ((MethodTable**)_pEHTable)[index - UnwindIndexBase];
pClause->ClauseType = (MethodTable*)value;
}
}

nuint enclosingIndex = metadata >> MetadataShift;
return enclosingIndex;
}

private readonly nuint GetMetadata(nuint index, nuint* pMetadata)
#pragma warning disable CA1822 // Member 'GetStackTraceIP' does not access instance data and can be marked as static
public readonly nint GetStackTraceIP()
{
Debug.Assert(IsCatchUnwindIndex(index));
nuint metadata;
switch (_format)
{
case FormatClauseType:
*pMetadata = MetadataClauseTypeFormat;
return UnwindIndexNotInTry;
case FormatSmall:
metadata = ((byte*)_pEHTable)[-(nint)(index - UnwindIndexBase + 1)];
break;
default:
Debug.Assert(_format == FormatLarge);
metadata = ((uint*)_pEHTable)[-(nint)(index - UnwindIndexBase + 1)];
break;
}

*pMetadata = metadata & MetadataMask;
return metadata >> MetadataShift;
#if TARGET_BROWSER
int wasmFunctionIndex = Unsafe.ReadUnaligned<int>(_pEHTable - sizeof(int));
int wasmFunctionIndexWithBias = Exception.GetBiasedWasmFunctionIndex(wasmFunctionIndex);
return wasmFunctionIndexWithBias;
#else
return 0;
#endif
}
}

Expand Down Expand Up @@ -459,9 +463,9 @@ private static void WasmEHLog(string message, int pass, string prefix = "")
}

[Conditional("ENABLE_NOISY_WASM_EH_LOG")]
private static void WasmEHLogFirstPassEnter(object exception, RhEHFrameType flags)
private static void WasmEHLogFirstPassEnter(object exception, bool isFirstRethrowFrame)
{
string kind = (flags & RhEHFrameType.RH_EH_FIRST_RETHROW_FRAME) != 0 ? "Rethrowing" : "Throwing";
string kind = isFirstRethrowFrame ? "Rethrowing" : "Throwing";
WasmEHLog(kind + ": [" + exception.GetType() + "]", 1, "\n");
}

Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/RuntimeExports.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,13 +292,8 @@ public static unsafe void RhUnbox(object? obj, ref byte data, MethodTable* pUnbo
[MethodImpl(MethodImplOptions.NoInlining)] // Ensures that the RhGetCurrentThreadStackTrace frame is always present
public static unsafe int RhGetCurrentThreadStackTrace(IntPtr[] outputBuffer)
{
#if TARGET_WASM
// TODO-LLVM: https://github.com/dotnet/runtimelab/issues/2404.
throw new NotImplementedException();
#else
fixed (IntPtr* pOutputBuffer = outputBuffer)
return RhpGetCurrentThreadStackTrace(pOutputBuffer, (uint)((outputBuffer != null) ? outputBuffer.Length : 0), new UIntPtr(&pOutputBuffer));
#endif
}

#pragma warning disable SYSLIB1054 // Use DllImport here instead of LibraryImport because this file is used by Test.CoreLib.
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/nativeaot/Runtime/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ if (CLR_CMAKE_TARGET_ARCH_WASM)
${ARCH_SOURCES_DIR}/StubDispatch.cpp
${ARCH_SOURCES_DIR}/WriteBarriers.cpp
)
if (CLR_CMAKE_TARGET_BROWSER)
list(APPEND COMMON_RUNTIME_SOURCES ${ARCH_SOURCES_DIR}/StackTrace.Browser.cpp)

add_library(StackTraceIpCanary OBJECT ${ARCH_SOURCES_DIR}/StackTraceIpCanary.cpp)
add_dependencies(nativeaot StackTraceIpCanary)
install(FILES $<TARGET_OBJECTS:StackTraceIpCanary> DESTINATION aotsdk COMPONENT nativeaot RENAME libStackTraceIpCanary.o)
endif()
endif (CLR_CMAKE_TARGET_ARCH_WASM)

list(APPEND RUNTIME_SOURCES_ARCH_ASM
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/nativeaot/Runtime/RuntimeInstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ FCIMPL1(uint8_t *, RhGetRuntimeVersion, int32_t* pcbLength)
}
FCIMPLEND

#ifndef TARGET_BROWSER
FCIMPL1(uint8_t *, RhFindMethodStartAddress, void * codeAddr)
{
uint8_t *startAddress = dac_cast<uint8_t *>(GetRuntimeInstance()->FindMethodStartAddress(dac_cast<PTR_VOID>(codeAddr)));
Expand All @@ -76,6 +77,7 @@ FCIMPL1(uint8_t *, RhFindMethodStartAddress, void * codeAddr)
#endif
}
FCIMPLEND
#endif // !TARGET_BROWSER

PTR_uint8_t RuntimeInstance::FindMethodStartAddress(PTR_VOID ControlPC)
{
Expand Down
Loading