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.

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

feat: Capture Native Instruction Addrs for Exceptions #683

Merged
merged 28 commits into from
Jun 20, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
c5b626c
feat: Capture Native Instruction Addrs for Exceptions
Swatinem Jun 1, 2022
7ddbba6
Apply suggestions from code review
Swatinem Jun 1, 2022
13b8d24
fixup code after suggestions, guard free
Swatinem Jun 1, 2022
32f493d
revert integration test changes
Swatinem Jun 1, 2022
45caf63
Merge branch 'main' into feat/native-stack
bitsandfoxes Jun 1, 2022
1d0a073
version sensitive il2cpp methods
bitsandfoxes Jun 2, 2022
64a6871
Format code
getsentry-bot Jun 2, 2022
f30d517
expression bodies are expressive
bitsandfoxes Jun 2, 2022
cd77c2d
Merge branch 'main' into feat/native-stack
bitsandfoxes Jun 2, 2022
173e387
Merge branch 'feat/native-stack' of https://github.com/getsentry/sent…
bitsandfoxes Jun 2, 2022
5f3f45c
merged main
bitsandfoxes Jun 9, 2022
e9e8b98
unfix hex formatting of instruction addrs
Swatinem Jun 9, 2022
dc339c1
use absolute addressing mode, set a hardcoded macho image type
Swatinem Jun 10, 2022
3b2c226
remove usage of AddrMode
Swatinem Jun 14, 2022
dd7ce59
detect the correct debug image platform
Swatinem Jun 14, 2022
fdb7fdb
merged main
bitsandfoxes Jun 14, 2022
658e79c
I do as the Rider guides
bitsandfoxes Jun 14, 2022
243c491
merged conflicts
bitsandfoxes Jun 14, 2022
ed9ea32
Format code
getsentry-bot Jun 14, 2022
b12c03b
fixed tests
bitsandfoxes Jun 14, 2022
f9e7479
removed nullable warnings inside unity
bitsandfoxes Jun 14, 2022
622b74f
Merge branch 'feat/native-stack' of https://github.com/getsentry/sent…
bitsandfoxes Jun 14, 2022
3c053c8
revert breaking changes to il2cpp native methods
Swatinem Jun 15, 2022
9221641
use a better workaround for empty code_file
Swatinem Jun 15, 2022
426930d
Merge branch 'main' of github.com:getsentry/sentry-unity into feat/na…
bruno-garcia Jun 18, 2022
a0df812
enabled the il2cpp processor only on il2cpp ;-)
Swatinem Jun 20, 2022
82a3bac
added opt-in flag to unity options
bitsandfoxes Jun 20, 2022
62afa40
Merge branch 'main' into feat/native-stack
bitsandfoxes Jun 20, 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
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Capture Native Instruction Addrs for Exceptions ([#683](https://github.com/getsentry/sentry-unity/pull/683))

## 0.17.0

### Features
Expand Down Expand Up @@ -59,7 +65,7 @@
### Fixes

- Refactor InApp logic from Stack Traces ([#661](https://github.com/getsentry/sentry-unity/pull/661))
- Whitespaces no longer cause issues when uploading symbols for Windows native ([#655](https://github.com/getsentry/sentry-unity/pull/655))
- Whitespaces no longer cause issues when uploading symbols for Windows native ([#655](https://github.com/getsentry/sentry-unity/pull/655))
- AndroidManifest update removes previous `io.sentry` entries ([#652](https://github.com/getsentry/sentry-unity/pull/652))
- Bump Sentry .NET SDK 3.16.0 ([#678](https://github.com/getsentry/sentry-unity/pull/678))
- [changelog 3.16.0](https://github.com/getsentry/sentry-dotnet/blob/3.16.0/CHANGELOG.md)
Expand Down Expand Up @@ -127,7 +133,7 @@

### Fixes

- Sentry.Unity.Editor.iOS.dll no longer breaks builds on Windows when the iOS module has not been installed ([#559](https://github.com/getsentry/sentry-unity/pull/559))
- Sentry.Unity.Editor.iOS.dll no longer breaks builds on Windows when the iOS module has not been installed ([#559](https://github.com/getsentry/sentry-unity/pull/559))
- Importing the link.xml when opening the config window no longer causes an infinite loop ([#539](https://github.com/getsentry/sentry-unity/pull/539))
- Bump Sentry .NET SDK 3.14.0 ([#561](https://github.com/getsentry/sentry-unity/pull/561))
- [changelog](https://github.com/getsentry/sentry-dotnet/blob/3.14.0/CHANGELOG.md)
Expand Down
19 changes: 19 additions & 0 deletions src/Sentry.Unity.Editor/Il2CppOption.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using UnityEditor;
using UnityEditor.Build;
using UnityEditor.Build.Reporting;
using UnityEngine;

namespace Sentry.Unity.Editor
{
internal class Il2CppOption : IPreprocessBuildWithReport
{
public int callbackOrder => 0;

public void OnPreprocessBuild(BuildReport report)
{
var arguments = "--emit-source-mapping";
Debug.Log($"Setting additional IL2CPP arguments = '{arguments}' for platform {report.summary.platform}");
PlayerSettings.SetAdditionalIl2CppArgs(arguments);
}
}
}
1 change: 1 addition & 0 deletions src/Sentry.Unity.Editor/SentryCliOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public sealed class SentryCliOptions : ScriptableObject

[field: SerializeField] public bool UploadSymbols { get; set; } = true;
[field: SerializeField] public bool UploadDevelopmentSymbols { get; set; } = false;

[field: SerializeField] public string? UrlOverride { get; set; }
[field: SerializeField] public string? Auth { get; set; }
[field: SerializeField] public string? Organization { get; set; }
Expand Down
156 changes: 156 additions & 0 deletions src/Sentry.Unity/Il2CppEventProcessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.InteropServices;
using Sentry.Extensibility;
using Sentry.Protocol;

namespace Sentry.Unity
{
// TODO: Make sure this whole functionality/class is only compiled when:
// * Compiling for the il2cpp backend.
// * Using Unity 2020 or later, as we use internal `libil2cpp` APIs that are
// only available there.
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved

internal class UnityIl2CppEventExceptionProcessor : ISentryEventExceptionProcessor
{
public void Process(Exception incomingException, SentryEvent sentryEvent)
{
var sentryExceptions = sentryEvent.SentryExceptions;
if (sentryExceptions == null)
{
return;
}
var exceptions = EnumerateChainedExceptions(incomingException);

// Unity by definition only builds a single library/image,
// which we add once to our list of debug images.
var debugImages = (sentryEvent.DebugImages ??= new List<DebugImage>());
// The il2cpp APIs give us image-relative instruction addresses, not
// absolute ones, and we also do not get the image addr.
// For this reason we will use the "rel:N" AddressMode, giving the
// index of the image in the list of all debug images.
string? addrMode = null;

foreach (var (sentryException, exception) in sentryExceptions.Zip(exceptions, (se, ex) => (se, ex)))
{
var sentryStacktrace = sentryException.Stacktrace;
if (sentryStacktrace == null)
{
// We will only augment an existing stack trace with native
// instructions, so with no stack trace, there is nothing to do
continue;
}

var nativeStackTrace = GetNativeStackTrace(exception);

if (addrMode == null)
{
var imageIdx = debugImages.Count;
debugImages.Add(new DebugImage
{
// NOTE: This obviously is not wasm, but that type is used for
// images that do not have a `image_addr` but are rather used with "rel:N" AddressMode.
Type = "wasm",
Swatinem marked this conversation as resolved.
Show resolved Hide resolved
CodeFile = nativeStackTrace.ImageName,
DebugId = nativeStackTrace.ImageUuid,
});
addrMode = "rel:" + imageIdx;
}

var nativeLen = nativeStackTrace.Frames.Length;
var len = Math.Min(sentryStacktrace.Frames.Count, nativeLen);
for (int i = 0; i < len; i++)
{
// The sentry stack trace is sorted parent (caller) to child (callee),
// whereas the native stack trace is sorted from callee to caller.
var frame = sentryStacktrace.Frames[i];
var nativeFrame = nativeStackTrace.Frames[nativeLen - 1 - i];
frame.InstructionAddress = $"0x{nativeFrame:X8}");
frame.AddressMode = addrMode;
}
}
}

// This is the same logic as `MainExceptionProcessor` uses to create the `SentryEvent.SentryExceptions` list.
Copy link
Member

Choose a reason for hiding this comment

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

This behavior just changed in the .NET SDK: https://github.com/getsentry/sentry-dotnet/pull/1672/files

We should instead refactor this out so we use either one strategy or the other, and have the base behavior on some base class or helper method

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats a good point. I would have loved to just have an iterator that yields exceptions.
The .NET SDK could just map over that iterator to yield sentry exceptions. And this here could use the exact same iterator to zip.

Copy link
Contributor

Choose a reason for hiding this comment

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

// It yields chained Exceptions in innermost to outer Exception order.
internal IEnumerable<Exception> EnumerateChainedExceptions(Exception exception)
{
if (exception is AggregateException ae)
{
foreach (var inner in ae.InnerExceptions.SelectMany(EnumerateChainedExceptions))
{
yield return inner;
}
}
else if (exception.InnerException != null)
{
foreach (var inner in EnumerateChainedExceptions(exception.InnerException))
{
yield return inner;
}
}
yield return exception;
}

private NativeStackTrace GetNativeStackTrace(Exception e)
{
// Create a `GCHandle` for the exception, which we can then use to
// essentially get a pointer to the underlying `Il2CppException` C++ object.
var gch = GCHandle.Alloc(e);
// The `il2cpp_native_stack_trace` allocates and writes the native
// instruction pointers to the `addresses`/`numFrames` out-parameters.
var addresses = IntPtr.Zero;
try
{
var gchandle = GCHandle.ToIntPtr(gch).ToInt32();
var addr = il2cpp_gchandle_get_target(gchandle);

var numFrames = 0;
string? imageUUID = null;
string? imageName = null;
il2cpp_native_stack_trace(addr, out addresses, out numFrames, out imageUUID, out imageName);

// Convert the C-Array to a managed "C#" Array, and free the underlying memory.
var frames = new IntPtr[numFrames];
Marshal.Copy(addresses, frames, 0, numFrames);
}
finally
{
// We are done with the `GCHandle`.
gch.Free();

il2cpp_free(addresses);
}

return new NativeStackTrace
{
Frames = frames,
ImageUuid = imageUUID,
ImageName = imageName,
};
}

// NOTE: fn is available in Unity `2019.4.34f1` (and later)
// Il2CppObject* il2cpp_gchandle_get_target(uint32_t gchandle)
[DllImport("__Internal")]
private static extern IntPtr il2cpp_gchandle_get_target(int gchandle);

// NOTE: fn is available in Unity `2020.3.30f1` (and later)
// void il2cpp_native_stack_trace(const Il2CppException * ex, uintptr_t** addresses, int* numFrames, char** imageUUID, char** imageName)
[DllImport("__Internal")]
private static extern void il2cpp_native_stack_trace(IntPtr exc, out IntPtr addresses, out int numFrames, out string? imageUUID, out string? imageName);

// NOTE: fn is available in Unity `2019.4.34f1` (and later)
// void il2cpp_free(void* ptr)
[DllImport("__Internal")]
private static extern void il2cpp_free(IntPtr ptr);
}

internal class NativeStackTrace
{
public IntPtr[] Frames { get; set; } = Array.Empty<IntPtr>();
public string? ImageUuid { get; set; };
public string? ImageName { get; set; };
}
}
5 changes: 4 additions & 1 deletion src/Sentry.Unity/SentryUnityOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ internal SentryUnityOptions(IApplication application, bool isBuilding)
this.AddInAppExclude("UnityEngine");
this.AddInAppExclude("UnityEditor");
this.AddEventProcessor(new UnityEventProcessor(this, SentryMonoBehaviour.Instance));
this.AddExceptionProcessor(new UnityEventExceptionProcessor());

// TODO: conditionally use this only when compiling with il2cpp and targeting Unity >= 2020
this.AddExceptionProcessor(new UnityIl2CppEventExceptionProcessor());

this.AddIntegration(new UnityLogHandlerIntegration());
this.AddIntegration(new UnityBeforeSceneLoadIntegration());
this.AddIntegration(new SceneManagerIntegration());
Expand Down
7 changes: 0 additions & 7 deletions src/Sentry.Unity/UnityEventProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -283,13 +283,6 @@ private void PopulateUser(SentryEvent @event)
}
}

internal class UnityEventExceptionProcessor : ISentryEventExceptionProcessor
{
public void Process(Exception exception, SentryEvent sentryEvent)
{
}
}

internal static class TagValueNormalizer
{
internal static string ToTagValue(this Boolean value) => value ? "true" : "false";
Expand Down
12 changes: 12 additions & 0 deletions test/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@
<HintPath>$(UnityTestPath)/UnityEngine.TestRunner.dll</HintPath>
<Private>false</Private>
</Reference>

<!-- TODO: The parent directory contains a 'UnityEngine.dll' but that one is different? But if that one does not get -->
<!-- referenced here and we just add the CoreModule it leads to 'ambiguous references' and 'Type exists in both -->
<!-- 'UnityEngine.CoreModule and UnityEngine' -->
<Reference Include="UnityEngine">
<HintPath>$(UnityManagedPath)/UnityEngine/UnityEngine.dll</HintPath>
<Private>false</Private>
</Reference>
<Reference Include="UnityEngine.CoreModule">
<HintPath>$(UnityManagedPath)/UnityEngine/UnityEngine.CoreModule.dll</HintPath>
<Private>false</Private>
</Reference>
</ItemGroup>
<Error Condition="!Exists('$(UnityTestPath)/UnityEngine.TestRunner.dll')" Text="TestRunner not found. Expected: $(UnityTestPath)/UnityEngine.TestRunner.dll"></Error>
</Target>
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Unity.Editor.Tests/SentryCliTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class SentryCliTests
[Test]
public void GetSentryCliPlatformName_UnrecognizedPlatform_ThrowsInvalidOperationException()
{
var application = new TestApplication(platform: RuntimePlatform.CloudRendering);
var application = new TestApplication(platform: RuntimePlatform.LinuxPlayer);

Assert.Throws<InvalidOperationException>(() => SentryCli.GetSentryCliPlatformName(application));
}
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Unity.Tests/IntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public IEnumerator ThrowException_SendDefaultPiiIsFalse_EventDoesNotIncludeEnvir
}

[UnityTest]
public IEnumerator BugFarmScene_MultipleSentryInit_SendEventForTheLatest()
public IEnumerator DebugLogError_InTask_IsCapturedAndIsMainThreadIsFalse()
{
yield return SetupSceneCoroutine("1_BugFarm");

Expand Down