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

[mono] Mark AOT modules unusable if no AOT version is found #106026

Merged

Conversation

matouskozak
Copy link
Member

@matouskozak matouskozak commented Aug 6, 2024

Description

On Android, we might hit a collision where we instead of <libName>.dll.so try to load <libName>.so. By detecting this issue early we can mark the library as AOT unusable. We do this by checking if we were able to get the AOT file version and if not, we mark the .so file as unusable.

Full issue description #104397.

This is a fix from #104397 (comment). cc: @lambdageek

Verification

Tried locally build runtime with Xamarin Android app previously crashing due to SkiaSharp load. The crash wasn't present with this PR and the app started up correctly.

Previous behavior:

02-10 22:05:39.195 25696 25696 D monodroid-assembly: monodroid_dlopen: hash for name 'SkiaSharp.so' is 0x12e73d483788709d
02-10 22:05:39.195 25696 25696 D monodroid-assembly: monodroid_dlopen: hash match found, DSO name is 'libSkiaSharp.so'
--------- beginning of crash
02-10 22:05:39.198 25696 25696 F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x234 in tid 25696 (aSharpLoadCrash), pid 25696 (aSharpLoadCrash)

This PR:

02-11 00:32:00.684 30843 30843 D monodroid-assembly: monodroid_dlopen: hash for name 'SkiaSharp.so' is 0x12e73d483788709d
02-11 00:32:00.684 30843 30843 D monodroid-assembly: monodroid_dlopen: hash match found, DSO name is 'libSkiaSharp.so'
02-11 00:32:00.687 30843 30843 D Mono    : AOT: module SkiaSharp.so is unusable: wrong file format version (expected 186 got -1).
02-11 00:32:00.687 30843 30843 D Mono    : Assembly found with the filesystem probing logic: 'SkiaSharp'.
02-11 00:32:00.687 30843 30843 D Mono    : Requesting loading reference 0 (of 3) of SkiaSharp
02-11 00:32:00.687 30843 30843 D Mono    : Loading reference 0 of SkiaSharp (default ALC), looking for System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
02-11 00:32:00.687 30843 30843 D Mono    : Request to load System.Private.CoreLib in alc 0x7579e4bdc0
02-11 00:32:00.687 30843 30843 D Mono    : Assembly already loaded in the active ALC: 'System.Private.CoreLib'.
02-11 00:32:00.687 30843 30843 D Mono    : Assembly Ref addref SkiaSharp[0x760f7729a0] -> System.Private.CoreLib[0x760f771640]: 5

Copy link
Contributor

Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

@matouskozak
Copy link
Member Author

/azp run runtime-extra-platforms

@matouskozak matouskozak marked this pull request as ready for review August 6, 2024 17:55
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

lgtm

one minor suggestion

src/mono/mono/mini/aot-runtime.c Show resolved Hide resolved
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
@matouskozak
Copy link
Member Author

/azp run runtime-extra-platforms

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matouskozak
Copy link
Member Author

The runtime-extra-platforms failures are un-related.

@matouskozak matouskozak merged commit af1b3b0 into dotnet:main Aug 7, 2024
106 of 118 checks passed
jonpryor pushed a commit to dotnet/android that referenced this pull request Aug 27, 2024
Fixes: #9081

Context: dotnet/runtime#104397
Context: dotnet/runtime#106026
Context: #9081 (comment)
Context: c227042

("Compatibility is fun")

Consider a P/Invoke method declaration:

	[DllImport("libSkiaSharp")]
	static extern void gr_backendrendertarget_delete(IntPtr rendertarget);

Historically, when attempting to resolve this method, Mono would try
loading the following native libraries:

  * `libSkiaSharp`          (original name)
  * `libSkiaSharp.so`       (add `.so` suffix)
  * `liblibSkiaSharp`       (add `lib` prefix)
  * `liblibSkiaSharp.so`    (`lib` prefix + `.so` suffix)

.NET for Android would further permute these names, *removing* the
`lib` prefix, for attempted compatibility in case there is a P/Invoke
into `"SkiaSharp"`.

The unfortunate occasional result would be an *ambiguity*: when told
to resolve "SkiaSharp", what should we return?  The information for
`libSkiaSharp.so`, or for the *AOT'd image* of the assembly
`SkiaSharp.dll`, by way of `libaot-SkiaSharp.dll.so`?

        %struct.DSOCacheEntry {
                i64 u0x12e73d483788709d, ; from name: SkiaSharp.so
                i64 u0x3cb282562b838c95, ; uint64_t real_name_hash
                i1 false, ; bool ignore
                ptr @.DSOCacheEntry.23_name, ; name: libaot-SkiaSharp.dll.so
                ptr null; void* handle
        }, ; 71
        %struct.DSOCacheEntry {
                i64 u0x12e73d483788709d, ; from name: SkiaSharp.so
                i64 u0x43db119dcc3147fa, ; uint64_t real_name_hash
                i1 false, ; bool ignore
                ptr @.DSOCacheEntry.7_name, ; name: libSkiaSharp.so
                ptr null; void* handle
        }, ; 72

If we return the wrong thing, then the app may crash or otherwise
behave incorrectly.

Fix this by:

  * Splitting up the DSO cache into AOT-related `.so` files and
    everything else.
  * Updating `PinvokeOverride::load_library_symbol()` so that the AOT
    files are *not* consulted when resolving P/Invoke libraries.
  * Updating `MonodroidDl::monodroid_dlopen()` -- which is called by
    MonoVM via `mono_dl_fallback_register()` -- so that the AOT files
    *are* consulted *first* when resolving AOT images.

When dotnet/runtime#104397 is fixed, it will make the AOT side of the
split more efficient as we won't have to permute the shared library
name as many times as now.
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants