Skip to content

Marshal methods part 4 of 5 (runtime) - #7285

Merged
grendello merged 60 commits into
dotnet:mainfrom
grendello:mm-runtime
Sep 7, 2022
Merged

Marshal methods part 4 of 5 (runtime)#7285
grendello merged 60 commits into
dotnet:mainfrom
grendello:mm-runtime

Conversation

@grendello

@grendellogrendello commented Aug 19, 2022

Copy link
Copy Markdown
Contributor

Context: dotnet/java-interop#1027

Implement the necessary runtime elements to enable running of applications with marshal methods.
This commit, if marshal methods are enabled, makes it possible to run both plain XA and MAUI apps.

The [UnmanagedCallersOnly] attribute used by marshal methods requires that the invoked method
doesn't have any non-blittable types as either its return type or type of any parameters passed
to the method. Implement generation of wrapper methods which replace bool (the only non-blittable
type currently found in any Mono.Android, AndroidX bindings or in MAUI) with byte and convert
the values appropriately before calling the actual target method. In a "hello world" MAUI test
application there are 133 such methods (out of around 180 total).

Implement code that enables us to show error messages with the proper assembly, class and method
names on failure to look up or obtain pointer to native callback methods.

The point of this commit is only to generate code and make sure it's
valid as far as compiling and linking are concerned. The code has not
been tested at run time as not all the infrastructure on the
Xamarin.Android side is implemented yet. This is on purpose, to keep PRs
smaller.
The majority of this PR introduces various classes, enums and structures
related to code generation. Support for various LLVM IR instructions is
limited only to those we actually use and only to elements of those
constructions that we use. As such, it's not a general purpose code
generator which allows us to make some assumptions and take some
shortcuts (without compromising correctness and validity of the
generated code)
Portions of the PR (the native type handling system) are to be treated
as proof-of-concept as they are not as optimized (design wise) as they
should be. The reason for this limitation is that it requires modifying
the previous LLVM IR data generation code and it would contribute to
this PR's already substantial size. The next PR in the series will take
care of that rewrite as well as it will focus on implementing the
runtime side of marshal methods, making it possible to actually run
applications which use marshal methods.
What this PR implements is the following:
* LLVM IR
* function and instruction attributes
* function parameter (declaration/definition) and argument (runtime)
handling
* function variable (including parameters) handling, including
unnamed local variables
* support for native function signatures and pointers to functions
* The ret, store, load, icmp, br, call and phi instructions
* Marshal method generator
* managed to JNI signature and symbol name translations
Implement missing runtime support so that applications are able to
actually run. Also fix issues with marshal method overloads as well as
add code to handle the case where two unrelated methods (from different
types) end up using the same JNI symbol name:
* `Java_crc64e1fb321c08285b90_CellAdapter_n_1onActionItemClicked
* `System.Boolean Android.Views.ActionMode/ICallback::OnActionItemClicked(Android.Views.ActionMode,Android.Views.IMenuItem)`
* `System.Boolean AndroidX.AppCompat.View.ActionMode/ICallback::OnActionItemClicked(AndroidX.AppCompat.View.ActionMode,Android.Views.IMenuItem)`
* main:
LEGO: Merge pull request 7221
LEGO: Merge pull request 7219
[Xamarin.Android.Build.Tasks] use `ToJniName(type, cache)` (dotnet#7211)
[docs] Synchronize with MicrosoftDocs/xamarin-docs (dotnet#7208)
[Mono.Android] Remove System.Linq usage (dotnet#7210)
Bump to Android NDK r25 (dotnet#6764)
Bump to mono/opentk@daa9b2d5 (dotnet#7192)
[Mono.Android] Optional NTLMv2 support in AndroidMessageHandler (dotnet#6999)
Bump to dotnet/installer@53587f9 7.0.100-rc.1.22374.1 (dotnet#7198)
* mm-codegen:
LEGO: Merge pull request 7221
LEGO: Merge pull request 7219
[Xamarin.Android.Build.Tasks] use `ToJniName(type, cache)` (dotnet#7211)
[docs] Synchronize with MicrosoftDocs/xamarin-docs (dotnet#7208)
[Mono.Android] Remove System.Linq usage (dotnet#7210)
Bump to Android NDK r25 (dotnet#6764)
Bump to mono/opentk@daa9b2d5 (dotnet#7192)
[Mono.Android] Optional NTLMv2 support in AndroidMessageHandler (dotnet#6999)
Bump to dotnet/installer@53587f9 7.0.100-rc.1.22374.1 (dotnet#7198)
* main:
Bump to dotnet/java-interop@a5756ca8. (dotnet#7226)
Bump `$(AndroidNet6Version)` to 32.0.447 (dotnet#7224)
* mm-codegen:
Bump to dotnet/java-interop@a5756ca8. (dotnet#7226)
Bump `$(AndroidNet6Version)` to 32.0.447 (dotnet#7224)
Also, deal with duplicate native symbol names
* main:
[Mono.Android] fix crash on startup with EnableLLVM (dotnet#7188)
[ci] Add Android Designer test template (dotnet#7227)
* mm-codegen:
[Mono.Android] fix crash on startup with EnableLLVM (dotnet#7188)
[ci] Add Android Designer test template (dotnet#7227)
* main:
[Localization] Import translated resx files (dotnet#7190)
* mm-codegen:
[Localization] Import translated resx files (dotnet#7190)
Hit a snag with one of the callbacks using non-blittable
arguments (`bool` in this case), but it seems to be the last (famous
last words) obstacle preventing the app from starting.
A handful of hacks are needed ATM, too.
* main:
[build] update to latest JDKs (dotnet#7236)
* mm-codegen:
[build] update to latest JDKs (dotnet#7236)
Unfortunately, 133 out of 182 methods are still registered dynamically,
the reason being that they either return `bool` or take it as one of
their parameters. `bool` isn't a blittable type and thus such methods
cannot be used with `[UnregisteredCallersOnly]`. To move farther, we
need to modify the generator to stop generating native callbacks (just
them) with `bool`.
* main:
[ci] Upload test assemblies after signing (dotnet#7241)
* mm-codegen:
[ci] Upload test assemblies after signing (dotnet#7241)
The app still crashes for some reason (appears something is not
decorated with the `[UnmanagedCallersOnly]` attribute) but the IL code
generation is complete and the app doesn't report any native/runtime
linking errors.
TBC next week
* main:
Bump to xamarin/monodroid@210073e1 (dotnet#7272)
[OneLoc] Localize Microsoft.Android.Templates (dotnet#7248)
[README] Add links to XA 13.0 release installers (dotnet#7251)
Bump to dotnet/installer@716bd17 7.0.100-rc.1.22409.23 (dotnet#7247)
Bump manifest-merger from 30.2.1 to 30.2.2 (dotnet#7238)
unmanagedCallersOnlyAttributes.Add (asm, CreateImportedUnmanagedCallersOnlyAttribute (asm, unmanagedCallersOnlyAttributeCtor));
}

Console.WriteLine ();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this and other Console.WriteLine()s here use Log.LogDebugMessage() instead?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Not all of them will stay. The next PR in the series cleans that up.

Comment threadsrc/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs Outdated
}
}

string ToStringOrNull (object? o)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I totally expected this method to "normalize" the empty string to null, not what this method actually does.

Also, given that this appears to be for "debug" output, why is it 'null' (with single-quotes)? Wouldn't outuput of:

method.CallbackField == null

make more sense than the current:

method.CallbackField == 'null'

?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This method will go away in the next PR in the series.


MethodDefinition GenerateBlittableWrapper (MarshalMethodEntry method, Dictionary<AssemblyDefinition, CustomAttribute> unmanagedCallersOnlyAttributes)
{
Console.WriteLine ($"\t Generating blittable wrapper for: {method.NativeCallback.FullName}");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/Console.WriteLine/Log.LogDebugMessage/g

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Most of these will go away in the next PR

@@ -13,29 +14,44 @@ namespace Xamarin.Android.Tasks
#if ENABLE_MARSHAL_METHODS
public sealed class MarshalMethodEntry

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there actually need for many of these types to be public? We're not hitting them from a separate unit test assembly, so should they be public at all?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The next PR in the series will do all kinds of cleanups like this.

Comment threadsrc/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs Outdated
foreach (IList<MarshalMethodEntry> entryList in MarshalMethods.Values) {
var allMethods = new List<MarshalMethodInfo> ();

// It's possible that several otherwise different methods (from different classes, but with the same

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there an example of this? Given that the Java_… names contain the declaring package name and declaring type name, how can two different methods "from different classes" have the same name?

Or is "name" in this context the method name, not the Java_… name?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yep, there are a few examples in the Hello Maui sample app:

Overloaded MM: Java_crc64e1fb321c08285b90_CellAdapter_n_1onActionItemClicked
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Boolean Android.Views.ActionMode/ICallback::OnActionItemClicked(Android.Views.ActionMode,Android.Views.IMenuItem))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onActionItemClicked'
Using FULL signature
mangling 'Landroid/view/ActionMode;Landroid/view/MenuItem;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onActionItemClicked__Landroid_view_ActionMode_2Landroid_view_MenuItem_2
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Boolean AndroidX.AppCompat.View.ActionMode/ICallback::OnActionItemClicked(AndroidX.AppCompat.View.ActionMode,Android.Views.IMenuItem))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onActionItemClicked'
Using FULL signature
mangling 'Landroidx/appcompat/view/ActionMode;Landroid/view/MenuItem;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onActionItemClicked__Landroidx_appcompat_view_ActionMode_2Landroid_view_MenuItem_2
Overloaded MM: Java_crc64e1fb321c08285b90_CellAdapter_n_1onCreateActionMode
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Boolean Android.Views.ActionMode/ICallback::OnCreateActionMode(Android.Views.ActionMode,Android.Views.IMenu))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onCreateActionMode'
Using FULL signature
mangling 'Landroid/view/ActionMode;Landroid/view/Menu;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onCreateActionMode__Landroid_view_ActionMode_2Landroid_view_Menu_2
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Boolean AndroidX.AppCompat.View.ActionMode/ICallback::OnCreateActionMode(AndroidX.AppCompat.View.ActionMode,Android.Views.IMenu))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onCreateActionMode'
Using FULL signature
mangling 'Landroidx/appcompat/view/ActionMode;Landroid/view/Menu;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onCreateActionMode__Landroidx_appcompat_view_ActionMode_2Landroid_view_Menu_2
Overloaded MM: Java_crc64e1fb321c08285b90_CellAdapter_n_1onDestroyActionMode
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Void Android.Views.ActionMode/ICallback::OnDestroyActionMode(Android.Views.ActionMode))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onDestroyActionMode'
Using FULL signature
mangling 'Landroid/view/ActionMode;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onDestroyActionMode__Landroid_view_ActionMode_2
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Void AndroidX.AppCompat.View.ActionMode/ICallback::OnDestroyActionMode(AndroidX.AppCompat.View.ActionMode))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onDestroyActionMode'
Using FULL signature
mangling 'Landroidx/appcompat/view/ActionMode;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onDestroyActionMode__Landroidx_appcompat_view_ActionMode_2
Overloaded MM: Java_crc64e1fb321c08285b90_CellAdapter_n_1onPrepareActionMode
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Boolean Android.Views.ActionMode/ICallback::OnPrepareActionMode(Android.Views.ActionMode,Android.Views.IMenu))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onPrepareActionMode'
Using FULL signature
mangling 'Landroid/view/ActionMode;Landroid/view/Menu;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onPrepareActionMode__Landroid_view_ActionMode_2Landroid_view_Menu_2
implemented in: Microsoft.Maui.Controls.Handlers.Compatibility.CellAdapter (System.Boolean AndroidX.AppCompat.View.ActionMode/ICallback::OnPrepareActionMode(AndroidX.AppCompat.View.ActionMode,Android.Views.IMenu))
mangling 'crc64e1fb321c08285b90/CellAdapter'
mangling 'n_onPrepareActionMode'
Using FULL signature
mangling 'Landroidx/appcompat/view/ActionMode;Landroid/view/Menu;'
new native symbol name: Java_crc64e1fb321c08285b90_CellAdapter_n_1onPrepareActionMode__Landroidx_appcompat_view_ActionMode_2Landroid_view_Menu_2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Either you updated the comment, or I horribly misread this comment; I thought you were saying that the long/full name had conflicts/collisions.

What you're showing here are overloads.

For example, consider Activity.startActivity():

You should be able to override both of these in C#:

classMyActivity:Activity{publicoverrideunsafevoidStartActivity(Android.Content.Intent?intent)=>;publicoverrideunsafevoidStartActivity(Android.Content.Intent?intent,Android.OS.Bundle?options)=>;}

This would also result in needing to use the full Java_… name. (This would likely also make a good unit test in a future commit...)

Updating the comment to use the term "method overload" would be a helpful clarification. The fact that the methods being overridden came from different classes/interfaces is only slightly relevant; the more important bit is that within the JCW type, these are method overloads, a'la:

interfaceIntFooable {
voidfoo(intvalue);
}
interfaceStringFooable {
voidfoo(Stringvalue);
}
classMyFooableimplementsIntFooable, StringFooable {
publicnativevoidfoo(intvalue) {}
publicnativevoidfoo(Stringvalue) {}
}

Here, you can't just have Java_example_MyFooable_foo, because the method is overloaded. Instead, the full names must be used.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I modified the comment by adding the examples, but the rest of the wording remained the same. The "simple" overloads you give above are handled in the classifer, the case handled here is when two different managed types will end up having the same Java package name (like the "standard" Android vs AndroidX overloads in the comment) and thus become "native" overloads, requiring native symbol name with full signature.

sb.Append ("_0");
sb.Append (((int)ch).ToString ("x04"));
} else {
sb.Append (ch);

@jonpryorjonpryorAug 22, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The problem with this is that it'll "pass through" characters which shouldn't be passed through:

In particular, Kotlin methods can contain -. (We don't support overriding such methods yet, as we use Java as an intermediary, but if we ever start using Kotlin as an intermediary, we will be able to override such methods.)

Thus, what we should instead do is allow only[a-zA-Z0-9], and escape anything else:

if((ch>='0'&&ch<='0')||(ch>='a'&&ch<='z')||(ch>='A'&&ch<='Z')){sb.Append(ch);}else{sb.Append("_0").Append(((int)ch).ToString("x04"));}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't know if restricting the allowed character set to [a-zA-Z0-9] will yield the same symbol name as computed by JNI. According to JNI docs, only non-ASCII characters are encoded in this way. I'd prefer to keep the current algorithm until we actually hit the Kotlin - problem.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would make for a fascinating thing to test. Where's a good Java bytecode assembler? I need to create a Java method which contains \n!

* main:
Bump r8 from 3.3.28 to 3.3.75 (dotnet#7292)
[Xamarin.Android.Build.Tasks] Pass `hybrid` to AOT, if enabled (dotnet#7263)
Bump to lz4/lz4@5ff83968 [v1.9.4] (dotnet#7262)
[Mono.Android] add "built-in" delegate for MAUI+non-Shell (dotnet#7267)
* main:
[xaprepare] Generate SourceLink.json (dotnet#7298)
* main:
Bump to dotnet/installer@84d26a0 7.0.100-rc.2.22419.24 (dotnet#7294)
Bump to xamarin/java.interop/main@e31d9c62 (dotnet#7266)
@jonpryor

Copy link
Copy Markdown
Contributor
Context: 903ba37ce70d2840983774e1d6fb55f8002561e2
Context: e1af9587bb98d4c249bbc392ceccc2b53ffff155
Context: https://github.com/xamarin/java.interop/issues/1027
Commit 903ba37c mentioned a TODO:
> Update/rewrite infrastructure to focus on implementing the runtime> side of marshal methods, making it possible to actually run> applications which use marshal methods.
Implement the necessary runtime elements to enable running of
applications with marshal methods. It is now possible, if
LLVM marshal methods are enabled/`ENABLE_MARSHAL_METHODS` is defined,
to run both plain .NET SDK for Android and MAUI apps.
Update `src/Microsoft.Android.Sdk.ILLink/PreserveLists/System.Runtime.InteropServices.xml`
so that `System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute`
is always preserved, as it is required by LLVM Marshal Methods.
The `[UnmanagedCallersOnly]` attribute used by marshal methods
requires that the invoked method have [blittable types][0] for the
method return type and all parameter types. Unfortunately, `bool`
is *not* a blittable type. Implement generation of wrapper methods
which replace `bool` with `byte` and convert the values appropriately
before calling the actual target method. In a "hello world" MAUI
test app there are 133 such methods (out of around 180 total).
Implement code that enables us to show error messages with the proper
assembly, class and method names on failure to look up or obtain
pointer to native callback methods.
TODO:
* Process *all* assemblies, including `Mono.Android.dll`, for
Java Callable Wrapper generation. This is necessary so that we
can find and emit LLVM marshal methods for types defined within
`Mono.Android.dll`.
* Remove the `ENABLE_MARSHAL_METHODS` define, and enable
LLVM marshal methods for everyone.
* Update `<GenerateJavaStubs/>` to rewrite all assemblies for all
Supported ABIs. Currently, we don't support `Java.Lang.Object` &
`Java.Lang.Throwable` subclasses being located in per-ABI
assemblies.
* How do `Java_…` native functions interact with
`JNIEnv::RegisterNatives()`?
https://github.com/xamarin/xamarin-android/pull/7285#discussion_r951755971**Can* JNI `native` methods contain "non-printable" characters
such as `\n`, or "non-representable in ELF symbols" characters
such as `-` (e.g. Kotlin mangled methods)?
https://github.com/xamarin/xamarin-android/pull/7285#discussion_r951789869* Cleanup, cleanup, cleanup
[0]: https://docs.microsoft.com/en-us/dotnet/framework/interop/blittable-and-non-blittable-types

* main:
[tests] disable `$(_FastDeploymentDiagnosticLogging)` in PerformanceTest (dotnet#7307)
* main:
[ci] Split up signing for files to be notarized (dotnet#7321)
[ci] Add support to net7.0 for multi-targeting in VS (dotnet#7311)
[ci] Skip classic tests on .NET release branches (dotnet#7312)
[tests] Use `$(AndroidSdkPlatformToolsVersion)`=33.0.3 (dotnet#7320)
Bump to xamarin/java.interop/main@4f813cb2 (dotnet#7308)
Bump to mono/mono.posix@d8994ca, dotnet/android-libzipsharp@98e9173 (dotnet#7309)
* main:
Bump to dotnet/installer@2d1a4de 7.0.100-rc.2.22426.5 (dotnet#7314)
[FabricBot] Apply 'needs-triage' label to new issues. (dotnet#7327)
[Mono.Android] .NET 6+ & GetHttpMessageHandler & null, oh my! (dotnet#7214)
* main:
[FabricBot] Add `possibly-stale` GitHub label for closing old issues (dotnet#7343)
Don't use Dictionary<K,V> to avoid duplicate value exception (dotnet#7340)
[One .NET] fix `dotnet run -c Release` (dotnet#7341)
* main:
Bump to dotnet/installer@330dee3 (dotnet#7334)
[xaprepare] CGManifest.json conforms to JSON schema (dotnet#7342)
@grendello

Copy link
Copy Markdown
ContributorAuthor

The test failures in the latest run don't appear to be related to this PR which contains mostly disabled code. I'm going to merge, to make it possible to move on with marshal methods.

@grendello
grendello merged commit a760281 into dotnet:mainSep 7, 2022
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 24, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@grendello@jonpryor