Skip to content

[TrimmableTypeMap] Fix UCO boolean return type mismatch causing n_* callback trimming #11168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
simonrozsival merged 3 commits into from
Apr 21, 2026
+186 −3
Merged

[TrimmableTypeMap] Fix UCO boolean return type mismatch causing n_* callback trimming #11168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1409607
[trimmable typemap] Fix UCO boolean return type mismatch causing n_* …
simonrozsival Apr 21, 2026
f89dbc3
Add tests for UCO boolean signature encoding
simonrozsival Apr 21, 2026
b6f4494
Fix duplicate XML doc summary block on EncodeClrType
simonrozsival Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/JniSignatureHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,36 @@ static JniParamKind ParseSingleType (string sig, ref int i)
}

/// <summary>
/// Encodes the CLR type for a JNI parameter kind into a signature type encoder.
/// Encodes a JNI type as its CLR equivalent for [UnmanagedCallersOnly] UCO wrapper signatures.
/// </summary>
/// <remarks>
/// JNI boolean (Z) maps to <c>byte</c> (unsigned, blittable for the JNI ABI).
/// </remarks>
public static void EncodeClrType (SignatureTypeEncoder encoder, JniParamKind kind)
{
switch (kind) {
case JniParamKind.Boolean: encoder.Byte (); break; // JNI jboolean is unsigned byte; must be blittable for UCO
case JniParamKind.Boolean: encoder.Byte (); break; // JNI jboolean is unsigned byte; blittable for UCO
case JniParamKind.Byte: encoder.SByte (); break;
Copy link
Copy Markdown
Member Author

@simonrozsival simonrozsival Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Documentation — Now that EncodeClrTypeForCallback exists with sbyte for boolean, this inline comment could be expanded to mention the dual encoding: e.g., // JNI jboolean: byte (UCO wrapper) vs sbyte (callback MemberRef). Helps future readers understand why there are two methods.

Rule: Comments explain "why", not "what" (Postmortem #59)

case JniParamKind.Char: encoder.Char (); break;
case JniParamKind.Short: encoder.Int16 (); break;
case JniParamKind.Int: encoder.Int32 (); break;
case JniParamKind.Long: encoder.Int64 (); break;
case JniParamKind.Float: encoder.Single (); break;
case JniParamKind.Double: encoder.Double (); break;
case JniParamKind.Object: encoder.IntPtr (); break;
default: throw new ArgumentException ($"Cannot encode JNI param kind {kind} as CLR type");
}
}

/// <summary>
/// Encodes a JNI type as its CLR equivalent matching the MCW-generated <c>n_*</c> callback
/// signatures. JNI boolean (Z) maps to <c>sbyte</c> (matching <c>_JniMarshal_*_B</c> delegates).
/// Use this when constructing member references to <c>n_*</c> methods.
/// </summary>
public static void EncodeClrTypeForCallback (SignatureTypeEncoder encoder, JniParamKind kind)
{
switch (kind) {
case JniParamKind.Boolean: encoder.SByte (); break; // MCW n_* callbacks use sbyte for JNI boolean
case JniParamKind.Byte: encoder.SByte (); break;
case JniParamKind.Char: encoder.Char (); break;
case JniParamKind.Short: encoder.Int16 (); break;
Expand Down
13 changes: 12 additions & 1 deletion src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@ MethodDefinitionHandle EmitUcoMethod (UcoMethodData uco)
int paramCount = 2 + jniParams.Count;
bool isVoid = returnKind == JniParamKind.Void;

// UCO wrapper signature: uses JNI ABI types (byte for boolean)
Action<BlobEncoder> encodeSig = sig => sig.MethodSignature ().Parameters (paramCount,
rt => { if (isVoid) rt.Void (); else JniSignatureHelper.EncodeClrType (rt.Type (), returnKind); },
p => {
Expand All @@ -714,8 +715,18 @@ MethodDefinitionHandle EmitUcoMethod (UcoMethodData uco)
JniSignatureHelper.EncodeClrType (p.AddParameter ().Type (), jniParams [j]);
});

// Callback member reference: uses MCW n_* types (sbyte for boolean)
Action<BlobEncoder> encodeCallbackSig = sig => sig.MethodSignature ().Parameters (paramCount,
rt => { if (isVoid) rt.Void (); else JniSignatureHelper.EncodeClrTypeForCallback (rt.Type (), returnKind); },
p => {
p.AddParameter ().Type ().IntPtr ();
p.AddParameter ().Type ().IntPtr ();
for (int j = 0; j < jniParams.Count; j++)
JniSignatureHelper.EncodeClrTypeForCallback (p.AddParameter ().Type (), jniParams [j]);
});

var callbackTypeHandle = _pe.ResolveTypeRef (uco.CallbackType);
var callbackRef = _pe.AddMemberRef (callbackTypeHandle, uco.CallbackMethodName, encodeSig);
var callbackRef = _pe.AddMemberRef (callbackTypeHandle, uco.CallbackMethodName, encodeCallbackSig);

var handle = _pe.EmitBody (uco.WrapperName,
MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.HideBySig,
Expand Down
148 changes: 148 additions & 0 deletions ...s/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,154 @@ public void Generate_AcwProxy_HasUnmanagedCallersOnlyAttribute ()
Assert.Contains (methodDefs, name => name.Contains ("_uco_"));
}

[Theory]
[InlineData (1, 0x05)] // Boolean → byte (unsigned) for JNI ABI
[InlineData (2, 0x04)] // Byte → sbyte
[InlineData (3, 0x03)] // Char → char
[InlineData (4, 0x06)] // Short → int16
[InlineData (5, 0x08)] // Int → int32
[InlineData (6, 0x0A)] // Long → int64
[InlineData (7, 0x0C)] // Float → float32
[InlineData (8, 0x0D)] // Double → float64
[InlineData (9, 0x18)] // Object → IntPtr
public void EncodeClrType_ProducesCorrectPrimitiveTypeCode (int kindValue, byte expectedCode)
{
var kind = (JniParamKind) kindValue;
var blob = new BlobBuilder ();
JniSignatureHelper.EncodeClrType (new SignatureTypeEncoder (blob), kind);
Assert.Equal (expectedCode, blob.ToArray () [0]);
}

[Theory]
[InlineData (1, 0x04)] // Boolean → sbyte — matches MCW n_* callbacks
[InlineData (2, 0x04)] // Byte → sbyte
[InlineData (3, 0x03)] // Char → char
[InlineData (4, 0x06)] // Short → int16
[InlineData (5, 0x08)] // Int → int32
[InlineData (6, 0x0A)] // Long → int64
[InlineData (7, 0x0C)] // Float → float32
[InlineData (8, 0x0D)] // Double → float64
[InlineData (9, 0x18)] // Object → IntPtr
public void EncodeClrTypeForCallback_ProducesCorrectPrimitiveTypeCode (int kindValue, byte expectedCode)
{
var kind = (JniParamKind) kindValue;
var blob = new BlobBuilder ();
JniSignatureHelper.EncodeClrTypeForCallback (new SignatureTypeEncoder (blob), kind);
Assert.Equal (expectedCode, blob.ToArray () [0]);
}

[Fact]
public void EncodeClrType_Boolean_DiffersFromCallback ()
{
var ucoBlob = new BlobBuilder ();
JniSignatureHelper.EncodeClrType (new SignatureTypeEncoder (ucoBlob), JniParamKind.Boolean);

var cbBlob = new BlobBuilder ();
JniSignatureHelper.EncodeClrTypeForCallback (new SignatureTypeEncoder (cbBlob), JniParamKind.Boolean);

var ucoBytes = ucoBlob.ToArray ();
var cbBytes = cbBlob.ToArray ();
Assert.NotEqual (ucoBytes, cbBytes);
Assert.Equal (0x05, ucoBytes [0]); // byte (unsigned)
Assert.Equal (0x04, cbBytes [0]); // sbyte (signed)
}

[Fact]
public void EncodeClrType_Void_Throws ()
{
var blob = new BlobBuilder ();
Assert.ThrowsAny<ArgumentException> (() =>
JniSignatureHelper.EncodeClrType (new SignatureTypeEncoder (blob), JniParamKind.Void));
}

[Fact]
public void EncodeClrTypeForCallback_Void_Throws ()
{
var blob = new BlobBuilder ();
Assert.ThrowsAny<ArgumentException> (() =>
JniSignatureHelper.EncodeClrTypeForCallback (new SignatureTypeEncoder (blob), JniParamKind.Void));
}

[Theory]
[InlineData (2)] // Byte
[InlineData (3)] // Char
[InlineData (4)] // Short
[InlineData (5)] // Int
[InlineData (6)] // Long
[InlineData (7)] // Float
[InlineData (8)] // Double
[InlineData (9)] // Object
public void EncodeClrType_NonBooleanTypes_IdenticalToCallback (int kindValue)
{
var kind = (JniParamKind) kindValue;
var ucoBlob = new BlobBuilder ();
JniSignatureHelper.EncodeClrType (new SignatureTypeEncoder (ucoBlob), kind);

var cbBlob = new BlobBuilder ();
JniSignatureHelper.EncodeClrTypeForCallback (new SignatureTypeEncoder (cbBlob), kind);

Assert.Equal (ucoBlob.ToArray (), cbBlob.ToArray ());
}

[Fact]
public void Generate_UcoMethod_BooleanReturn_WrapperUsesByte_CallbackUsesSByte ()
{
// Regression test: the UCO wrapper must use byte (unsigned, JNI ABI) for boolean,
// but the callback MemberRef must use sbyte (signed, MCW convention).
// A mismatch caused ILLink to fail resolving the member reference and trim n_* methods.
var peer = FindFixtureByJavaName ("my/app/TouchHandler");
using var stream = GenerateAssembly (new [] { peer }, "BoolReturnTest");
using var pe = new PEReader (stream);
var reader = pe.GetMetadataReader ();

// Find the UCO wrapper method for onTouch (returns Z → boolean)
var ucoMethod = reader.MethodDefinitions
.Select (h => reader.GetMethodDefinition (h))
.First (m => reader.GetString (m.Name).Contains ("onTouch") &&
reader.GetString (m.Name).Contains ("_uco_"));
var ucoSig = ucoMethod.DecodeSignature (SignatureTypeProvider.Instance, null);
Assert.Equal ("System.Byte", ucoSig.ReturnType);

// Find the callback MemberRef that the UCO wrapper calls (n_OnTouch on the TouchHandler type)
var callbackRef = FindCallbackMemberRef (reader, "n_OnTouch");
var callbackSig = callbackRef.DecodeMethodSignature (SignatureTypeProvider.Instance, null);
Assert.Equal ("System.SByte", callbackSig.ReturnType);
}

[Fact]
public void Generate_UcoMethod_BooleanParam_WrapperUsesByte_CallbackUsesSByte ()
{
// Regression test: boolean parameters must also use the correct encoding.
var peer = FindFixtureByJavaName ("my/app/TouchHandler");
using var stream = GenerateAssembly (new [] { peer }, "BoolParamTest");
using var pe = new PEReader (stream);
var reader = pe.GetMetadataReader ();

// Find the UCO wrapper for onFocusChange (takes Z as 3rd param → boolean parameter)
var ucoMethod = reader.MethodDefinitions
.Select (h => reader.GetMethodDefinition (h))
.First (m => reader.GetString (m.Name).Contains ("onFocusChange") &&
reader.GetString (m.Name).Contains ("_uco_"));
var ucoSig = ucoMethod.DecodeSignature (SignatureTypeProvider.Instance, null);
// Params: IntPtr (jnienv), IntPtr (self), IntPtr (View object), byte (boolean)
Assert.Equal ("System.Byte", ucoSig.ParameterTypes.Last ());

// Find the callback MemberRef
var callbackRef = FindCallbackMemberRef (reader, "n_OnFocusChange");
var callbackSig = callbackRef.DecodeMethodSignature (SignatureTypeProvider.Instance, null);
Assert.Equal ("System.SByte", callbackSig.ParameterTypes.Last ());
}

static MemberReference FindCallbackMemberRef (MetadataReader reader, string methodName)
{
var refs = Enumerable.Range (1, reader.GetTableRowCount (TableIndex.MemberRef))
.Select (i => reader.GetMemberReference (MetadataTokens.MemberReferenceHandle (i)))
.Where (m => reader.GetString (m.Name) == methodName)
Copy link
Copy Markdown
Member Author

@simonrozsival simonrozsival Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Testing — FindCallbackMemberRef matches by name only, which could become ambiguous if a fixture adds overloaded methods. Consider constraining by parent type reference for robustness. Not blocking since current fixtures have unique names.

Rule: Test assertions must be specific

.ToList ();
Assert.Single (refs);
return refs [0];
}

[Theory]
[InlineData ("()V", 0)]
[InlineData ("(I)V", 1)]
Expand Down