[native] Use full marshal method signature when a method is overloaded (#10433)

Fixes: #10417
Context: https://docs.oracle.com/en/java/javase/17/docs/specs/jni/design.html#resolving-native-method-names
Context: #10417 (comment)

Whenever we generate marshal methods code for overloaded Java methods, we must be sure to use full native signature
instead of the short form, even if one of the overloads doesn't have parameters.

Failure to do so results in hard-to fix subtle bugs like in case of #10417. In this instance, we had a
class (`Android.Runtime.InputStreamAdapter`) with 3 method overloads:

  * `public override int Read ();`
  * `public override int Read (byte[] bytes)`
  * `public override int Read (byte[] bytes, int offset, int length)`

The sample application failed to load an image using a Glide decoder, which called the `Read (byte[], int, int)`
overload above. It was puzzling, because the Java code clearly called the right native function and yet, it ended up in
a marshal method wrapper for the `Read ()` overload.

The cause became apparent when I checked names of the generated native functions. All of them were valid and yet the
whole was subtly broken: the `Read ()` overload used a (valid) "short form" native symbol name, which omits the part
that encodes parameter types in the symbol name. The reason for this was that the generator code saw there were no
parameters and so it output the short form, because it is the **first** symbol name looked up by JavaVM - thus it seemed
to be a good optimization at the time of implementing the marshal methods generator.

The problems began when **both** overloads were used somewhere in the application code and the `Read()` overload
was **always** found first, since it used the short form name, and thus Java never got to looking up the other overload!
Then it proceeded to call the parameter-less overload, which caused sort-of worked (as in it didn't crash) it it was far
from correct.

The fix implemented here is to always emit the `__` native symbol suffix for any overloaded methods that have no
parameters.

* Added on-device test to repro issue.

These tests should return 1024, but fail with `$(AndroidEnableMarshalMethods)`:

    08-21 11:53:13.738 16263 16289 I NUnit   : InputStreamAdapter_Read_bytes
    08-21 11:53:13.738 16263 16289 D StreamTest: StreamTest.InputStreamAdapter_Read_bytes, underlying stream type: mono.android.runtime.InputStreamAdapter
    08-21 11:53:13.745 16263 16289 E NUnit   : 	[FAIL]
    08-21 11:53:13.746 16263 16289 E NUnit   :  :   Expected: 1024
    08-21 11:53:13.746 16263 16289 E NUnit   :   But was:  0
    08-21 11:53:13.746 16263 16289 E NUnit   :    at System.IOTests.StreamTest.InputStreamAdapter_Read_bytes()

If `0` is returned, it means that `read()` was called instead of the `read(...)` overload
which reproduces the issue.

This test appears to pass with the above changes.

Co-authored-by: Jonathan Peppers <[email protected]>

Author: grendello

src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs

@@ -346,9 +346,14 @@ string MakeNativeSymbolName (MarshalMethodEntryObject entry, bool useFullNativeS
 				}
 
 				string sigParams = signature.Substring (1, sigEndIdx - 1);
-				if (sigParams.Length > 0) {
+				bool haveParams = sigParams.Length > 0;
+				if (useFullNativeSignature || haveParams) {
+					// We always append the underscores for overloaded methods, see https://github.com/dotnet/android/issues/10417#issuecomment-3210789627
+					// for the reason why
 					sb.Append ("__");
-					sb.Append (MangleForJni (sigParams));
+					if (haveParams) {
+						sb.Append (MangleForJni (sigParams));
+					}
 				}
 			}
 

tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj

@@ -105,6 +105,7 @@
     <Compile Include="System.Drawing\TypeConverterTest.cs" />
     <Compile Include="System.IO\DirectoryTest.cs" />
     <Compile Include="System.IO\DriveInfoTest.cs" />
+    <Compile Include="System.IO\StreamTest.cs" />
     <Compile Include="System.IO.Compression\GZipStreamTest.cs" />
     <Compile Include="System.Linq\LinqExpressionTest.cs" />
     <Compile Include="System.Net\ProxyTest.cs" />

tests/Mono.Android-Tests/Mono.Android-Tests/System.IO/StreamTest.cs

@@ -0,0 +1,34 @@
+using System;
+using System.IO;
+using NUnit.Framework;
+using JavaStreamTest = global::Net.Dot.Android.Test.StreamTest;
+
+namespace System.IOTests;
+
+[TestFixture]
+public class StreamTest
+{
+	[Test]
+	public void InputStreamAdapter_Read ()
+	{
+		using var stream = new MemoryStream (new byte [JavaStreamTest.BufferSize]);
+		var result = JavaStreamTest.InputStreamAdapter_Read (stream);
+		Assert.AreEqual (0, result); // First byte is 0
+	}
+
+	[Test]
+	public void InputStreamAdapter_Read_bytes ()
+	{
+		using var stream = new MemoryStream (new byte [JavaStreamTest.BufferSize]);
+		var result = JavaStreamTest.InputStreamAdapter_Read_bytes (stream);
+		Assert.AreEqual (JavaStreamTest.BufferSize, result);
+	}
+
+	[Test]
+	public void InputStreamAdapter_Read_bytes_int_int ()
+	{
+		using var stream = new MemoryStream (new byte [JavaStreamTest.BufferSize]);
+		var result = JavaStreamTest.InputStreamAdapter_Read_bytes_int_int (stream);
+		Assert.AreEqual (JavaStreamTest.BufferSize, result);
+	}
+}

tests/Mono.Android-Tests/Mono.Android-Tests/java/net/dot/android/test/StreamTest.java

@@ -0,0 +1,31 @@
+package net.dot.android.test;
+
+import android.util.Log;
+import java.io.InputStream;
+import java.io.IOException;
+
+public class StreamTest
+{
+    private static final String TAG = "StreamTest";
+    public static final int BUFFER_SIZE = 1024;
+
+    public static int InputStreamAdapter_Read(InputStream stream) throws IOException
+    {
+        Log.d(TAG, "StreamTest.InputStreamAdapter_Read, underlying stream type: " + stream.getClass().getName());
+        return stream.read();
+    }
+
+    public static int InputStreamAdapter_Read_bytes(InputStream stream) throws IOException
+    {
+        Log.d(TAG, "StreamTest.InputStreamAdapter_Read_bytes, underlying stream type: " + stream.getClass().getName());
+        byte[] buffer = new byte[BUFFER_SIZE];
+        return stream.read(buffer);
+    }
+
+    public static int InputStreamAdapter_Read_bytes_int_int(InputStream stream) throws IOException
+    {
+        Log.d(TAG, "StreamTest.InputStreamAdapter_Read_bytes_int_int, underlying stream type: " + stream.getClass().getName());
+        byte[] buffer = new byte[BUFFER_SIZE];
+        return stream.read(buffer, 0, BUFFER_SIZE);
+    }
+}