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

NativeAOT produces static library that makes ld crash #88032

Closed
rolfbjarne opened this issue Jun 26, 2023 · 13 comments
Closed

NativeAOT produces static library that makes ld crash #88032

rolfbjarne opened this issue Jun 26, 2023 · 13 comments
Assignees
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented Jun 26, 2023

Description

NativeAOT produces static library that makes ld crash.

Reproduction Steps

This is rather complicated, it would mean checking out the branch here: xamarin/xamarin-macios#18489, and building the monotouch-test test for NativeAOT locally. If needed, I can help with that, but hopefully I can give enough info to avoid having to do this.

Repro for the ld crash with the binaries already built from NativeAOT: https://www.dropbox.com/scl/fi/ptop654nghf40cwmd8t0r/repro.zip?dl=0&rlkey=1mub7w2c94dfayvrkfvc51vn6

Just execute ./test.sh to reproduce.

The zip file also contains a binlog and the rsp file for ILC.

Crash report for ld: https://gist.github.com/rolfbjarne/acd543f3386341192258ca96aa368541

Expected behavior

No crash.

Actual behavior

Segmentation fault: 11 ld ...

Regression?

Yes.

This started happening in this maestro bump: xamarin/xamarin-macios#18489

Which has this range for dotnet/runtime: 76da696...3195fbb

Known Workarounds

Don't pass -dead_strip to the native linker.

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 26, 2023
@ghost
Copy link

ghost commented Jun 26, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

NativeAOT produces static library that makes ld crash.

Reproduction Steps

This is rather complicated, it would mean checking out the branch here: xamarin/xamarin-macios#18489, and building the monotouch-test test for NativeAOT locally. If needed, I can help with that, but hopefully I can give enough info to avoid having to do this.

Repro for the ld crash with the binaries from NativeAOT: https://www.dropbox.com/scl/fi/ptop654nghf40cwmd8t0r/repro.zip?dl=0&rlkey=1mub7w2c94dfayvrkfvc51vn6

Just execute ./test.sh to reproduce.

The zip file also contains a binlog and the rsp file for ILC.

Crash report for ld: https://gist.github.com/rolfbjarne/acd543f3386341192258ca96aa368541

Expected behavior

No crash.

Actual behavior

Segmentation fault: 11 ld ...

Regression?

Yes.

This started happening in this maestro bump: xamarin/xamarin-macios#18489

Which has this range for dotnet/runtime: 76da696...3195fbb

Known Workarounds

Don't pass -dead_strip to the native linker.

Configuration

No response

Other information

No response

Author: rolfbjarne
Assignees: -
Labels:

untriaged, area-NativeAOT-coreclr

Milestone: -

@rolfbjarne
Copy link
Member Author

rolfbjarne commented Jun 26, 2023

