According to Rolf it's fine to always add since the native linker will
figure out if it's really needed and so customers don't need to do
anything when using -all_load.
```
/Users/poupou/git/master/xamarin-macios/tools/linker/MonoTouch.Tuner/RemoveBitcodeIncompatibleCodeStep.cs(14,7): warning CS0105: The using directive for 'Xamarin.Linker' appeared previously in this namespace [/Users/poupou/git/master/xamarin-macios/tools/mtouch/mtouch.csproj]
```
The current code did not consider that overrides could be swept later,
if unused/unmarked by the linker, so we were missing opportunities to
make methods as final.
* [linker] Remove non-bitcode compatible code, and show a warning.
Remove code not currently compatible with bitcode and replace it with an
exception instead (otherwise we'll assert at runtime).
Also show a warning when we detect this.
This is quite helpful when looking at watch device test runs to filter out
failures we already know about.
This fixes point #2 in #4763.
* Improve documentation.
* Simplify linker code by using a substep.
* Fix whitespace issues.
* Improve reporting.
* Add support for reporting more than one MT2105 at the same time when making
the errors instead of warnings.
* Only report MT2105 for methods that haven't been linked away.
* Format the error message nicer for properties.
* Tweak a bit for warning tests to pass.
* Use ExceptionalSubStep to provide better error information.
* Adjust where linker warnings/errors are reported from to avoid a NullReferenceException.
* Make PreserveSmartEnumConversionsSubStep use error codes that are not
already used elsewhere: this isn't obvious at first, but all
ExceptionalSubStep errors use a range of error numbers (10 numbers from NNN0
to NNN9), so we need to reserve that entire range. In this case there were
other errors using some of the numbers of the range for PreserveSmartEnumConversionsSubStep.
* Make sure there are anchor links for all the variations of each ExceptionalSubStep error.
Commit 996d90614b fixed the use of XML files. However it did not set the flags so if it's used in both XML and in code then the dictionary is being added twice, which throws an exception like
```
error MT2102: Error processing the method 'System.Boolean SignalGo.Shared.Helpers.ReflectionHelper/d__0::MoveNext()' in the assembly 'SignalGo.Shared.dll': An item with the same key has already been added. Key: System.String System.Reflection.ParameterInfo::get_Name()
```
reference: https://github.com/xamarin/xamarin-macios/issues/4978
Rename a method that clashes with a new method in a base class to avoid a
compiler warning about hidden inherited members.
Fixes:
xamarin-macios/tools/linker/MobileSweepStep.cs(41,26): warning CS0114: 'MobileSweepStep.SweepAssembly(AssemblyDefinition)' hides inherited member 'SweepStep.SweepAssembly(AssemblyDefinition)'. To make the current member override that implementation, add the override keyword. Otherwise add the new keyword.
We want to avoid separate `mscorlib.dll` assemblies for 32/64 bits so
the architecture specific code for `n[u]int` and `nfloat` must be
preserved in both cases.
Because a single, slightly larger, assembly is much smaller than two
(slightly smaller) ones.
This is not a common situation since the extraneous preserved API are
often used in the application (or 3rd party code) so, in most cases,
a single `mscorlib.dll` was already used.
This merely close the gap for some cases, like our `link all` application
where this happened.
Improved linker to:
* Not mark protocol interfaces by the mere virtue of having a type that
implements them. This is implemented by not marking protocol interfaces when
they're implementing a class, but instead when a method implementation is
found to implement a method from a protocol interface.
* Mark the wrapper type for protocols (this allows us to remove the Protocol
attribute, since that's the link between the protocol and its wrapper type).
* Remove the [Protocol], [ProtocolMember] and [Adopts] attributes.
The static registrar still needs some of the information linked away, so a few
changes are required to make it available post linker.
* [ObjCRuntime] Add a BindingImplAttribute.
* [linker] Make ProviderToString an extension method on ICustomAttributeProvider to make it more discoverable.
* [generator] Use [BindingImpl] instead of [CompilerGenerated].
The entire diff is big (89MB), so it can't be gisted. However, most of it is
either removal of `using System.Runtime.CompilerServices;` or the change from
`[CompilerGenerated]` to `[BindingImpl (...)]` like this:
https://gist.github.com/rolfbjarne/8bfda3ed37b956d0342a1c1e9b079244
If I remove those parts of the diff, there's nothing significant left:
https://gist.github.com/rolfbjarne/4156164d6bdb1376366200394eb8a091
* [linker] Teach the linker about the new [BindingImpl] attribute.
In addition to the existing logic where the linker would optimize some
[CompilerGenerated] code (sometimes with additional requirements), it will now
also optimize all [BindingImpl (Optimizable)] code (without any additional
requirements).
* [tests] Add tests to make sure [BindingImpl (Optimizable)] works as expected.
* [linker] Check for [BindingImpl] before [CompilerGenerated] and stop checking for [CompilerGenerated] in XAMCORE_4_0.
Check for [BindingImpl] before checking for [CompilerGenerated], since the
former is more common.
Also stop checking for [CompilerGenerated] (at least to mean that code is
optimizable) in our next non-compatible evolutionary leap (XAMCORE_4_0):
* [introspection] Impl a better typo check.
* [linker] Implement a generic method of storing attributes that may be removed by the linker.
Implement a generic method of storing attributes that may be removed by the
linker, so that those attributes can be accessed after the linker has linked
them away.
* [linker] Store availability attributes using the new generic location.
* [linker] Store the CompilerGenerated attribute using the new generic location.
* [mtouch/mmp] Give users more control over optimizations, and share more code between mtouch and mmp.
1. Add an --optimize flag to mtouch/mmp that allows users to select which
optimizations to apply (or not). This makes it easier to add future
optimizations, and allow users to disable any optimization that causes
problems without having to disable many other features.
2. Share as much optimization code as possible between mtouch and mmp. This
immediately gives a benefit to mmp, which has three new optimizations only
mtouch had: NSObject.IsDirectBinding inlining, IntPtr.Size inlining and
dead code elimination.
This results in ~6kb of disk space saved for a linked Xamarin.Mac app:
* link sdk: [Debug][1], [Release][2]
* link all: [Debug][3], [Release][4]
Testing also verifies that monotouchtest ([Debug][5], [Release][6]) has not
changed size at all, which means that no default optimizations have changed
inadvertedly.
[1]: https://gist.github.com/rolfbjarne/6b731e3b5ca6170355662e6505c3d492#link-sdk-mac--debug
[2]: https://gist.github.com/rolfbjarne/6b731e3b5ca6170355662e6505c3d492#link-sdk-mac--release
[3]: https://gist.github.com/rolfbjarne/6b731e3b5ca6170355662e6505c3d492#link-all-mac--debug
[4]: https://gist.github.com/rolfbjarne/6b731e3b5ca6170355662e6505c3d492#link-all-mac--release
[5]: https://gist.github.com/rolfbjarne/6b731e3b5ca6170355662e6505c3d492#monotouchtest-iphonedebug64
[6]: https://gist.github.com/rolfbjarne/6b731e3b5ca6170355662e6505c3d492#monotouchtest-iphonerelease64
* [tools] Don't enable the IsDirectBinding optimization by default for Xamarin.Mac apps, it's not safe.
* Fix whitespace issues.
* [doc] Document optimizations.
* Officially support optimizations by adding them to the Versions.plist.
* [linker] Improve IntPtr.Size inliner + dead code eliminatior and add tests.
* Properly handle operands for the ldc_i4_s instruction (they're sbyte).
* Fix less-than condition to actually do a less-than comparison.
* Make sure to look up the bitness in the Target, not the Application, since
the Application's value will be incorrect when building fat apps (both
Is32Build and Is64Build will be true).
* Remove unnecessary checks for the IntPtr.Size inliner: this optimization
does not depend on other instructions than the IntPtr.get_Size call, so
remove the checks that verify surrounding instructions. This makes the
IntPtr.Size inliner kick in in more scenarios (such as the new tests).
* Add tests.
* [tests] Add mmp tests for optimizations.
* [tests] Fix XM optimization tests.
* [tests] Fix test build error.
Caching the result of checking whether a type is a CIFilter or not in a
dictionary makes the total time spent in the method go down to approximately a
third (11ms to 3ms) when linking monotouch-test on my machine.
Previous behavior
=================
* The linker would determine if a class is a generated binding class or not.
This was determined by the presence of a constructor taking a single IntPtr
argument, and with a [CompilerGenerated] attribute.
* If a class was not a generated binding class, then that class and all its
superclasses were marked as not optimizable (in the context of inlining the
IsDirectBinding check).
* The end result was that all classes with a [CompilerGenerated] IntPtr
constructor that weren't subclassed, were optimized (the IsDirectBinding
value was implied to be 'true').
Unfortunately this does not match how the IsDirectBinding value is actually
computed, and is in many cases quite wrong.
Background
==========
The authorative value for the IsDirectBinding value is the register attribute:
```csharp
[Register ("MyClass", true)] // the second parameter specifies the IsDirectBinding value
class MyClass : NSObject {}
```
Due to history this second parameter is called `IsWrapper` and not
`IsDirectBinding`, but it's the exact same thing.
Unfortunately looking up this attribute every time a class is instantiated is
slow (since fetching attributes is slow), so we guess this value in NSObject's
initialization: if the actual type of the object is in the platform assembly,
then we assume IsDirectBinding=true:
```csharp
IsDirectBinding = (this.GetType ().Assembly == PlatformAssembly);
```
and any subclasses in the platform assembly which is not a direct binding have
to set the correct value in their constructors.
New behavior
============
In the linker we now track three states for the IsDirectBinding value for each
class: if it can be inlined into a constant true or false, or if it has to be
checked at runtime (a nullable bool is used, and null corresponds with this
last undetermined state).
* The linker will look at the `[Register]` attribute for a class, and:
* If the type is CIFilter, store that IsDirectBinding=undetermined.
* If IsWrapper=False, store that IsDirectBinding=False for that class, and
that IsDirectBinding=undetermined for all super classes where
IsWrapper=True.
* If IsWrapper=True, tentatively assume IsDirectBinding=True in that class
unless already determined to be undetermined (which will be overruled if
a non-wrapper subclass is found later).
Results
=======
For monotouch-test, the changes are as follows:
* Classes we can now assume IsDirectBinding=true: https://gist.github.com/rolfbjarne/acd6a8cf1236562a832d6db9400afee9#file-foo-diff-L1-L25
* Classes we previously assumed incorrectly that IsDirectBinding=true: https://gist.github.com/rolfbjarne/acd6a8cf1236562a832d6db9400afee9#file-foo-diff-L27-L645
* Classes we can now assume IsDirectBinding=false: https://gist.github.com/rolfbjarne/acd6a8cf1236562a832d6db9400afee9#file-foo-diff-L647-L2281
There are also minor size improvements (in the iPhone/Debug64 configuration):
The executable is 17.632 bytes smaller:
-rwxr-xr-x 1 rolf staff 73038384 Jan 12 12:40 /Users/rolf/test/old/monotouchtest.app/monotouchtest
-rwxr-xr-x 1 rolf staff 73020752 Jan 12 12:50 /Users/rolf/test/new/monotouchtest.app/monotouchtest
Xamarin.iOS.dll is 3.072 bytes smaller (this will probably be 0 on a release (stripped) build).
-rw-r--r-- 1 rolf staff 2522624 Jan 12 12:40 /Users/rolf/test/old/monotouchtest.app/Xamarin.iOS.dll
-rw-r--r-- 1 rolf staff 2519552 Jan 12 12:50 /Users/rolf/test/new/monotouchtest.app/Xamarin.iOS.dll
CIFilter
========
There's a complication with CIFilters [1] [2], their implementation is
somewhat special, so we do not want to optimize anything for those classes to
not risk getting anything wrong.
[1] https://github.com/xamarin/xamarin-macios/pull/3055
[2] https://bugzilla.xamarin.com/show_bug.cgi?id=15465
* [linker] Refactor and simplify code optimizations.
We had several types of code optimizations, where we'd determine a particular
expression is a constant, then remove the condition and the code for any
dead branches.
Unfortunately each type of expression had its own logic to determine the
sequence of code to remove, each slightly different depending on the actual
conditional expression. This has led to bugs in the past, either when
switching between compilers (mcs/csc), or compilers have changed their emitted
code.
So implement a more generic version where each optimization just changes the
specific condition to a constant value, and then at the end we have a dead-
code-elimination pass that eliminates dead code based on those constant
conditions.
This version is also a lot more defensive than the previous code, it will bail
out without doing anything if it finds a code sequence it doesn't understand.
This also makes it easier to implement other similar code optimizations.
Simple time measuring shows no slowdown in the linker, and the size difference for monotouch-test is negligable at best:
256 bytes more for the exectuable:
-rwxr-xr-x 1 rolf staff 72890464 Dec 1 14:39 /Users/rolf/test/old/monotouchtest.app/monotouchtest
-rwxr-xr-x 1 rolf staff 72890720 Dec 1 14:44 /Users/rolf/test/new/monotouchtest.app/monotouchtest
3584 bytes less for Xamarin.iOS.dll:
-rw-r--r-- 1 rolf staff 2506240 Dec 1 14:39 /Users/rolf/test/old/monotouchtest.app/Xamarin.iOS.dll
-rw-r--r-- 1 rolf staff 2502656 Dec 1 14:44 /Users/rolf/test/new/monotouchtest.app/Xamarin.iOS.dll
I don't know why the executable grew slightly, the IL diff for Xamarin.iOS.dll
shows nothing out of the ordinary:
https://gist.github.com/rolfbjarne/33590ad651a21f9a9352ac9b97b9dc06
* [linker] NewRefcount is constantly true for Unified, so trying to inline it is useless.
* [linker] Remove redundant code.
* [linker] Remove nops at the end of methods.
It's not legal IL to have a nop as the last instruction of a method, peverify complains:
> [IL]: Error: [D:\Documentos\Rolf\Xamarin\Xamarin.iOS.dll : AVFoundation.AVAssetImageGenerator::get_AppliesPreferredTrackTransform][offset 0x0000001E] fall through end of the method without returning
so detect and remove nops at the end of methods.
Also add a sanity check to ensure we don't remove nops that are targets for
branch instructions (should never happen, but it doesn't hurt to be safe).
PEVerify shows much fewer errors now, [1721][1] vs [2581][2], although the [IL diff][3]
is bigger (due to the removed nops).
Updated timing measurements show no slowdown in the linker, and the size
difference for monotouch-test is negligable:
32 bytes less for the exectuable:
-rwxr-xr-x 1 rolf staff 72890880 Jan 12 00:54:49 2018 /Users/rolf/test/old/monotouchtest.app/monotouchtest*
-rwxr-xr-x 1 rolf staff 72890848 Jan 12 00:56:57 2018 /Users/rolf/test/new/monotouchtest.app/monotouchtest*
12288 bytes less for Xamarin.iOS.dll (this is a debug build, a release build
would probably not show any difference at all):
-rw-r--r-- 1 rolf staff 2506240 Jan 12 00:54:49 2018 /Users/rolf/test/old/monotouchtest.app/Xamarin.iOS.dll
-rw-r--r-- 1 rolf staff 2493952 Jan 12 00:56:57 2018 /Users/rolf/test/new/monotouchtest.app/Xamarin.iOS.dll
[1]: https://gist.github.com/rolfbjarne/9c8fa519a6f8e1718b125472f05ded07#file-peverify-new-txt-L1726
[2]: https://gist.github.com/rolfbjarne/9c8fa519a6f8e1718b125472f05ded07#file-peverify-old-txt-L2586
[3]: https://gist.github.com/rolfbjarne/35d5bfec1809ad2a80a7781cebe5b574
Don't assume that a marked method has a caller, since a method can be marked
from XML as well.
This fixes a NullReferenceException:
> error : Could not link assemblies.
> Type: `System.Reflection.RuntimeParameterInfo`
> Assembly: `mscorlib, Version=2.0.5.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e`
> Reason: Object reference not set to an instance of an object
> --- inner exception
> System.NullReferenceException: Object reference not set to an instance of an object (Aufgaben-ID: 165)
> at MonoTouch.Tuner.MonoTouchMarkStep.MarkMethod (Mono.Cecil.MethodReference reference) [0x0004f] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MonoTouch.Tuner/MonoTouchMarkStep.cs:105
> at Mono.Linker.Steps.MarkStep.MarkMethodCollection (System.Collections.IEnumerable methods) [0x00017] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:1163
> at Mono.Linker.Steps.MarkStep.MarkMethods (Mono.Cecil.TypeDefinition type) [0x0000b] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:1157
> at Xamarin.Linker.Steps.MobileMarkStep.MarkMethods (Mono.Cecil.TypeDefinition type) [0x0000b] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MobileMarkStep.cs:123
> at Mono.Linker.Steps.MarkStep.ApplyPreserveInfo (Mono.Cecil.TypeDefinition type) [0x0004a] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:1102
> at Mono.Linker.Steps.MarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x001ee] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:607
> at Xamarin.Linker.Steps.MobileMarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x00001] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MobileMarkStep.cs:71
> at Xamarin.Linker.Steps.CoreMarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x00046] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/CoreMarkStep.cs:156
> at MonoTouch.Tuner.MonoTouchMarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x00001] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MonoTouch.Tuner/MonoTouchMarkStep.cs:84
> at Mono.Linker.Steps.MarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x0007d] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:566
> at Xamarin.Linker.Steps.MobileMarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x00001] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MobileMarkStep.cs:71
> at Xamarin.Linker.Steps.CoreMarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x00046] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/CoreMarkStep.cs:156
> at MonoTouch.Tuner.MonoTouchMarkStep.MarkType (Mono.Cecil.TypeReference reference) [0x00001] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MonoTouch.Tuner/MonoTouchMarkStep.cs:84
> at Mono.Linker.Steps.MarkStep.InitializeType (Mono.Cecil.TypeDefinition type) [0x0005b] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:94
> at Mono.Linker.Steps.MarkStep.InitializeAssembly (Mono.Cecil.AssemblyDefinition assembly) [0x00025] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:81
> at Mono.Linker.Steps.MarkStep.Initialize () [0x00016] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:73
> at Mono.Linker.Steps.MarkStep.Process (Mono.Linker.LinkContext context) [0x00008] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker.Steps/MarkStep.cs:66
> at Xamarin.Linker.Steps.MobileMarkStep.Process (Mono.Linker.LinkContext context) [0x00001] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MobileMarkStep.cs:33
> at Xamarin.Linker.Steps.CoreMarkStep.Process (Mono.Linker.LinkContext context) [0x00017] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/CoreMarkStep.cs:26
> at MonoTouch.Tuner.MonoTouchMarkStep.Process (Mono.Linker.LinkContext context) [0x0001d] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/linker/MonoTouch.Tuner/MonoTouchMarkStep.cs:36
> at Mono.Linker.Pipeline.Process (Mono.Linker.LinkContext context) [0x00023] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/external/linker/linker/Mono.Linker/Pipeline.cs:128
> at MonoTouch.Tuner.Linker.Process (MonoTouch.Tuner.LinkerOptions options, MonoTouch.Tuner.MonoTouchLinkContext& context, System.Collections.Generic.List`1[Mono.Cecil.AssemblyDefinition]& assemblies) [0x000e0] in /Users/builder/data/lanes/5481/2f8bbec0/source/xamarin-macios/tools/mtouch/Tuning.cs:82
https://bugzilla.xamarin.com/show_bug.cgi?id=60176
Only apply smart enum conversion to enums, and downgrade related warnings to log messages.
This avoids spurious warnings like this:
> warning MT4124: Invalid BindAsAttribute found on 'Bindings.Test.ObjCRegistrarTest.GetBooleanArray': could not find the smart extension type System.BooleanExtensions. Please file a bug report at https://bugzilla.xamarin.com
when the BindAs attribute is valid, but just not about a smart enum in the first place.
Prevents these little gems:
> warning MT4124: Invalid BindAsAttribute found on 'Mono.Cecil.MethodReturnType': could not find the smart extension type ...
Fix collecting required internal symbols which aren't in the objc_msgSend
family by not bailing out early for a function which isn't in the objc_msgSend
family.
Also add a test.
When we process P/Invokes to add support for exception marshaling, we may
change P/Invokes to be __Internal. This means that we need to move the check
for __Internal P/Invokes to after processing P/Invokes for exception
marshaling.
https://bugzilla.xamarin.com/show_bug.cgi?id=57833
* [mtouch] Improve how we make sure native symbols aren't stripped away. Fixes#51710 and #54417.
* Refactor required symbol collection to store more information about each
symbol (field, function, Objective-C class), and in general make the code
more straight forward.
* Implement support for generating source code that references these symbols,
and do this whenever we can't ask the native linker to keep these symbols
(when using bitcode). Additionally make it possible to do this manually, so
that the source code can be generated for non-bitcode platforms too (which
is useful if the number of symbols is enormous, in which case we might
surpass the maximum command-line length).
* Also make it possible to completely ignore native symbols, or ignore them on
a per-symbol basis. This provides a fallback for users if we get something
right and we try to preserve something that shouldn't be preserved (for
instance if it doesn't exist), and the user ends up with unfixable linker
errors.
* Don't collect Objective-C classes unless they're in an assembly with
LinkWith attributes. We don't need to preserve Objective-C classes in any
other circumstances.
* Implement everything for both Xamarin.iOS and Xamarin.Mac, and share the
code between them.
* Remove previous workaround for bug #51710, since it's no longer needed.
* Add tests.
https://bugzilla.xamarin.com/show_bug.cgi?id=54417https://bugzilla.xamarin.com/show_bug.cgi?id=51710
* [mtouch] Make sure to only keep symbols from the current app when code sharing.
This fixes a build problem with the interdependent-binding-projects test when
testing in Today Extension mode.
`csc` without `/optimize` generates different IL, including additional
and not required load and store instructions. We previously ignored them
but that could leave the stack unbalanced leading to runtime exceptions.
We are now nop'ing the extraneous instructions (same as the rest of the
unneeded branch code)
This fixes#53872 [1] and also the known parts of #56209 (the other parts
are still NEEDINFO and could, possibly, be unrelated).
[1] https://bugzilla.xamarin.com/show_bug.cgi?id=53872
[2] https://bugzilla.xamarin.com/show_bug.cgi?id=56209#c2
a. System.Net.Http.Primitives.dll is user code *and* contains type
forwarders (it's like a facade) to another facade assembly,
System.Net.Primitives.dll, that ships with the SDK;
b. The former, System.Net.Http.Primitives.dll, is not processed by
the linker, e.g. no code is removed and the assembly cannot be deleted.
However we save back (as much as we can [1]) the result of any type
being resolved;
c. It also means the later, System.Net.Primitives.dll, is fully linked
and (in many cases) can be removed from the final application (as it's
mostly forwarders).
d. This means the final, re-saved, System.Net.Http.Primitives.dll binary
could point to non-existing metadata, i.e. the removed
System.Net.Primitives.dll, because of [1].
Because we resolve (and save) the forwarders *and* because we do not
allow code downloads or generation (Apple restriction) it is possible to
remove the forwarders, which will fix the issue for XI.
[1] The scope of exported types cannot be updated
abb4e902da/Mono.Cecil/ExportedType.cs (L41)
There is also a enhancement bug, #11165, about this but it predated our
PCL support and the resolve-n-save that we now do for forwarders. This
is now _fully_ fixed.
References:
* https://bugzilla.xamarin.com/show_bug.cgi?id=11165 (enhancement)
* https://bugzilla.xamarin.com/show_bug.cgi?id=51805
The native linker treats object files (.o) and static libraries (.a files,
which are archives of .o files) differently.
The native linker will always include object files into the executable:
$ echo "void xxx () {}" > foo.m
$ clang -c foo.m -o foo.o -arch x86_64
$ ld foo.o -dylib -o foo.dylib -macosx_version_min 10.12 -arch x86_64
$ nm foo.dylib
0000000000000fe0 T _xxx
However, if the object file is inside a static library:
$ echo "void xxx () {}" > foo.m
$ clang -c foo.m -o foo.o -arch x86_64
$ ar cru foo.a foo.o
$ ld foo.a -dylib -o foo.dylib -macosx_version_min 10.12 -arch x86_64
$ nm foo.dylib
<no output>
This means that our testing library (libtest.a) which is a fat library of
_object files_, do not show the problems reported in bug #51548.
So:
a) I've fixed the creation of libtest.a to be a fat library of _static
libraries_. This causes the `FastDev_LinkWithTest` test to fail exactly
like in bug #51548.
b) I've made mtouch pass `-u <native symbol>` to the native linker, for every
native symbol referenced in a managed assembly, when creating a dylib.
Amazingly this seems to work fine even with symbols to Objective-C classes
(`_OBJC_CLASS_$_<class name>`).
c) This also required adding support for collecting the Objective-C names of
all managed types registered with Objective-C to the linker. The
information is already available in the static registrar, but that would
require us to make sure the static registrar is executed before compiling
dylibs, which means those two tasks won't be able to run in parallel (also
there's no guarantee we'll even run the static registrar).
https://bugzilla.xamarin.com/show_bug.cgi?id=51548
TL&DR: This is *how* it should be done and tested, it's not complete
(single, simple case) nor the most interesting case ;-)
The trick is to make sure each case is covered by tests so a mono
_bump_ won't give us a BCL that does not conform to what the linker
expect.
What's the impact ?
1. There is the expected reduction of metadata in mscorlib. Since both
methods don't call other API there's no indirect effect (removal).
--- before 2017-01-15 11:12:44.000000000 -0500
+++ after 2017-01-15 11:12:56.000000000 -0500
@@ -13166,9 +13166,6 @@
System.Void System.Security.SecurityException::.ctor(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext)
System.Void System.Security.SecurityException::.ctor(System.String)
System.Void System.Security.SecurityException::GetObjectData(System.Runtime.Serialization.SerializationInfo,System.Runtime.Serialization.StreamingContext)
-System.Boolean System.Security.SecurityManager::CheckElevatedPermissions()
-System.Security.SecurityManager
-System.Void System.Security.SecurityManager::EnsureElevatedPermissions()
System.Security.SecurityRulesAttribute
System.Security.SecurityRuleSet System.Security.SecurityRulesAttribute::m_ruleSet
System.Void System.Security.SecurityRulesAttribute::.ctor(System.Security.SecurityRuleSet)
2. There is no visible size change (even with #1) in mscorlib.dll due to
padding (compiler /filealign)
mscorlib.dll 793,600 793,600 0 0.00 %
3. there's a *very* small reduction of mscorlib.*.aotdata size
mscorlib.armv7.aotdata 717,264 717,216 -48 -0.01 %
mscorlib.arm64.aotdata 712,840 712,704 -136 -0.02 %
AOT data *.aotdata 6,460,064 6,459,880 -184 0.00 %
4. there's no change in executable size - normal as the AOT compiler has
_likely_ already doing the same optimization (before this commit)
Executable 29,270,272 29,270,272 0 0.00 %
Full comparison: https://gist.github.com/spouliot/0464c8fa3a92b6486dfd90595d9eb718
Alexander Köplinger
Sebastien Pouliot
Rolf Bjarne Kvinge
Martin Baulig
Chris Hamons
Marek Safar