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.

Sign up for GitHub

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

Print linking instructions for NativeLib=static #103960

Closed
wants to merge 8 commits into from
Closed

Print linking instructions for NativeLib=static #103960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a908b4e
Print linking instructions for NativeLib=static
am11 Jun 25, 2024
9c1fa98
Use %var% syntax on windows
am11 Jun 25, 2024
5d14ca9
Format
am11 Jun 25, 2024
4aab2a3
Fix windows quoting issues
am11 Jun 25, 2024
c1d5ce4
Generate flatten list
am11 Jun 26, 2024
b84ccd3
Prefix linker opt with -Wl
am11 Jun 26, 2024
486b0f1
Merge dotnet/main into feature/nativeaot/static-link-instructions
am11 Jun 27, 2024
57ce66f
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Nativ…
am11 Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,25 @@
Condition="'$(_symbolIsFile)' == 'false' and Exists('$(_symbolSourcePath)')" />
</Target>

<Target Name="_WriteLinkingInstructions" AfterTargets="_CopyAotSymbols"
Condition="'$(NativeLib)' == 'Static' and '$(SkipLinkingInstructions)' != 'true'">

<ItemGroup>
<FileAsset Include="$(PublishDir)\$(TargetName)$(NativeBinaryExt)" />
<FileAsset Include="@(CustomLinkerArg)" Condition="Exists('%(CustomLinkerArg.Identity)') and '%(CustomLinkerArg.ExcludeFromStaticInstructions)' != 'true'" />
<LinkOption Include="@(CustomLinkerArg)" Condition="!Exists('%(CustomLinkerArg.Identity)') and '%(CustomLinkerArg.ExcludeFromStaticInstructions)' != 'true'" />
Comment on lines +135 to +137
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would make sense to lean into the linker argument separation we started for Xamarin/MAUI in #88294 (comment) and #89916. The separation should already be there on mac and we might need to put a couple things into different ItemGroup on Linux.

Basically instead of doing a heuristic filtering on CustomLinkerArg, we'd compose these lists from the ~5 properties and ItemGroups.

This heuristic would also work, but we'll probably end up with many ExcludeFromStaticInstructions exclusions (Linux has more switches that are in the unncessary/potentially problematic bucket - and even on mac, we'd probably want to exclude rpath, for example).

Copy link
Member Author

