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

[dotnet] Prevent linking out code referenced by P/Invoke #10182

Merged
merged 6 commits into from
Dec 3, 2020
Merged

[dotnet] Prevent linking out code referenced by P/Invoke #10182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d0c18eb
[dotnet] Generate references.mm to prevent linking out code reference…
filipnavara Nov 30, 2020
9f8ccad
Address PR feedback: Namespaces.Foundation -> "Foundation."
filipnavara Nov 30, 2020
d9b782b
Address PR feedback: Simplify hasSymbols
filipnavara Nov 30, 2020
76d5c84
Reuse ListExportedSymbols step, respect App.SymbolMode
filipnavara Dec 2, 2020
7f603a4
Address PR feedback
filipnavara Dec 2, 2020
39948aa
Workaround for tvOS
filipnavara Dec 3, 2020
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
13 changes: 12 additions & 1 deletion dotnet/targets/Xamarin.Shared.Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,14 @@
<ReadItemsFromFile File="$(_LinkerItemsDirectory)/_RegistrarFile.items" Condition="Exists('$(_LinkerItemsDirectory)/_RegistrarFile.items')">
<Output TaskParameter="Items" ItemName="_RegistrarFile" />
</ReadItemsFromFile>
<!-- Load _ReferencesFile -->
<ReadItemsFromFile File="$(_LinkerItemsDirectory)/_ReferencesFile.items" Condition="Exists('$(_LinkerItemsDirectory)/_ReferencesFile.items')">
<Output TaskParameter="Items" ItemName="_ReferencesFile" />
</ReadItemsFromFile>
<!-- Load _ReferencesLinkerFlags -->
<ReadItemsFromFile File="$(_LinkerItemsDirectory)/_ReferencesLinkerFlags.items" Condition="Exists('$(_LinkerItemsDirectory)/_ReferencesLinkerFlags.items')">
<Output TaskParameter="Items" ItemName="_ReferencesLinkerFlags" />
</ReadItemsFromFile>

<ItemGroup>
<_AssembliesToAOT Include="@(ResolvedFileToPublish)" Condition="'%(Extension)' == '.dll' Or '%(Extension)' == '.exe' " />
Expand Down Expand Up @@ -467,6 +475,9 @@
<_CompileNativeExecutableFile Include="@(_RegistrarFile)">
<OutputFile>$(_IntermediateNativeLibraryDir)%(Filename).o</OutputFile>
</_CompileNativeExecutableFile>
<_CompileNativeExecutableFile Include="@(_ReferencesFile)">
<OutputFile>$(_IntermediateNativeLibraryDir)%(Filename).o</OutputFile>
</_CompileNativeExecutableFile>
<_XamarinMainIncludeDirectory Include="$(_XamarinIncludeDirectory)" />
</ItemGroup>
</Target>
Expand Down Expand Up @@ -570,7 +581,7 @@
DylibRPath="$(_DylibRPath)"
EntitlementsInExecutable="$(_CompiledEntitlements)"
Frameworks="@(_NativeExecutableFrameworks);@(_BindingLibraryFrameworks)"
LinkerFlags="@(_BindingLibraryLinkerFlags);@(_MainLinkerFlags)"
LinkerFlags="@(_BindingLibraryLinkerFlags);@(_ReferencesLinkerFlags);@(_MainLinkerFlags)"
LinkWithLibraries="@(_XamarinMainLibraries);@(_BindingLibraryLinkWith);@(_MainLinkWith)"
MinimumOSVersion="$(_MinimumOSVersion)"
ObjectFiles="@(_NativeExecutableObjectFiles)"
Expand Down
2 changes: 2 additions & 0 deletions tools/dotnet-linker/SetupStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ protected override void TryProcess ()
post_sweep_substeps.Add (new RemoveAttributesStep ());
}

Steps.Add (new ListExportedSymbols (null));
Steps.Add (new LoadNonSkippedAssembliesStep ());
Steps.Add (new ExtractBindingLibrariesStep ());
Steps.Add (new RegistrarStep ());
Steps.Add (new GenerateMainStep ());
Steps.Add (new GenerateReferencesStep ());
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Steps.Add (new GatherFrameworksStep ());

