Skip to content

Commit 8f28775

Browse files
grendellogrendello
committed
[Mono.Android] Prevent NullPointerException in TranslateStackTrace (#8795)
Context: #8788 (comment) Context: 1aa0ea7 The [`java.lang.StackTraceElement(String, String, String, int)`][0] constructor requires that the `declaringClass` and `methodName` parameters never be `null`. Failure to do so results in a `NullPointerException`: I DOTNET : JavaProxyThrowable: translation threw an exception: Java.Lang.NullPointerException: Declaring class is null I DOTNET : at Java.Interop.JniEnvironment.InstanceMethods.CallNonvirtualVoidMethod(JniObjectReference instance, JniObjectReference type, JniMethodInfo method, JniArgumentValue* args) I DOTNET : at Java.Interop.JniPeerMembers.JniInstanceMethods.FinishCreateInstance(String constructorSignature, IJavaPeerable self, JniArgumentValue* parameter s) I DOTNET : at Java.Lang.StackTraceElement..ctor(String declaringClass, String methodName, String fileName, Int32 lineNumber) I DOTNET : at Android.Runtime.JavaProxyThrowable.TranslateStackTrace() I DOTNET : at Android.Runtime.JavaProxyThrowable.Create(Exception innerException) I DOTNET : --- End of managed Java.Lang.NullPointerException stack trace --- I DOTNET : java.lang.NullPointerException: Declaring class is null I DOTNET : at java.util.Objects.requireNonNull(Objects.java:228) I DOTNET : at java.lang.StackTraceElement.<init>(StackTraceElement.java:71) I DOTNET : at crc6431345fe65afe8d98.AvaloniaMainActivity_1.n_onCreate(Native Method) Update `JavaProxyThrowable.TranslateStackTrace()` (1aa0ea7) so that if `StackFrame.GetMethod()` returns `null`, we fallback to: 1. Trying to extract class name and method name from [`StackFrame.ToString()`][1]: MainActivity.OnCreate() + 0x37 at offset 55 in file:line:column <filename unknown>:0:0 2. If (1) fails, pass `Unknown` for `declaringClass` and `methodName`. Additionally, the `lineNumber` parameter is now set to `-2` if we think a stack frame points to native code. [0]: https://developer.android.com/reference/java/lang/StackTraceElement#StackTraceElement(java.lang.String,%20java.lang.String,%20java.lang.String,%20int) [1]: https://github.com/xamarin/xamarin-android/pull/8758/files#r1504920023
1 parent 4cc496a commit 8f28775

File tree

2 files changed

+103
-13
lines changed

2 files changed

+103
-13
lines changed

‎src/Mono.Android/Android.Runtime/JavaProxyThrowable.cs

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
using System;
22
using System.Diagnostics;
33
using System.Diagnostics.CodeAnalysis;
4+
using System.Globalization;
45
using System.Reflection;
6+
using System.Text;
57

68
using StackTraceElement = Java.Lang.StackTraceElement;
79

@@ -37,6 +39,81 @@ public static JavaProxyThrowable Create (Exception innerException)
3739
return proxy;
3840
}
3941

42+
(int lineNumber, string? methodName, string? className) GetFrameInfo (StackFrame? managedFrame, MethodBase? managedMethod)
43+
{
44+
string? methodName = null;
45+
string? className = null;
46+
47+
if (managedFrame == null) {
48+
if (managedMethod != null) {
49+
methodName = managedMethod.Name;
50+
className = managedMethod.DeclaringType?.FullName;
51+
}
52+
53+
return (-1, methodName, className);
54+
}
55+
56+
int lineNumber = -1;
57+
lineNumber = managedFrame.GetFileLineNumber ();
58+
if (lineNumber == 0) {
59+
// -2 means it's a native frame
60+
lineNumber = managedFrame.HasNativeImage () ? -2 : -1;
61+
}
62+
63+
if (managedMethod != null) {
64+
// If we have no line number information and if it's a managed frame, add the
65+
// IL offset.
66+
if (lineNumber == -1 && managedFrame.HasILOffset ()) {
67+
methodName = $"{managedMethod.Name} + 0x{managedFrame.GetILOffset():x}";
68+
} else {
69+
methodName = managedMethod.Name;
70+
}
71+
72+
return (lineNumber, methodName, managedMethod.DeclaringType?.FullName);
73+
}
74+
75+
string frameString = managedFrame.ToString ();
76+
var sb = new StringBuilder ();
77+
78+
// We take the part of the returned string that stretches from the beginning to the first space character
79+
// and treat it as the method name.
80+
// https://github.com/dotnet/runtime/blob/18c3ad05c3fc127c3b7f37c49bc350bf7f8264a0/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs#L15-L55
81+
int pos = frameString.IndexOf (' ');
82+
string? fullName = null;
83+
if (pos > 1) {
84+
fullName = frameString.Substring (0, pos);
85+
}
86+
87+
if (!String.IsNullOrEmpty (fullName) && (pos = fullName.LastIndexOf ('.')) >= 1) {
88+
className = pos + 1 < fullName.Length ? fullName.Substring (pos + 1) : null;
89+
fullName = fullName.Substring (0, pos);
90+
}
91+
92+
if (!String.IsNullOrEmpty (fullName)) {
93+
sb.Append (fullName);
94+
} else if (managedFrame.HasNativeImage ()) {
95+
// We have no name, so we'll put the native IP
96+
nint nativeIP = managedFrame.GetNativeIP ();
97+
sb.Append (CultureInfo.InvariantCulture, $"Native 0x{nativeIP:x}");
98+
}
99+
100+
if (sb.Length > 0) {
101+
// We will also append information native offset information, if available and only if we
102+
// have recorded any previous information, since the offset without context is useless.
103+
int nativeOffset = managedFrame.GetNativeOffset ();
104+
if (nativeOffset != StackFrame.OFFSET_UNKNOWN) {
105+
sb.Append (" + ");
106+
sb.Append (CultureInfo.InvariantCulture, $"0x{nativeOffset:x}");
107+
}
108+
}
109+
110+
if (sb.Length > 0) {
111+
methodName = sb.ToString ();
112+
}
113+
114+
return (lineNumber, methodName, className);
115+
}
116+
40117
void TranslateStackTrace ()
41118
{
42119
// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
@@ -61,20 +138,22 @@ void TranslateStackTrace ()
61138
// ..but ignore
62139
}
63140