Looks like this regression is not in the P6 branch, only main (it doesn't reproduce in this PR: xamarin/xamarin-macios#18497)

That means it's probably somewhere in this range: edfe491...3195fbb

@rolfbjarne
Copy link
Member Author

We got another bump, and the crash is gone, so seems like this was fixed.

@rolfbjarne rolfbjarne closed this as not planned Won't fix, can't repro, duplicate, stale Jun 26, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 26, 2023
@ivanpovazan
Copy link
Member

I am reopening this issue as I have experienced the same native linker crash when building MAUI Podcast app with NativeAOT using Xamarin https://github.com/xamarin/xamarin-macios/tree/release/8.0.1xx-preview6 branch.

Additionally, I confirm that the workaround of not passing -dead_strip to the native linker solves the issue.

I will try to provide more context, but it seems the issue is triggered when compiling bigger applications.

/cc: @filipnavara @akoeplinger

@ivanpovazan ivanpovazan reopened this Jul 5, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 5, 2023
@filipnavara
Copy link
Member

I did a simple test on empty app (dotnet new ios) to see how much effect the -dead_strip option has on reducing the final executable size.

With -dead_strip: 12_146_968
Without -dead_strip: 12_476_144
Diff: 329_176 / 2.7%

This is basically the upper limit on the savings. It will be less for real-world app that uses more of the runtime features. I think it's acceptable to disable -dead_strip for now and ship that way.

I also diffed the list of preserved symbols. I noticed that there's plenty of code in both the NativeAOT and Xamarin runtime that's never going to be used. We should eventually go through the list and remove the unused methods entirely. In the case of Xamarin runtime some of the unused methods date back all the way to legacy Xamarin/Mono and they are never used on .NET 6+ (eg. xamarin_timezone_* APIs). In the case of NativeAOT runtime there's a big unused chunk of llvm-libunwind. Then there's mono_wasi_load_icu_data and other "peanuts" that may be worth fixing eventually but probably won't make a big size difference.

@ivanpovazan
Copy link
Member

ivanpovazan commented Jul 11, 2023

With -dead_strip: 12_146_968
Without -dead_strip: 12_476_144
Diff: 329_176 / 2.7%

Measuring the effect on the dotnet new maui app:

With -dead_strip: 39_614_336
Without -dead_strip: 39_769_312
Diff: 154_976 / 0,39%

dalexsoto pushed a commit to xamarin/xamarin-macios that referenced this issue Jul 13, 2023
#18553)

This PR disables passing `-dead_strip` to the native linker in case of
NativeAOT runtime to prevent build failures.

Additionally, this change affects the size of the application in the
following way - measured with `dotnet new maui` app version
`8.0.0-preview.7.23359.1`:

| MAUI iOS | -dead_strip | no -dead_strip | diff (b) | diff (Kb) | diff
(%) |
|----------|--------------|----------|----------|-----------|----------|
| .ipa (b) | 13377583 | 13435276 | 57693 | 57,693 | 0,43% |
| Size on disk (b) | 41883897 | 42038873 | 154976 | 154,976 | 0,37% |
| binary (b) | 39614336 | 39769312 | 154976 | 154,976 | 0,39% |

Even though the size of the application regresses, with this change we
have a more stable product.

Finally, once this PR gets merged we can open a tracking issue to solve
the size regression either by fixing:
- dotnet/runtime#88032 or
- by manually removing the dead code as proposed by @filipnavara here:
dotnet/runtime#88032 (comment)

--- 
Fixes: #18552
@MichalStrehovsky
Copy link
Member

Looking at https://opensource.apple.com/source/cctools/cctools-622.5.1/RelNotes/CompilerTools.html?txt it seems like object files need to be recompiled with toolchains that understand the dead_strip optimization. I wonder if the problem is that LLVM is generating the object file in a way that looks compatible with dead_strip and then it isn't because we don't know what the proprietary magic sauce is.

I assume the dead_strip is getting passed on Xamarin side since I don't see it in our .targets.

@MichalStrehovsky MichalStrehovsky added this to the 9.0.0 milestone Aug 2, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 2, 2023
@ivanpovazan
Copy link
Member

ivanpovazan commented Aug 2, 2023

I assume the dead_strip is getting passed on Xamarin side since I don't see it in our .targets.

Yes, that is correct and as a workaround it got disabled via: xamarin/xamarin-macios#18553

However, the end goal is to be able to use the flag during linking (tracked by: xamarin/xamarin-macios#18605) as it can have a significant impact with projects that use third-party bindings. /cc: @rolfbjarne

@rolfbjarne
Copy link
Member Author

as it can have a significant impact with projects that use third-party bindings.

Correct. In one case I saw an app go from >1gb to 40mb by using -dead_strip.

@MichalStrehovsky MichalStrehovsky added the os-ios Apple iOS label Aug 2, 2023
@ghost
Copy link

ghost commented Aug 2, 2023

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

NativeAOT produces static library that makes ld crash.

Reproduction Steps

This is rather complicated, it would mean checking out the branch here: xamarin/xamarin-macios#18489, and building the monotouch-test test for NativeAOT locally. If needed, I can help with that, but hopefully I can give enough info to avoid having to do this.

Repro for the ld crash with the binaries already built from NativeAOT: https://www.dropbox.com/scl/fi/ptop654nghf40cwmd8t0r/repro.zip?dl=0&rlkey=1mub7w2c94dfayvrkfvc51vn6

Just execute ./test.sh to reproduce.

The zip file also contains a binlog and the rsp file for ILC.

Crash report for ld: https://gist.github.com/rolfbjarne/acd543f3386341192258ca96aa368541

Expected behavior

No crash.

Actual behavior

Segmentation fault: 11 ld ...

Regression?

Yes.

This started happening in this maestro bump: xamarin/xamarin-macios#18489

Which has this range for dotnet/runtime: 76da696...3195fbb

Known Workarounds

Don't pass -dead_strip to the native linker.

Configuration

No response

Other information

No response

Author: rolfbjarne
Assignees: -
Labels:

os-ios, area-NativeAOT-coreclr

Milestone: 9.0.0

@MichalStrehovsky
Copy link
Member

I've added the os-ios label to get this off our team's .NET 8 queries since this looks iOS specific - I'll leave this up to you if this is .NET 8 or 9. It's unlikely our team will have time to look at mystery linker crashes that require a mac to repro. If I was investigating this, I would start with why the linker even thinks the objects produced by ILC are eligible for dead_strip. It's possible LLVM is injecting something into the object files on our behalf. It wouldn't be first such thing.

@filipnavara
Copy link
Member

I am not necessarily sure that the dead-stripability of the object file is what matters here (the algorithm in ld64 is recursive and just runs out of stack space). That said, it is controlled by the MH_DEAD_STRIPPABLE_DYLIB flag in the Mach-O header. I won't be able to check until next week if we set it or not.

rolfbjarne pushed a commit to xamarin/xamarin-macios that referenced this issue Jul 3, 2024
…ault (#20796)

### Description

NativeAOT now properly marks all symbols as non-deadstrippable in the object file it generates: dotnet/runtime@d9a6607
And uses `-dead_strip` as the default build option.

In this PR we are doing the same - reenabling the `-dead_strip` as the default platform linker switch.
This reverts previous workaround introduced in: f212f6b 

### Size savings

| MySingleView | Main | This PR | diff (%) |
|--------------|------|---------|----------|
| SOD (MB)     | 2,87 | 2,70    | -5,99%   |
|,ipa (MB)    | 1,22 | 1,17    | -4,61%   |

Even though this brings `4-5%` size reduction for a MySingleView app, the size savings are not proportional to the app size, as the savings are coming from stripping native libraries only. 
Based on measurements reported in dotnet/runtime@d9a6607 the actual expected savings for a MAUI app are around `~1.5%`.

---
Fixes #18605 and dotnet/runtime#88032

---------

Co-authored-by: Ivan Povazan <[email protected]>
@ivanpovazan
Copy link
Member

This does not seem to be reproducible anymore according to: xamarin/xamarin-macios#20796
I am closing this issue for now and we will keep tracking if the crashes start appearing again.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

No branches or pull requests

4 participants