Fix native stack frame parsing crash for frames without symbols #258
Conversation
Add bounds checks in SetNativeStackTraceInformation Add NativeParserTests to validate safe native frame parsing
| { | ||
| private static BacktraceStackFrame Parse(string frame) | ||
| { | ||
| var instance = new BacktraceUnhandledException("msg", ""); |
There was a problem hiding this comment.
this is not a valid way to test it. Please follow the parser tests that we already have in the tests.
| var mi = typeof(BacktraceUnhandledException) | ||
| .GetMethod("SetNativeStackTraceInformation", | ||
| BindingFlags.NonPublic | BindingFlags.Instance); | ||
| Assert.IsNotNull(mi); |
| private static BacktraceStackFrame Parse(string frame) | ||
| { | ||
| var instance = new BacktraceUnhandledException("msg", ""); | ||
| var mi = typeof(BacktraceUnhandledException) |
| [Test] | ||
| public void SymbolLessModuleLine_ParsesAddressAndLibrary_NoThrow() | ||
| { | ||
| var f = Parse("0x00007ffad7723088 (UnityPlayer)"); |
| targetPlatform: ${{ matrix.targetPlatform }} | ||
| projectPath: ${{ matrix.projectPath }}/ | ||
| allowDirtyBuild: true | ||
| allowDirtyBuild: true |
There was a problem hiding this comment.
yup allowDirtyBuild & testMode cannot co-exist. also allowDirtyBuild shouldn't have been used here.
| using System.Text; | ||
| using UnityEngine.TestTools; | ||
|
|
||
| namespace Backtrace.Unity.Tests.Runtime{ |
There was a problem hiding this comment.
this file is not formatted
There was a problem hiding this comment.
It's a draft to review the native parsing logic;
Formatting, var names and other cosmetics will be fixed later.
I only need your thoughts on Parsing for now ~
| using UnityEngine.TestTools; | ||
|
|
||
| namespace Backtrace.Unity.Tests.Runtime{ | ||
| public class NativeParserTests |
There was a problem hiding this comment.
Can we be more specific? I would call it NativeStackTraceParser instead
| //methodname index should be greater than 0 AND '(' should be before ')' | ||
| if (methodNameEndIndex < 1 && frameString[methodNameEndIndex - 1] != '(') | ||
| // we require a '(' that appears before this ')' | ||
| int openParenIndex = frameString.LastIndexOf('(', methodNameEndIndex); |
| /// Try to convert native stack frame | ||
| /// Try to safely convert a native stack frame string into a Backtrace stack frame. | ||
| /// Handles frames with or without symbols (e.g. "0xADDR (Module)" or "0xADDR (Module) Symbol"). | ||
| /// Prevents ArgumentOutOfRangeException by validating index ranges and allowing empty symbols. |
There was a problem hiding this comment.
whats the point of adding this info to the summary?
| FunctionName = string.Empty, | ||
| Line = 0 |
There was a problem hiding this comment.
Are you sure you need to set them?
There was a problem hiding this comment.
Yes, needed to guarantee consistent defaults for native frames (especially when the parser exits early). It avoids nulls and keeps the frame type marked as Native even for empty input.
There was a problem hiding this comment.
yeah but why we should avoid nulls? I would say null in this case gives us important information - do not set entry in the json field. Are you sure symbolication process later will handle it properly?
There was a problem hiding this comment.
Solid point, exactly what I needed!
I’ll keep StackFrameType = Native but stop forcing FunctionName = "" and Line = 0.
For frames we can’t parse I’ll leave FunctionName null and only set Line when found.
2 participants
Summary
When Unity full stack traces include native frames that end after the module
(for example: 0x00007ffad7723088 (UnityPlayer))the SDK crashes withArgumentOutOfRangeException.This PR makes native frame parsing safe, ensures all string operations are bounds-checked, and adds unit tests to confirm the fix.
Changes
ConvertStackFrames
appears beforebefore parsing.SetNativeStackTraceInformation : Added parsing logic to handle:
[file:line]suffixes.Tests Added NativeParserTests:
ref: BT-5953