64-
65141
StackFrame[] frames = trace.GetFrames ();
66142
int nElements = frames.Length + (javaTrace?.Length ?? 0);
67143
StackTraceElement[] elements = new StackTraceElement[nElements];
68144

145+
const string Unknown = "Unknown";
69146
for (int i = 0; i < frames.Length; i++) {
70147
StackFrame managedFrame = frames[i];
71148
MethodBase? managedMethod = StackFrameGetMethod (managedFrame);
72149

150+
// https://developer.android.com/reference/java/lang/StackTraceElement?hl=en#StackTraceElement(java.lang.String,%20java.lang.String,%20java.lang.String,%20int)
151+
(int lineNumber, string? methodName, string? declaringClass) = GetFrameInfo (managedFrame, managedMethod);
73152
var throwableFrame = new StackTraceElement (
74-
declaringClass: managedMethod?.DeclaringType?.FullName,
75-
methodName: managedMethod?.Name,
153+
declaringClass: declaringClass ?? Unknown,
154+
methodName: methodName ?? Unknown,
76155
fileName: managedFrame?.GetFileName (),
77-
lineNumber: managedFrame?.GetFileLineNumber () ?? -1
156+
lineNumber: lineNumber
78157
);
79158

80159
elements[i] = throwableFrame;

‎tests/Mono.Android-Tests/System/ExceptionTest.cs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ public void InnerExceptionIsSet ()
3939
ex = e;
4040
}
4141

42-
using (Java.Lang.Throwable proxy = CreateJavaProxyThrowable (ex))
43-
using (var source = new Java.Lang.Throwable ("detailMessage", proxy))
44-
using (var alias = new Java.Lang.Throwable (source.Handle, JniHandleOwnership.DoNotTransfer)) {
45-
CompareStackTraces (ex, proxy);
46-
Assert.AreEqual ("detailMessage", alias.Message);
47-
Assert.AreSame (ex, alias.InnerException);
48-
}
42+
using Java.Lang.Throwable proxy = CreateJavaProxyThrowable (ex);
43+
using var source = new Java.Lang.Throwable ("detailMessage", proxy);
44+
using var alias = new Java.Lang.Throwable (source.Handle, JniHandleOwnership.DoNotTransfer);
45+
46+
CompareStackTraces (ex, proxy);
47+
Assert.AreEqual ("detailMessage", alias.Message);
48+
Assert.AreSame (ex, alias.InnerException);
4949
}
5050

5151
void CompareStackTraces (Exception ex, Java.Lang.Throwable throwable)
@@ -61,10 +61,21 @@ void CompareStackTraces (Exception ex, Java.Lang.Throwable throwable)
6161
var mf = managedFrames[i];
6262
var jf = javaFrames[i];
6363

64-
Assert.AreEqual (mf.GetMethod ()?.Name, jf.MethodName, $"Frame {i}: method names differ");
64+
// Unknown line locations are -1 on the Java side if they're managed, -2 if they're native
65+
int managedLine = mf.GetFileLineNumber ();
66+
if (managedLine == 0) {
67+
managedLine = mf.HasNativeImage () ? -2 : -1;
68+
}
69+
70+
if (managedLine > 0) {
71+
Assert.AreEqual (mf.GetMethod ()?.Name, jf.MethodName, $"Frame {i}: method names differ");
72+
} else {
73+
string managedMethodName = mf.GetMethod ()?.Name ?? String.Empty;
74+
Assert.IsTrue (jf.MethodName.StartsWith ($"{managedMethodName} + 0x"), $"Frame {i}: method name should start with: '{managedMethodName} + 0x'");
75+
}
6576
Assert.AreEqual (mf.GetMethod ()?.DeclaringType.FullName, jf.ClassName, $"Frame {i}: class names differ");
6677
Assert.AreEqual (mf.GetFileName (), jf.FileName, $"Frame {i}: file names differ");
67-
Assert.AreEqual (mf.GetFileLineNumber (), jf.LineNumber, $"Frame {i}: line numbers differ");
78+
Assert.AreEqual (managedLine, jf.LineNumber, $"Frame {i}: line numbers differ");
6879
}
6980
}
7081
}

0 commit comments

Comments
 (0)