[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

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

@@ -1,7 +1,9 @@
 using System;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
+using System.Globalization;
 using System.Reflection;
+using System.Text;
 
 using StackTraceElement = Java.Lang.StackTraceElement;
 
@@ -37,6 +39,81 @@ public static JavaProxyThrowable Create (Exception innerException)
 			return proxy;
 		}
 
+		(int lineNumber, string? methodName, string? className) GetFrameInfo (StackFrame? managedFrame, MethodBase? managedMethod)
+		{
+			string? methodName = null;
+			string? className = null;
+
+			if (managedFrame == null) {
+				if (managedMethod != null) {
+					methodName = managedMethod.Name;
+					className = managedMethod.DeclaringType?.FullName;
+				}
+
+				return (-1, methodName, className);
+			}
+
+			int lineNumber = -1;
+			lineNumber = managedFrame.GetFileLineNumber ();
+			if (lineNumber == 0) {
+				// -2 means it's a native frame
+				lineNumber = managedFrame.HasNativeImage () ? -2 : -1;
+			}
+
+			if (managedMethod != null) {
+				// If we have no line number information and if it's a managed frame, add the
+				// IL offset.
+				if (lineNumber == -1 && managedFrame.HasILOffset ()) {
+					methodName = $"{managedMethod.Name} + 0x{managedFrame.GetILOffset():x}";
+				} else {
+					methodName = managedMethod.Name;
+				}
+
+				return (lineNumber, methodName, managedMethod.DeclaringType?.FullName);
+			}
+
+			string frameString = managedFrame.ToString ();
+			var sb = new StringBuilder ();
+
+			// We take the part of the returned string that stretches from the beginning to the first space character
+			// and treat it as the method name.
+			// https://github.com/dotnet/runtime/blob/18c3ad05c3fc127c3b7f37c49bc350bf7f8264a0/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs#L15-L55
+			int pos = frameString.IndexOf (' ');
+			string? fullName = null;
+			if (pos > 1) {
+				fullName = frameString.Substring (0, pos);
+			}
+
+			if (!String.IsNullOrEmpty (fullName) && (pos = fullName.LastIndexOf ('.')) >= 1) {
+				className = pos + 1 < fullName.Length ? fullName.Substring (pos + 1) : null;
+				fullName = fullName.Substring (0, pos);
+			}
+
+			if (!String.IsNullOrEmpty (fullName)) {
+				sb.Append (fullName);
+			} else if (managedFrame.HasNativeImage ()) {
+				// We have no name, so we'll put the native IP
+				nint nativeIP = managedFrame.GetNativeIP ();
+				sb.Append (CultureInfo.InvariantCulture, $"Native 0x{nativeIP:x}");
+			}
+
+			if (sb.Length > 0) {
+				// We will also append information native offset information, if available and only if we
+				// have recorded any previous information, since the offset without context is useless.
+				int nativeOffset = managedFrame.GetNativeOffset ();
+				if (nativeOffset != StackFrame.OFFSET_UNKNOWN) {
+					sb.Append (" + ");
+					sb.Append (CultureInfo.InvariantCulture, $"0x{nativeOffset:x}");
+				}
+			}
+
+			if (sb.Length > 0) {
+				methodName = sb.ToString ();
+			}
+
+			return (lineNumber, methodName, className);
+		}
+
 		void TranslateStackTrace ()
 		{
 			// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
@@ -61,20 +138,22 @@ void TranslateStackTrace ()
 				// ..but ignore
 			}
 
-
 			StackFrame[] frames = trace.GetFrames ();
 			int nElements = frames.Length + (javaTrace?.Length ?? 0);
 			StackTraceElement[] elements = new StackTraceElement[nElements];
 
+			const string Unknown = "Unknown";
 			for (int i = 0; i < frames.Length; i++) {
 				StackFrame managedFrame = frames[i];
 				MethodBase? managedMethod = StackFrameGetMethod (managedFrame);
 
+				// https://developer.android.com/reference/java/lang/StackTraceElement?hl=en#StackTraceElement(java.lang.String,%20java.lang.String,%20java.lang.String,%20int)
+				(int lineNumber, string? methodName, string? declaringClass) = GetFrameInfo (managedFrame, managedMethod);
 				var throwableFrame = new StackTraceElement (
-					declaringClass: managedMethod?.DeclaringType?.FullName,
-					methodName: managedMethod?.Name,
+					declaringClass: declaringClass ?? Unknown,
+					methodName: methodName ?? Unknown,
 					fileName: managedFrame?.GetFileName (),
-					lineNumber: managedFrame?.GetFileLineNumber () ?? -1
+					lineNumber: lineNumber
 				);
 
 				elements[i] = throwableFrame;

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

@@ -39,13 +39,13 @@ public void InnerExceptionIsSet ()
 				ex = e;
 			}
 
-			using (Java.Lang.Throwable proxy = CreateJavaProxyThrowable (ex))
-			using (var source = new Java.Lang.Throwable ("detailMessage", proxy))
-			using (var alias  = new Java.Lang.Throwable (source.Handle, JniHandleOwnership.DoNotTransfer)) {
-				CompareStackTraces (ex, proxy);
-				Assert.AreEqual ("detailMessage", alias.Message);
-				Assert.AreSame (ex, alias.InnerException);
-			}
+			using Java.Lang.Throwable proxy = CreateJavaProxyThrowable (ex);
+			using var source = new Java.Lang.Throwable ("detailMessage", proxy);
+			using var alias  = new Java.Lang.Throwable (source.Handle, JniHandleOwnership.DoNotTransfer);
+
+			CompareStackTraces (ex, proxy);
+			Assert.AreEqual ("detailMessage", alias.Message);
+			Assert.AreSame (ex, alias.InnerException);
 		}
 
 		void CompareStackTraces (Exception ex, Java.Lang.Throwable throwable)
@@ -61,10 +61,21 @@ void CompareStackTraces (Exception ex, Java.Lang.Throwable throwable)
 				var mf = managedFrames[i];
 				var jf = javaFrames[i];
 
-				Assert.AreEqual (mf.GetMethod ()?.Name,                   jf.MethodName, $"Frame {i}: method names differ");
+				// Unknown line locations are -1 on the Java side if they're managed, -2 if they're native
+				int managedLine = mf.GetFileLineNumber ();
+				if (managedLine == 0) {
+					managedLine = mf.HasNativeImage () ? -2 :  -1;
+				}
+
+				if (managedLine > 0) {
+					Assert.AreEqual (mf.GetMethod ()?.Name,                   jf.MethodName, $"Frame {i}: method names differ");
+				} else {
+					string managedMethodName = mf.GetMethod ()?.Name ?? String.Empty;
+					Assert.IsTrue (jf.MethodName.StartsWith ($"{managedMethodName} + 0x"), $"Frame {i}: method name should start with: '{managedMethodName} + 0x'");
+				}
 				Assert.AreEqual (mf.GetMethod ()?.DeclaringType.FullName, jf.ClassName,  $"Frame {i}: class names differ");
 				Assert.AreEqual (mf.GetFileName (),                       jf.FileName,   $"Frame {i}: file names differ");
-				Assert.AreEqual (mf.GetFileLineNumber (),                 jf.LineNumber, $"Frame {i}: line numbers differ");
+				Assert.AreEqual (managedLine,                             jf.LineNumber, $"Frame {i}: line numbers differ");
 			}
 		}
 	}