From cddf083119c10974310e18997f8a412d00a47d93 Mon Sep 17 00:00:00 2001 From: Stefan Jandl Date: Tue, 21 Feb 2023 14:20:51 +0100 Subject: [PATCH] feat: Setting the `instruction_addr_adjustment` for IL2CPP exceptions (#1165) --- CHANGELOG.md | 4 ++ package-dev/Runtime/SentryInitialization.cs | 15 ------ src/Sentry.Unity/ISentryUnityInfo.cs | 1 - src/Sentry.Unity/Il2CppEventProcessor.cs | 52 +++++++++---------- .../SentryUnityOptionsExtensions.cs | 2 +- 5 files changed, 31 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 810abce0c..2349b2a70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Feature + +- Much improved line numbers for IL2CPP builds by setting the `instruction_addr_adjustment` appropriately ([#1165](https://github.com/getsentry/sentry-unity/pull/1165)) + ### Fixes - Preventing `LoggingIntegration` from registering multiple times ([#1178](https://github.com/getsentry/sentry-unity/pull/1178)) diff --git a/package-dev/Runtime/SentryInitialization.cs b/package-dev/Runtime/SentryInitialization.cs index bcd8de35d..fdfd0211f 100644 --- a/package-dev/Runtime/SentryInitialization.cs +++ b/package-dev/Runtime/SentryInitialization.cs @@ -132,21 +132,6 @@ public bool IL2CPP #endif } - public string Platform - { - get => -#if UNITY_IOS || UNITY_STANDALONE_OSX - "macho" -#elif UNITY_ANDROID || UNITY_STANDALONE_LINUX - "elf" -#elif UNITY_STANDALONE_WIN - "pe" -#else - "unknown" -#endif - ; - } - public Il2CppMethods Il2CppMethods => _il2CppMethods; private Il2CppMethods _il2CppMethods diff --git a/src/Sentry.Unity/ISentryUnityInfo.cs b/src/Sentry.Unity/ISentryUnityInfo.cs index dabc7da6f..1150bacaf 100644 --- a/src/Sentry.Unity/ISentryUnityInfo.cs +++ b/src/Sentry.Unity/ISentryUnityInfo.cs @@ -5,7 +5,6 @@ namespace Sentry.Unity public interface ISentryUnityInfo { public bool IL2CPP { get; } - public string? Platform { get; } public Il2CppMethods? Il2CppMethods { get; } } diff --git a/src/Sentry.Unity/Il2CppEventProcessor.cs b/src/Sentry.Unity/Il2CppEventProcessor.cs index 03be0bc06..195504c40 100644 --- a/src/Sentry.Unity/Il2CppEventProcessor.cs +++ b/src/Sentry.Unity/Il2CppEventProcessor.cs @@ -1,27 +1,23 @@ using System; using System.Collections.Generic; -using System.IO; using System.Linq; using System.Runtime.InteropServices; using Sentry.Extensibility; -using Sentry.Protocol; using Sentry.Unity.Integrations; using Sentry.Unity.NativeUtils; using UnityEngine; -using static System.Net.Mime.MediaTypeNames; +using Application = UnityEngine.Application; namespace Sentry.Unity { internal class UnityIl2CppEventExceptionProcessor : ISentryEventExceptionProcessor { private readonly SentryUnityOptions _options; - private readonly ISentryUnityInfo _sentryUnityInfo; private readonly Il2CppMethods _il2CppMethods; - public UnityIl2CppEventExceptionProcessor(SentryUnityOptions options, ISentryUnityInfo sentryUnityInfo, Il2CppMethods il2CppMethods) + public UnityIl2CppEventExceptionProcessor(SentryUnityOptions options, Il2CppMethods il2CppMethods) { _options = options; - _sentryUnityInfo = sentryUnityInfo; _il2CppMethods = il2CppMethods; _options.SdkIntegrationNames.Add("IL2CPPLineNumbers"); @@ -39,7 +35,7 @@ public void Process(Exception incomingException, SentryEvent sentryEvent) var exceptions = EnumerateChainedExceptions(incomingException); var usedImages = new HashSet(); - _logger = _options.DiagnosticLogger; + Logger = _options.DiagnosticLogger; // Unity usually produces stack traces with relative offsets in the GameAssembly library. // However, at least on Unity 2020 built Windows player, the offsets seem to be absolute. @@ -55,6 +51,11 @@ public void Process(Exception incomingException, SentryEvent sentryEvent) continue; } + sentryStacktrace.AddressAdjustment = + Application.platform == RuntimePlatform.Android + ? InstructionAddressAdjustment.None + : InstructionAddressAdjustment.All; + var nativeStackTrace = GetNativeStackTrace(exception); _options.DiagnosticLogger?.LogDebug("NativeStackTrace Image: '{0}' (UUID: {1})", nativeStackTrace.ImageName, nativeStackTrace.ImageUuid); @@ -72,7 +73,6 @@ public void Process(Exception incomingException, SentryEvent sentryEvent) // Wouldn't it cause invalid frame info? var nativeLen = nativeStackTrace.Frames.Length; var eventLen = sentryStacktrace.Frames.Count; - if (nativeLen != eventLen) { _options.DiagnosticLogger?.LogWarning( @@ -89,17 +89,9 @@ public void Process(Exception incomingException, SentryEvent sentryEvent) var nativeFrame = nativeStackTrace.Frames[nativeLen - 1 - i]; var mainImageUUID = NormalizeUUID(nativeStackTrace.ImageUuid); - // _options.DiagnosticLogger?.Log(SentryLevel.Debug, "Processing stack frame '{0}' image address: {1:X8}, packge: {2}", - // null, frame.Function, frame.ImageAddress, frame.Package); - - // The instructions in the stack trace generally have "return addresses" - // in them. But for symbolication, we want to symbolicate the address of - // the "call instruction", which in almost all cases happens to be - // the instruction right in front of the return address. - // A good heuristic to use in that case is to just subtract 1. // TODO should we do this for all addresses or only relative ones? // If the former, we should also update `frame.InstructionAddress` down below. - var instructionAddress = (ulong)nativeFrame.ToInt64() - 1; + var instructionAddress = (ulong)nativeFrame.ToInt64(); // We cannot determine whether this frame is a main library frame just from the address // because even relative address on the frame may correspond to an absolute addres of a loaded library. @@ -136,11 +128,19 @@ public void Process(Exception incomingException, SentryEvent sentryEvent) continue; } - // First, try to find the image among loaded one, otherwise create a dummy one. + // First, try to find the image among the loaded ones, otherwise create a dummy one. mainLibImage ??= DebugImagesSorted.Value.Find((info) => string.Equals(NormalizeUUID(info.Image.DebugId), mainImageUUID))?.Image; mainLibImage ??= new DebugImage { - Type = _sentryUnityInfo.Platform, + Type = Application.platform switch + { + RuntimePlatform.Android => "elf", + RuntimePlatform.IPhonePlayer => "macho", + RuntimePlatform.OSXPlayer => "macho", + RuntimePlatform.LinuxPlayer => "elf", + RuntimePlatform.WindowsPlayer => "pe", + _ => "unknown" + }, // NOTE: il2cpp in some circumstances does not return a correct `ImageName`. // A null/missing `CodeFile` however would lead to a processing error in sentry. // Since the code file is not strictly necessary for processing, we just fall back to @@ -213,7 +213,7 @@ public DebugImageInfo(DebugImage image) public bool ContainsAddress(ulong address) => StartAddress <= address && address <= EndAddress; } - private static IDiagnosticLogger? _logger; + private static IDiagnosticLogger? Logger; private static readonly Lazy> DebugImagesSorted = new(() => { @@ -228,14 +228,14 @@ public DebugImageInfo(DebugImage image) { if (image.ImageSize is null) { - _logger?.Log(SentryLevel.Debug, + Logger?.Log(SentryLevel.Debug, "Skipping debug image '{0}' (CodeId {1} | DebugId: {2}) because its size is NULL", null, image.CodeFile, image.CodeId, image.DebugId); continue; } var info = new DebugImageInfo(image); - int i = 0; + var i = 0; for (; i < result.Count; i++) { if (info.StartAddress < result[i].StartAddress) @@ -246,7 +246,7 @@ public DebugImageInfo(DebugImage image) } result.Insert(i, info); - _logger?.Log(SentryLevel.Debug, + Logger?.Log(SentryLevel.Debug, "Found debug image '{0}' (CodeId {1} | DebugId: {2}) with addresses between {3:X8} and {4:X8}", null, image.CodeFile, image.CodeId, image.DebugId, info.StartAddress, info.EndAddress); } @@ -259,11 +259,11 @@ public DebugImageInfo(DebugImage image) var list = DebugImagesSorted.Value; // Manual binary search implementation on "value in range". - int lowerBound = 0; - int upperBound = list.Count - 1; + var lowerBound = 0; + var upperBound = list.Count - 1; while (lowerBound <= upperBound) { - int mid = (lowerBound + upperBound) / 2; + var mid = (lowerBound + upperBound) / 2; var info = list[mid]; if (info.StartAddress <= instructionAddress) diff --git a/src/Sentry.Unity/SentryUnityOptionsExtensions.cs b/src/Sentry.Unity/SentryUnityOptionsExtensions.cs index 3c4c258a9..90c9d380e 100644 --- a/src/Sentry.Unity/SentryUnityOptionsExtensions.cs +++ b/src/Sentry.Unity/SentryUnityOptionsExtensions.cs @@ -67,7 +67,7 @@ internal static void AddIl2CppExceptionProcessor(this SentryUnityOptions options { if (unityInfo.Il2CppMethods is not null) { - options.AddExceptionProcessor(new UnityIl2CppEventExceptionProcessor(options, unityInfo, unityInfo.Il2CppMethods)); + options.AddExceptionProcessor(new UnityIl2CppEventExceptionProcessor(options, unityInfo.Il2CppMethods)); } else {