Skip to content
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.

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

Fix native code generation when marshal methods are disabled #7899

Merged

Conversation

grendello
Copy link
Contributor

Fix native code generation when marshal methods are disabled

Disabling marshal method generation turns off a lot of native code
generation in the MarshalMethodsNativeAssemblyGenerator class, but the
class is also responsible for outputting correctly sized cache area (an
array of X pointers) to be used by the native runtime to cache pointers
to MonoImage instances.

Native runtime trusts the managed runtime to generate correct code and,
thus, does not verify the size of generated array. This trust,
unfortunately, was broken because with marshal methods disabled, the
native code generator output cache array that was 0 entries in size,
thus leading to segfault when attempting to run the application.

Fix the issue and also parametrize one of the on-device tests to be
built twice, with marshal methods explicitly disabled and explicitly
enabled.

Disabling marshal method generation turns off a lot of native code
generation in the `MarshalMethodsNativeAssemblyGenerator` class, but the
class is also responsible for outputting correctly sized cache area (an
array of X pointers) to be used by the native runtime to cache pointers
to `MonoImage` instances.

Native runtime trusts the managed runtime to generate correct code and,
thus, does not verify the size of generated array.  This trust,
unfortunately, was broken because with marshal methods disabled, the
native code generator output cache array that was 0 entries in size,
thus leading to segfault when attempting to run the application.

Fix the issue and also parametrize one of the on-device tests to be
built twice, with marshal methods explicitly disabled and explicitly
enabled.
@@ -413,7 +413,7 @@ void AddEnvironment ()
Log
);
} else {
marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (uniqueAssemblyNames);
marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (assemblyCount, uniqueAssemblyNames);
Copy link
Member

Choose a reason for hiding this comment

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

Why is assemblyCount != uniqueAssemblyNames.Count? In what instances will assemblyCount be greater than uniqueAssemblyNames.Count? I guess UseAssemblyStore? https://github.com/xamarin/xamarin-android/blob/54a3f6e0bedc9f9b204a535aaf0f664e22285119/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs#L272-L275

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it's the assembly stores case

@jonpryor jonpryor merged commit d2ade69 into dotnet:main Mar 23, 2023
@grendello grendello deleted the fix-assembly-cache-without-marshal-methods branch March 23, 2023 20:54
jonathanpeppers pushed a commit that referenced this pull request Mar 24, 2023
… methods are disabled (#7899)

Disabling marshal method generation via
`$(AndroidEnableMarshalMethods)`=False turns off a lot of native code
generation in the `MarshalMethodsNativeAssemblyGenerator` class, but
the class is also responsible for outputting a correctly sized cache
area (an array of *X* pointers) to be used by the native runtime to
cache pointers to `MonoImage` instances.

`libmonodroid.so` trusts that `Xamarin.Android.Build.Tasks.dll` et al
will generate correct code, and thus does not verify the size of
generated array.  This trust, unfortunately, was broken because with
marshal methods disabled, the native code generator created an output
cache array that was 0 entries in size, thus leading to segfault when
attempting to run the application.

Fix the issue and also parametrize one of the on-device tests to be
built twice, with marshal methods explicitly disabled and explicitly
enabled.
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 24, 2023
* main:
  [Xamarin.Android.Build.Tasks] Fix native code generation when marshal methods are disabled (dotnet#7899)
  [ci] Optimize 'APK's .NET' test job overhead. (dotnet#7904)
  [Mono.Android] delay JNINativeWrapper.get_runtime_types() (dotnet#7913)
  Bump external/Java.Interop from `73ebad2` to `53bfb4a` (dotnet#7914)
grendello added a commit to grendello/xamarin-android that referenced this pull request Apr 6, 2023
* main: (94 commits)
  [ci] Remove remaining Classic test jobs. (dotnet#7924)
  Add Nightly Tests for Humanizer
  [readme] Add aka.ms links for d17.5. (dotnet#7935)
  [tests] Remove `net472` multitargeting (dotnet#7932)
  [monodroid] Fix `ld` build error on Nightly Builds. (dotnet#7925)
  Bump to xamarin/Java.Interop/main@0355acf (dotnet#7931)
  [tests] Use msftconnecttest.com in QuoteInvalidQuoteUrlsShouldWork (dotnet#7919)
  [ci] Don't set demands for megapipeline (dotnet#7929)
  [Mono.Android] Bind API-UpsideDownCake Developer Preview 1 (dotnet#7796)
  Bump to dotnet/installer@d109cba3ff 8.0.100-preview.4.23176.5 (dotnet#7921)
  [Xamarin.Android.Build.Tasks] Fix Android Version Code for Release builds (dotnet#7795)
  Bump to dotnet/installer@27d6769dfb 8.0.100-preview.4.23172.16 (dotnet#7910)
  [Xamarin.Android.Build.Tasks] Fix native code generation when marshal methods are disabled (dotnet#7899)
  [ci] Optimize 'APK's .NET' test job overhead. (dotnet#7904)
  [Mono.Android] delay JNINativeWrapper.get_runtime_types() (dotnet#7913)
  Bump external/Java.Interop from `73ebad2` to `53bfb4a` (dotnet#7914)
  [build] remove .NET 6 support (dotnet#7900)
  [profiled-aot] update `dotnet.aotprofile` for .NET 8 (dotnet#7908)
  [tests] Add backup ssl sites in case of 429 response. (dotnet#7909)
  $(AndroidPackVersionSuffix)=preview.4; net8 is 34.0.0-preview.4 (dotnet#7912)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to 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.

4 participants