@am11 am11 Jun 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have tested it on both mac and linux and tried the resultant lists usage with eval cc myglue.o -o myexe $(cat dist/sources.txt) $(cat dist/linkoptions.txt) using this program. It links with clang on mac and linux and gcc on linux drivers and bfd linker on linux (lld runs into #70277 (comment), I'll investigate it separately). Both on linux and mac I'm getting:

Hello from C# Add!
Hello from C callback!
sum is: 3

These options are ok to make linked app just work. If they want to customize it further, like generating a static binary out of the NativeLib=Static, they will need to set StaticICULinking, StaticOpenSslLinking etc. and pass -static to link options. While we can't predict and automate for all kinds of use-cases (this is just a head-start for user), we can definitely minimize this set to bare minimum and filter out the rest to let user make their own choices.

The problem with separate lists is that LinkerArg is something used in the wild. Users have their own options set. That's why I thought of using this inference. If we are going to split them and document the "sources" vs. "options" msbuild items, that would be great, but as it stands, I couldn't think of better way.

Copy link
Member Author

Choose a reason for hiding this comment

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

lld runs into #70277 (comment), I'll investigate it separately

I take it back. To use lld, we actually need to publish with -p:LinkerFlavor=lld (so we get -Wl,-T,"obj/Release/net9.0/linux-musl-arm64/native/sections.ld" in linkoptions.txt).

Note that if user published with LinkerFlavor=lld and then decided to use bfd to link with their own code, it gives syntax error (all linkers have seem to invented their own layout script format and there is no consensus). So they will need to just drop the -Wl,-T line.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would make sense to lean into the linker argument separation we started for Xamarin/MAUI

I think this is a good point. Do we believe that Xamarin/MAUI can switch over to this scheme? They are the real-world use for static linking. If they are not, it suggests that this is not the best design. It would be best to avoid creating and maintaining multiple recipes for static linking.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jkotas do we have a publicaly visible link where they are using NativeLib=static? Maybe they are wrapping it under mono library-mode and bypassing the BuildIntegration?

Also, for classification, we would need to consider <LinkerArg in user's csproj where this approach seems to be working.

Copy link
Member

Choose a reason for hiding this comment

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

Why is static library reducing size compared to .o? Everything that we emit into .o is expected to be necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Archive .a is the static library format on Unix-like systems and a well-established standard for distributing compiled code. Whereas, .o is the intermediate form, object file. The linker knows how to read indexed symbol table out of .a for efficient linkage. With NativeLib=static, user expects and gets a static library.

Copy link
Member

Choose a reason for hiding this comment

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

I do not think that anybody should be distributing these static libraries. It is too fragile system to be distributed and it does not behave like a regular static library (e.g. you cannot link two these to a project - they would collide).

This is only meant to be used locally and tightly integrated into some larger project build.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should stop calling this "static libraries" to avoid implying that the output behaves like a regular static library.

Copy link
Member Author

Choose a reason for hiding this comment

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

e.g. you cannot link two these to a project - they would collide

That problem also exists with object files and the solution is the same; don't use multiple of them and put all code in a single compilation unit or pass -Wl,--allow-multiple-definition to the linker.

</ItemGroup>

<WriteLinesToFile File="$([MSBuild]::NormalizePath('$(MSBuildProjectDirectory)', '$(PublishDir)', 'sources.txt'))"
Lines="@(FileAsset)"
Overwrite="true" />

<WriteLinesToFile File="$([MSBuild]::NormalizePath('$(MSBuildProjectDirectory)', '$(PublishDir)', 'linkoptions.txt'))"
Lines="@(LinkOption)"
Overwrite="true" />

<Message Importance="High" Text="Linking helpers 'sources.txt' and 'linkoptions.txt' written to $([MSBuild]::NormalizePath('$(MSBuildProjectDirectory)', '$(PublishDir)'))." />

</Target>

</Project>
2 changes: 1 addition & 1 deletion src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ The .NET Foundation licenses this file to you under the MIT license.
</PropertyGroup>

<ItemGroup Condition="'$(UseLdClassicXCodeLinker)' != 'false' and '$(_IsApplePlatform)' == 'true'">
<CustomLinkerArg Condition="'$(_XcodeVersion)' &gt;= '15'" Include="-ld_classic" />
<CustomLinkerArg Condition="'$(_XcodeVersion)' &gt;= '15'" Include="-Wl,-ld_classic" />
</ItemGroup>

<PropertyGroup>
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,12 +331,12 @@ The .NET Foundation licenses this file to you under the MIT license.
DependsOnTargets="$(LinkNativeDependsOn)">

<ItemGroup>
<CustomLinkerArg Include="&quot;$(NativeObject)&quot;" />
<CustomLinkerArg Include="-o &quot;$(NativeBinary)&quot;" Condition="'$(_targetOS)' != 'win'" />
<CustomLinkerArg Include="/OUT:&quot;$(NativeBinary)&quot;" Condition="'$(_targetOS)' == 'win'" />
<CustomLinkerArg Include="&quot;$(NativeObject)&quot;" ExcludeFromStaticInstructions="true" />
<CustomLinkerArg Include="-o &quot;$(NativeBinary)&quot;" Condition="'$(_targetOS)' != 'win'" ExcludeFromStaticInstructions="true" />
<CustomLinkerArg Include="/OUT:&quot;$(NativeBinary)&quot;" Condition="'$(_targetOS)' == 'win'" ExcludeFromStaticInstructions="true" />
<CustomLinkerArg Include="/DEF:&quot;$(ExportsFile)&quot;" Condition="'$(_targetOS)' == 'win' and $(ExportsFile) != ''" />
<CustomLinkerArg Include="/LIBPATH:&quot;%(AdditionalNativeLibraryDirectories.Identity)&quot;" Condition="'$(_targetOS)' == 'win' and '@(AdditionalNativeLibraryDirectories->Count())' &gt; 0" />
<CustomLinkerArg Include="-exported_symbols_list &quot;$(ExportsFile)&quot;" Condition="'$(_IsApplePlatform)' == 'true' and '$(ExportsFile)' != ''" />
<CustomLinkerArg Include="-Wl,-exported_symbols_list,&quot;$(ExportsFile)&quot;" Condition="'$(_IsApplePlatform)' == 'true' and '$(ExportsFile)' != ''" />
<CustomLinkerArg Include="-exported_symbols_list /dev/null" Condition="'$(OutputType)' == 'exe' and '$(_IsApplePlatform)' == 'true' and '$(ExportsFile)' == ''" />
<CustomLinkerArg Include="-Wl,--version-script=$(ExportsFile)" Condition="'$(_targetOS)' != 'win' and '$(_IsApplePlatform)' != 'true' and '$(ExportsFile)' != ''" />
<CustomLinkerArg Include="-Wl,--export-dynamic" Condition="'$(_targetOS)' != 'win' and '$(_IsApplePlatform)' != 'true' and '$(ExportsFile)' != ''" />
Expand Down
Loading