[dotnet] Prevent linking out code referenced by P/Invoke #10182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A unit test like this should be adapted: https://github.com/xamarin/xamarin-macios/blob/72c7b1ffcc5b6c2a24be53d60a97043db54dc8b2/tests/mtouch/MTouch.cs#L92-L120. |
spouliot
reviewed
Nov 30, 2020
| @@ -89,6 +89,7 @@ protected override void TryProcess () | |||
| Steps.Add (new ExtractBindingLibrariesStep ()); | |||
| Steps.Add (new RegistrarStep ()); | |||
| Steps.Add (new GenerateMainStep ()); | |||
| Steps.Add (new GenerateReferencesStep ()); | |||
There was a problem hiding this comment.
Not clear in PR but it seems to be after sweeping/cleaning.
Do check that enabling the linker result in less generated symbols (to keep around)
|
build |
spouliot
added
community
|
Build failure |
|
build |
|
Build failure Test results62 tests failed, 28 tests passed.Failed tests
|
|
build |
rolfbjarne
requested changes
Dec 1, 2020
|
Build success |
filipnavara
changed the title
rolfbjarne
requested changes
Dec 2, 2020
| case SymbolMode.Linker: | ||
| case SymbolMode.Default: | ||
| foreach (var symbol in required_symbols) { | ||
| var item = new MSBuildItem { Include = "-u" + symbol.Prefix + symbol.Name }; |
There was a problem hiding this comment.
I think you need a space here:
Suggested change
| var item = new MSBuildItem { Include = "-u" + symbol.Prefix + symbol.Name }; | |
| var item = new MSBuildItem { Include = "-u " + symbol.Prefix + symbol.Name }; |
although that might have to be treated as two different MSBuildItems.
There was a problem hiding this comment.
I actually checked it and the space is not necessary. In fact the space would create a problem because it would pass the whole thing quoted to the linker and the linker would expect the space to be part of the symbol name.
|
build |
|
Build failure Test results1 tests failed, 92 tests passed.Failed tests
|
|
The failed test is related: The output suggests that it's indeed missing |
|
Not sure how to solve it. Seems like System.Net.Security.Native is missing from tvOS builds (https://github.com/dotnet/runtime/blob/3c39a5d3b8310d9adcb906718cf985d3e40122da/src/libraries/Native/Unix/CMakeLists.txt#L224) so the failure is somewhat warranted. I can place |
Something like this: case "System.Net.Security.Native":
#if NET
if (app.Platform == ApplePlatform.tvOS)
break; // tvOS does not ship with System.Net.Security.Native due to https://github.com/dotnet/runtime/issues/####
#endifbasically file an issue and hide the problem (and add the issue to the list here: #8901) |
|
Done. You'll have to link dotnet/runtime#45535 in #8901 yourself since I cannot edit other people's issues :-) |
|
build |
|
Build success |
rolfbjarne
approved these changes
Dec 3, 2020
|
can someone from the xamarin team look at the https://github.com/xamarin/xamarin-macios/issues/10884 issue? |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes running the following code:
which resulted into call to
xamarin_IntPtr_objc_msgSend_IntPtrwhich was linked out from the native code.This adds a step to dotnet-linker that mimics what mmp/mtouch was doing. Depending on symbol mode setting it either generates: a) references.m file that contains code referencing all the P/Invoked symbols to prevent the native linker from linking them out, or b)
-u<symbol>linker flags that mark the symbols to be preserved.