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

[wasm] fix invariant linking error #49492

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

lewing
Copy link
Member

@lewing lewing commented Mar 11, 2021

Disable the conditional inclusion to fix #48847 until we have a solution.

@lewing lewing added arch-wasm WebAssembly architecture area-Build-mono size-reduction Issues impacting final app size primary for size sensitive workloads labels Mar 11, 2021
@ghost
Copy link

ghost commented Mar 11, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Disable the conditional inclusion to fix #48847 until we have a solution.

Author: lewing
Assignees: -
Labels:

arch-wasm, area-Build-mono, size-reduction

Milestone: -

@lewing
Copy link
Member Author

lewing commented Mar 11, 2021

cc @CoffeeFlux

@directhex
Copy link
Member

To confirm, this is the fix for:

        [stdout]   error: undefined symbol: u_errorName (referenced by top-level compiled C/C++ code)
        [stdout]   error: undefined symbol: udata_setCommonData (referenced by top-level compiled C/C++ code)
        [stdout]   error: undefined symbol: ulocdata_getCLDRVersion (referenced by top-level compiled C/C++ code)

?

@lewing
Copy link
Member Author

lewing commented Mar 11, 2021

To confirm, this is the fix for:

        [stdout]   error: undefined symbol: u_errorName (referenced by top-level compiled C/C++ code)
        [stdout]   error: undefined symbol: udata_setCommonData (referenced by top-level compiled C/C++ code)
        [stdout]   error: undefined symbol: ulocdata_getCLDRVersion (referenced by top-level compiled C/C++ code)

?

yes

@vargaz
Copy link
Contributor

vargaz commented Mar 11, 2021

Adding these files to the link line is actually not increasing the size of the final executable if its not used, so it looks harmless.

@lewing
Copy link
Member Author

lewing commented Mar 11, 2021

Would reopen #43689 I verified the symbols are linked out when they aren't referenced

@lewing
Copy link
Member Author

lewing commented Mar 11, 2021

The errors is because the symbols are still reachable somehow, this will fix that problem. When the symbols are unreachable they get properly removed without this so it does no good. #49499

@@ -259,8 +259,12 @@
<_WasmRuntimePackNativeLibs Include="libSystem.Native.a"/>
<_WasmRuntimePackNativeLibs Include="libSystem.IO.Compression.Native.a"/>
<_WasmRuntimePackNativeLibs Include="libmono-profiler-aot.a"/>
<!-- these can't conditional until we've solved the linking issue
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to put a link to what "the linking issue" is here, so if someone reads this they know what it is talking about.

Copy link
Member Author

Choose a reason for hiding this comment

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

I verified the code is linked out when the build is correct so forcing them to be conditional here is only an optimization. #49499 is the issue tracking what might be breaking the build in CI.

@steveisok
Copy link
Member

Android device legs are backed up. Rest of the runs look good.

@steveisok steveisok merged commit 1d12854 into dotnet:main Mar 11, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 11, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wasm] AOT app build breaks with InvariantGlobalization=true
7 participants