Skip to content

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

Merged
matouskozak merged 2 commits intodotnet:mainfrom
matouskozak:aot-mark-no-version-modules-unusable
Aug 7, 2024
Merged

[mono] Mark AOT modules unusable if no AOT version is found#106026
matouskozak merged 2 commits intodotnet:mainfrom
matouskozak:aot-mark-no-version-modules-unusable

Conversation

@matouskozak
Copy link
Member

@matouskozak matouskozak commented Aug 6, 2024 •
edited
Loading

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

@matouskozak matouskozak self-assigned this Aug 6, 2024
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Aug 6, 2024

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

matouskozak commented Aug 6, 2024

/azp run runtime-extra-platforms

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

azure-pipelines bot commented Aug 6, 2024

Azure Pipelines successfully started running 1 pipeline(s).

lambdageek
lambdageek approved these changes Aug 6, 2024
View reviewed changes
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

This was referenced Aug 6, 2024
Open
Open
Closed
Closed
Closed
@matouskozak @lambdageek
Update src/mono/mono/mini/aot-runtime.c
cf141d3 
Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com>
@matouskozak
Copy link
Member Author

matouskozak commented Aug 6, 2024

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 6, 2024

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Aug 6, 2024
Open
Closed
Closed
@matouskozak
Copy link
Member Author

matouskozak commented Aug 7, 2024

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

@matouskozak matouskozak merged commit af1b3b0 into dotnet:main Aug 7, 2024
This was referenced Aug 7, 2024
Closed
Merged
Open
jonpryor pushed a commit to dotnet/android that referenced this pull request Aug 27, 2024
@grendello
[native] Split up "regular" and AOT DSO caches (#9117)
47a3b33 
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
@matouskozak matouskozak deleted the aot-mark-no-version-modules-unusable branch October 3, 2024 13:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@steveisok steveisok steveisok approved these changes

@lambdageek lambdageek lambdageek approved these changes

Assignees

@matouskozak matouskozak

Labels

area-VM-meta-mono os-android

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matouskozak @steveisok @lambdageek