Configuration.Write ();
Expand Down
48 changes: 48 additions & 0 deletions tools/dotnet-linker/Steps/GenerateReferencesStep.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
using System.Collections.Generic;
using System.IO;
using System.Text;

using Mono.Cecil;

using Xamarin.Bundler;
using Xamarin.Linker;

namespace Xamarin {

public class GenerateReferencesStep : ConfigurationAwareStep {
protected override string Name { get; } = "Generate References";
protected override int ErrorCode { get; } = 2320;

protected override void TryEndProcess ()
{
base.TryEndProcess ();

var app = Configuration.Application;
var required_symbols = Configuration.DerivedLinkContext.RequiredSymbols;
var items = new List<MSBuildItem> ();

switch (app.SymbolMode) {
case SymbolMode.Ignore:
break;
case SymbolMode.Code:
string reference_m = Path.Combine (Configuration.CacheDirectory, "reference.m");
reference_m = Configuration.Target.GenerateReferencingSource (reference_m, required_symbols);
if (!string.IsNullOrEmpty (reference_m)) {
var item = new MSBuildItem { Include = reference_m };
items.Add (item);
}
Configuration.WriteOutputForMSBuild ("_ReferencesFile", items);
break;
case SymbolMode.Linker:
foreach (var symbol in required_symbols) {
var item = new MSBuildItem { Include = "-u" + symbol.Prefix + symbol.Name };
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

items.Add (item);
}
Configuration.WriteOutputForMSBuild ("_ReferencesLinkerFlags", items);
break;
default:
throw ErrorHelper.CreateError (99, Errors.MX0099, $"invalid symbol mode: {app.SymbolMode}");
}
}
}
}
3 changes: 3 additions & 0 deletions tools/dotnet-linker/dotnet-linker.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@
<Compile Include="..\linker\MonoTouch.Tuner\Extensions.cs">
<Link>external\tools\linker\MonoTouch.Tuner\Extensions.cs</Link>
</Compile>
<Compile Include="..\linker\MonoTouch.Tuner\ListExportedSymbols.cs">
<Link>external\tools\linker\MonoTouch.Tuner\ListExportedSymbols.cs</Link>
</Compile>
<Compile Include="..\linker\MonoTouch.Tuner\PreserveSmartEnumConversionsSubStep.cs">
<Link>external\tools\linker\MonoTouch.Tuner\PreserveSmartEnumConversionsSubStep.cs</Link>
</Compile>
Expand Down
17 changes: 15 additions & 2 deletions tools/linker/MonoTouch.Tuner/ListExportedSymbols.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
using Xamarin.Bundler;
using Xamarin.Linker;
using Xamarin.Tuner;
using Xamarin.Utils;

namespace MonoTouch.Tuner
namespace Xamarin.Linker.Steps
{
public class ListExportedSymbols : BaseStep
{
Expand All @@ -19,7 +20,11 @@ public class ListExportedSymbols : BaseStep

public DerivedLinkContext DerivedLinkContext {
get {
#if NET
return LinkerConfiguration.GetInstance (Context).DerivedLinkContext;
#else
return (DerivedLinkContext) Context;
#endif
}
}

Expand All @@ -36,8 +41,10 @@ protected override void ProcessAssembly (AssemblyDefinition assembly)
if (Annotations.GetAction (assembly) == AssemblyAction.Delete)
return;

#if !NET
if (skip_sdk_assemblies && Profile.IsSdkAssembly (assembly))
return;
#endif

if (!assembly.MainModule.HasTypes)
return;
Expand Down Expand Up @@ -116,8 +123,14 @@ void ProcessMethod (MethodDefinition method)
case "__Internal":
DerivedLinkContext.RequiredSymbols.AddFunction (pinfo.EntryPoint).AddMember (method);
break;
case "System.Native":

case "System.Net.Security.Native":
#if NET
if (DerivedLinkContext.App.Platform == ApplePlatform.TVOS)
break; // tvOS does not ship with System.Net.Security.Native due to https://github.com/dotnet/runtime/issues/45535
goto case "System.Native";
#endif
case "System.Native":
case "System.Security.Cryptography.Native.Apple":
DerivedLinkContext.RequireMonoNative = true;
DerivedLinkContext.RequiredSymbols.AddFunction (pinfo.EntryPoint).AddMember (method);
Expand Down