Age | Commit message (Collapse) | Author |
|
* Fix error codes range to always have constant length for easier reading
* Test updates
|
|
We implement detection of methods such as IsAssignableFrom which requires marking of an InterfaceImplementation. An InterfaceImplementation is not an IMemberDefinition so I was unable to record this reflection pattern.
Changing RecognizedReflectionAccessPattern to accept an IMetadataTokenProvider allows us to record this detection.
|
|
* Remove net471 illink build
* Retarget ref package to netcoreapp3.0
* Set up package to include matching implementation
* Remove unnecessary Configurations
* Small cleanup in ILLink.Tasks packaging logic
* Rename package to Microsoft.NET.ILLink
* Fix mono build
|
|
* Add ability to turn of validation of the Kept attributes
It disables the entire AssemblyChecker, but I wanted an attribute name which is not tied to the implementation details. And currently the AssemblyChecker really only validates the Kept attributes.
* PR feedback - rename and make the attribute per-class
|
|
|
|
|
|
* Hide implementation details on public Annotations APIs
* Add more APIs
|
|
* Add Message class to output errors and warnings to the stdout.
* Add simple test case.
* Log messages through LinkerTestLogger
* Address feedback
* Add parameter to LogContainsAttribute that checks for regex match.
Remove Position and move line and column to MessageOrigin.
Remove MessageCode and use integer instead, check that code value falls into one of the known ranges and that it matches with the received category.
* Order msbuildMC params based on their importance
Remove toolname from MessageOrigin
* Remove unnecessary constructor and comparisons
* Clean up msbuild container to be universaly usable
* Update tests
* Tweak one more category
* Add more tests
* Use factory methods for MessageContainer
Update tests
* Update MessageContainerTests.cs
* Use Append instead of string concatenation
Co-Authored-By: Marek Safar <marek.safar@gmail.com>
Co-authored-by: Marek Safar <marek.safar@gmail.com>
|
|
* Add tracing reasons
This supplies a "reason" every time that we mark a piece of IL.
A "reason" is an instance of a new type, DependencyInfo, which tracks
a single source object and a flag (DependencyKind) indicating what
kind of dependency this is. Annotations.Mark now expects a
DependencyInfo, and the dependency reporting interface is extended to
report these reasons.
The mark sites within MarkStep and other steps have been updated with
specific reasons, and the corresponding Mark* methods have been
modified to take a reason that is passed through as many layers of the
marking logic as necessary until it reaches a call to
Annotations.Mark. The marking logic has been modified to more
frequently allow marking Cecil objects multiple times for different
reasons - while still ensuring that any expensive operations are done
only once per object (usually under an IsProcessed check, rather than
an IsMarked check).
In a few cases, the marking logic will process a Cecil object and mark
its dependencies, without marking the original Cecil object. This can
happen, for example, for custom attributes in some cases, or for
generic instantiations (since we only mark definitions). In these
cases, there are extra calls that report these intermediate
dependencies to the tracing interface without actually marking them.
Some of the DependencyKinds (for example AlreadyMarked) are only used
for internal tracking, and will not be reported to the dependency
tracing interface.
The set of DependencyKinds has been selected based on what seems like
an intuitive reason to report for each object, though sometimes the
choice is not obvious since the reasons are restricted to supplying a
single source object. Some interesting cases where potentially
non-obvious choices were made follow:
- When marking overrides on instantiated type, we choose as the reason
the instantiated type, rather than the base method (unless the
override removal optimization is disabled, in which case
instantiations are not taken into account when marking overrides - so
we blame the base method).
- When marking static constructors, we keep track of cases where they
are marked due to a field access or method call from another method,
and report these separately. For cases where we track enough
information to do so, this results in a more direct dependency chain
like: "method A triggers static constructor B", rather than "method A
access field B, field B has declaring type C, declaring type C has
static constructor D".
- When marking custom attributes on assemblies or modules, we record a
dependency from the assembly/module to the attribute, even though the
assembly/module may not have been marked or even recorded. Since the
attributes and their dependencies are kept regardless of what happens
to the assembly, these are conceptually entry points to the analysis.
This also includes a change that moves the reflection reporter from
LinkContext to Tracer, and extended to support multiple sinks, similar
to the dependency reporting.
I have also added assertions in a few places where I think it makes
sense to check invariants when we are running tests in Debug mode.
* Make DependencyKind arrays static, add more comments
* Number the DependencyKind enum
* Pass DependencyInfo as in parameter where it makes sense
The methods that reuse the "reason" variable were not modified.
* Use static fields for well-known DependencyInfo without source
Instead of a ctor that just sets the source to null.
* Undo ReflectionPatternRecorder changes
Move the reflection pattern recorder back to LinkContext
* Remove stack-based dependency recording
* Add DependencyKind.Custom
* Workaround for ReducedTracing
* Use -1 for DependencyKind.Custom
* Add back obsolete Mark method for monolinker build
|
|
* Feed cleanup
Switch away from blob feeds
* Add dotnet5 feed
* Add darc dependency for Microsoft.NET.Sdk.IL and upgrade to latest version
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
|
|
* Add sealer opt-in optimization
* Review feedback
* Add local cache for IsSubclassed check
* Generate verifiable metadata
* Move nested check before interface shortcut
|
|
* Add reference assembly build
This adds a new project that builds a reference assembly for some of
the surface area of the linker. The package is called illink. We
should finalize the name before releasing an official package. This
leaves a number of problems unsolved (described below), but provides
something we can use to start experimenting.
The preliminary included surface area contains public members of
BaseStep and its dependencies, in the FEATURE_ILLINK build flavor -
which notably makes the ref assembly incompatible with the monolinker
surface area. (illink has an AssemblyResolver derived from
Mono.Linker.DirectoryAssemblyResolver, whereas monolinker has one
derived from Mono.Cecil.BaseAssemblyResolver). Therefore the reference
assembly is currently designed for consumption by illink plugins, not
by monolinker plugins.
The reference assembly is built for netstandard2.0, chosen because it
is the lowest common denominator for netcoreapp3.0 and net471, the
TFMs for which we currently build Mono.Linker. The illink flavor
actually doesn't need to be built for net471 (only ILLink.Tasks does),
so we could choose a higher version of netstandard if we disabled that
build flavor.
The ref assembly package is unusual in a few ways, described below. We
may be able to change some of these quirks once we have a better
picture of how linker plugins will be produced and consumed.
- The package *does not* contain any implementation assemblies,
because it is not designed to be consumed by a project targeting a
platform TFM - rather it is only designed to be consumed
from *libraries* that will act as linker plugins. Attempting to
consume it directly from a netcoreapp3.0 project will work during
build, but will fail to locate illink.dll at runtime.
- The package *does not* have a transitive package dependency on
Mono.Cecil (nor does it include Mono.Cecil). This is because the
reference assembly is built against a submodule of the cecil sources,
so we do not have direct control over the package version. This
requires the custom step developer to add a dependency on Mono.Cecil.
To consume this package in a custom step, the custom step project
should:
- Target a netstandard version compatible with the reference
assembly's target framework (currently netstandard2.0).
- Include a dependency on Mono.Cecil compatible with the version
referenced during the linker build (currently 0:11:1:0, which
incidentally ships in the official Mono.Cecil package version
11.1). Attempting to use a higher version (the current latest release
is 11.2) will result in the linker failing to load the custom step.
* Add cecil to the package
This adds a new project which builds reference assemblies for
cecil. It should be kept in sync with the upstream cecil project
file. The cecil reference assembly is included into the illink package
alongside the illink.dll reference assembly.
Also prevent arcade from trying to upload a non-existent PDB for a ref
assembly.
* Set up ApiCompat
* Use .NET Foundation file header
* Pare down ref surface area
- Remove most members of Annotations
- Remove MarkingHelpers and references to it
- Remove protected members of LinkContext
- Remove UninitializedContextFactory and references to it
- Remove AssemblyResolver and references to it
* Fix undefined ExpectedFeedUrl error
The error was occurring because the early import of the Tools targets
was including Microsoft.DotNet.Build.Tasks.Feed.targets, which changed
the DefaultTargets to a publish-related target. Importing the Tools
targets later prevents overriding the DefaultTargets, causing building
to work as expected.
* Add back AnnotationStore ctor
Otherwise the implicitly-defined default ctor exists in the ref but
not the implementation. This was caught by ApiCompat.
* Build against official Mono.Cecil package
This adds an option to build against the submodule for local
development, but the new default is to build against the package.
* Minor project file cleanup
* Remove left-over files
* Further restrict public surface
* Build fix
* UseLocalCecil -> UseCecilPackage
* Use local cecil for monolinker build
* Remove Version from Mono.Linker projects
This will result in the version being taken from VersionPrefix
in Directory.Build.Props, matching the package version.
* Remove left-over reference to cecil ref build
* Copy package dependencies to ILLink.Tasks build output
This preserves the behavior of the ProjectReference dependencies,
which are always copied. The integration tests expect to find the
linker and cecil alongside ILLink.Tasks in the build output.
* Remove AssemblyAction
Co-authored-by: Marek Safar <marek.safar@gmail.com>
|
|
Don't assume that the test is running from the project directory. Some test runners will set the CurrentDirectory to a temporary directory.
|
|
* Add the command line option `--output-pinvokes`
* Remove unnecessary memory stream
* Address feedback
* Sort list of pinvokes, add unreachable DllImports to the tests
* Remove redundant if, change comparison order of PInvokeInfo
* Use an overload of Zip that is compatible with .NET 4.7.71
* Use StringComparison.Ordinal when comparing PInvokeInfo fields
Use List for storing PInvokeInfos
* Copy PInvokes json to tests output directory
|
|
To run tests from VS the NUnit3TestAdapter package is required, otherwise VS doesn't correctly recognize NUnit tests.
|
|
Using the CodeDomCompiler has been a constant source of windows only test failures being introduced.
I believe the reason CodeDomCompiler was still sticking around was because it was faster than roslyn but with server mode now widely supported and so many of our tests simply depending on roslyn now I don't think there is any advantage to using CodeDomCompiler anymore.
We switch to using roslyn by default a long time ago and it's been seamless so I expect making this same change upstream will be seamless as well.
Note that I did add /shared to our csc command line now. Without it, roslyn is noticeably slowing than CodeDomCompiler.
|
|
* Resolve custom steps from external assemblies
* Preserve order of custom steps.
Add `--custom-assembly` command line option.
Updater src\linker readme.
* Remove GetCustomStepParams.
* Address fedback
* Remove --custom-assembly option
* Address Marek's feedback
* Fix the usage message to use single comma for assembly path separator
* Use single comma for assembly path separator
* Update test/Mono.Linker.Tests.Cases/CommandLine/AddCustomStep.cs
Co-Authored-By: Marek Safar <marek.safar@gmail.com>
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
Co-authored-by: Marek Safar <marek.safar@gmail.com>
|
|
which allows setting the optimization for a specific assembly only
|
|
* Apply some code fixes
* Apply code fixes for linker project
* Apply code fixes to more projects
* Apply more code fixes
* Cleanup
* Add missing LangVersion
* Undo changes to shared file
* Undo using changes
|
|
- Remove Environment.Exit usage as it breaks linker as library hosting
- Add missing documentation entries
- Move custom steps insertion to the end to be deterministic
- Error out on invalid arguments with a reasonable message
- Allow omitting true value for bool like options
- Separate new_mvid and deterministic options
|
|
* Set ILLink.Tasks.Tests to be test project
* Unconditionally use arcade
* Fix ILLink.Tasks test project
- Renamed to ILLink.Tasks.IntegrationTests because it depends on the .nupkg and arcade schedules Test projects before Pack
- Fixed path to nupgk in nuget config
- Move override of Test target for NUnit project from root Directory.Build.targets. For some reason Condition doesn't work there so the target would always be overriden.
- Disabled MusicStore tests
They were already disabled in https://github.com/mono/linker/commit/04eb53822f98749bdf6ab47009ba7c269f504eae because of https://github.com/mono/linker/commit/04eb53822f98749bdf6ab47009ba7c269f504eae but looks like that got reverted somehow
* Make arcade enabled builds the default and remove special illink_ configs
Mono builds can be done with /p:MonoBuild=true
* Remove .net framework targets from netcore build
* Update ILLink.Tasks to netcore3.0
* Fix netcore condition
* More netcore3.0 updates
* Try to mimic weird arcade behaviour
* Enable test results publishing in arcade
* Remove DisableArcadeImport property and use temp folder for ILLink.Tasks test projects instead
* Fix net471 build on Unix
* Use a temp folder in the repo and write empty Directory.Build.props/targets instead
This works better with arcade temp folder overrides.
* Revert "Try to mimic weird arcade behaviour"
This reverts commit 3923c5410bf2d17073373924dc53020c91fea346.
* Force always downloading a local dotnet
We need this for the ILLink.Tasks tests.
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
|
|
|
|
|
|
* Fix a bug in the reflection pattern recorder usage
The code was written such that it would report almost all calls it didn't consider (or try to analyze) as unrecognized patterns.
This change adds the test infra to validate that no extra reflection patterns are recorded on a given test. It then uses this on two tests to validate the functionality.
The fix adds a boolean which tracks if the code is trying to analyze a given reflection pattern or not. It only reports unrecognized pattern if it did try to analyze a pattern, but didn't report any result explicitly.
* PR feedback and adding more tests
As per PR feedback added explicit reporting of unrecognized patterns in all branches. The "using" is now asserting if no result was reported (instead of reporting a generic unrecognized message).
Added many tests to improve the coverage of this code. Not all interesting code paths yet.
|
|
* Introduce IReflectionPatternRecorder to allow external recording of patterns
The Mark step performs pattern matching for some reflection methods to be able to figure out depenedncies even across reflection calls. Currently it only writes to log when it finds a pattern it can't recognize.
This change introduces a new interface IReflectionPatternRecorder which is called every time the pattern matching either sucessfuly recognizes a pattern or fails to do so. By default this is implemented to log on failure (just like before the change).
This allows additional tools to record the failures and or the successes in a different way.
Added support to test this functionality and modified a couple of tests to validate the basics.
The biggest change is in the MarkStep where the pattern matching has been refactored a little bit to fullfill a promise of always reporting success/failure for a given callsite. This is important for some consumers to be able to handle multiple call sites to the same reflection method in calling method body (without this we would have to somehow report the actual call site location which complicates things quite a bit on the consumption side).
* Fix build break on Mono
C# 7.3 doesn't support static local functions.
* Modify test attribute for pattern recognition to be strongly typed
* Fixes Activator.CreateInstance<T>() pattern matching
This pattern is not supported (recognized), so it should be reported as such. The existing code had two bugs:
- It didn't report it through the new interface
- It didn't match the CreateInstance<T> correctly and would never actually trigger
As such this effectively adds a new unrecognized pattern to the linker, but given that before it only reported these as low importance output messages it's a change I think we should take.
Adds a test for this as well.
* Refactor the reflection pattern matching per PR feedback
|
|
* Add support for fields substitutions.
* Update StubBodyWithValue.cs
* Update RemoveUnreachableBlocksStep.cs
* Tests and checks tweaks
* Add optional way to initialize stubbed fields
* Update BodySubstituterStep.cs
Co-authored-by: Marek Safar <marek.safar@gmail.com>
|
|
* Add abstraction between the linker and the dependency tracing/recording.
Adds IDependencyRecorder which separates the linker/tracer from the actual implementation which stores the dependencies.
Implements the default XML writer for the dependencies (should produce exactly the same output as before the change).
Adds test infra to test the dependency recorder directly (as a test mock) and a simple test.
Motivation: Future analyzers need to have visibility into the dependency analysis (MarkStep basically) to report what caused a given method/type to be included. We already expose this information through the dependency XML, this just adds an API to do this programatically.
* PR feedback
Use GetDefaultContext for context customization (made it instance call - virtual).
Remove any default tracer logic from Tracer and use XmlDependencyRecorder directly in places where we want XML output.
* Fix build break on Mono
|
|
We can easily replace the Jenkins build with one on Azure Pipelines now.
|
|
Two new steps have been introduced
BodySubstituterStep
This step removes any conditional blocks when the condition or conditions
are evaluated as constants. This step does not do any inlining. The conditional
logic is kept but based on the known values only one branch is always taken.
A simple example which can be detected by linker.
```c#
class C
{
static void Main ()
{
if (Result)
Console.WriteLine (); // <- this call will be marked and removed
}
static bool Result () => false;
}
```
RemoveUnreachableBlocksStep
A new command-line option called `--substitutions` allow external customization of any
methoda for assemblies which are linked. The syntax is same as for existing linker descriptor
XML files but it add way to control body modifications.
An example of XML file
```xml
<linker>
<assembly fullname="test, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null">
<type fullname="Mono.Linker.Tests.Cases.Substitutions.StubBodyWithValue">
<method signature="System.String TestMethod_1()" body="stub" value="abcd">
</method>
</type>
</assembly>
</linker>
```
The `value` attribute is optional and only required when the method stub should not
fallback to default behaviour.
Additional to `stub` value also `remove` value is supported to forcibly remove body
of the method when the method is marked. This is useful when the conditional logic
cannot be evaluated by linker and the method will be marked but never actually reached.
Applicability
Both steps can be combined to achieve the effect of externally customizing which
conditional branches can be removed during the linking.
I can illustrate that on IntPtr.Size property. With following substitution any code
that has compiled in conditional logic for 64 bits handling by checking IntPtr.Size will
be removed during linking.
```xml
<linker>
<assembly fullname="mscorlib">
<type fullname="System.IntPtr">
<method signature="System.Int32 get_Size()" body="stub" value="4">
</method>
</type>
</assembly>
</linker>
```
Implements #752
|
|
The trick is apprently adding the Configurations property.
The rest of the changes in the projects is more of a cleanup (moving general property groups up top).
Also renamed the ILLink.Tasks tests to match the folder and the product names.
VS on Windows forced rewrite of the illink.sln file unfortunately - trying to go back to the original ordering doesn't help - VS will overwrite it next time it opens it.
But it's really just order changes and project type GUIDs.
|
|
* Ignore reflection calls inside reflection implementation
Some relfection methods are implemented by calling different overloads of the same method. In these cases linker should not try to analyze the internal call.
For example:
```C#
MethodInfo GetMethod(string name) => GetMethod(name, null);
```
In this case the call to `GetMethod(name, null)` should not be analyzed and should not cause "warnings".
Added new test infra to be able to capture and validate logged messages.
* PR feedback
|
|
interfaces can be marked. First check unresolved interface type, if not found, check resolved interface type.
|
|
* forwardning -> forwarding
* furture -> future
* intefaces -> interfaces
* occurance -> occurrence
* pacakge -> package
|
|
Contributes to #153
|
|
Only added a test for when used-attrs-only is used for 2 reasons
1) That's the more interesting scenario
2) Annotating all of the compiler generated types that would survive without used-attrs-only is not possible with the current test framework abilities
|
|
|
|
|
|
|
|
We are hitting ever more complex scenarios in our linker. I'm finding that I'd rather know that the IL was valid before worrying about getting all the types, methods, etc correctly annotated with [Kept] attributes.
In our linker test framework we have extra attributes to allow for the cases to be executed and the output asserted. `InitialChecking` will be the place where we do that. Again, I'd rather see these failures first before I invest the time correctly annotating the kept attributes.
|
|
|
|
* Update dependencies from https://github.com/dotnet/arcade build 20190514.13
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19264.13
* Update dependencies from https://github.com/dotnet/arcade build 20190516.2
- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19266.2
* Sweep attributes on GenericParameters and InterfaceImplementations
This fixes test failures that show up due to NullableAttribute with
recent versions of .NET Core (https://github.com/mono/linker/pull/570)
* Increase line count limit for reduced tracing test
* Add test for sweeping attributes on GenericParameters
|
|
* Use SDK-style projects for linker and tests
* Clean up linker project file
Separate out the illink build properties from the monolinker
properties. Also disable building illink for net46 on unix/core
msbuild, which doesn't work due to missing reference assemblies. This
check is more specific than what we used to have, so that illink can
potentially be built with the mono runtime.
Building this project with "nuget restore" and "msbuild" will only
work with the default configuration because nuget restore does not set
the configuration and would see a different target framework from msbuild.
* Fix assembly title and description for illink
* Fix debug and optimization info for illink configurations
* Remove references to old illink solutions
* Update cecil submodule to latest mono/cecil
* Fix typo
* Update cecil configurations in illink.sln
* Fix cecil strongname build failure with arcade
Work around https://github.com/dotnet/arcade/issues/2321
* Remove unused configs from monolinker.sln
Also add a missing release config for cecil
* Set PublicKey and PublicKeyToken in cecil overrides
|
|
|
|
Instance method bodies do not need to be marked until an instance of the type could exist. Any instance methods that were marked on types that are never instantiated will be converted to a throw.
* A number of existing tests needed to be updated to avoid lazy body marking
* Add a new CodeOptimization to allow disabling this mechanism. Reasoning was, (1) it's simple to support an option. (2) It may be helpful if it causes problems. (3) I could see using this option to make writing certain tests easier.
* Marking of some small bodies will not be deferred. The idea is that the size cost of some methods is less than converting them to a throw. So we might as well leave them alone. See `IsWorthConvertingToThrow`
* Factored out `MarkAndCacheNotSupportedCtorString` since I need to call it from multiple places now.
* Expose the `RewriteBody*` methods in CodeRewriterStep for overriding. A few motivations for this. (1) We have a runtime that doesn't support exceptions. When we target that runtime we will need to override `ConvertToThrow` with something else. (2) I will likely override all of the `RewriteBody*` methods so that I can record which bodies are changed and hook it up to logging mechanisms that we have.
Note. This PR has a test that depends on https://github.com/mono/linker/pull/501 . That PR will need to land first and I will need to rebase after that lands. There will be 1 failing test until that happens.
|
|
* Fix MissingTargetReference on .NET Core
The test was failing because TypeForwarderMissingReference.il depends
on mscorlib without a version, and roslyn was looking for Object in
mscorlib, Version=0.0.0.0 as a result. This doesn't happen when using
csc from the command line because csc uses a different assembly
identity comparer by default: DesktopAssemblyIdentityComparer (even on
.NET Core).
* Also enable CanCompileILAssembly
* Also enable UnusedAttributeOnReturnTypeIsRemoved
|
|
This enables the unit tests to run on .NET Core. CodeDom compilation doesn't exist on core, so I've added the ability to compile in-process with Roslyn APIs. This still uses NUnit, using a recent version of NUnit package references with "dotnet test". This doesn't change how tests run on mono.
Finding the right reference assemblies for compilation is a bit different on .NET Core. As described in https://github.com/dotnet/roslyn/wiki/Runtime-code-generation-using-Roslyn-compilations-in-.NET-Core-App, they can either be found using packages (what the SDK does), or we can just compile against the implementation assemblies. In this change, I'm compiling against implementation assemblies, since that's fairly similar to what the tests do on mono, where they find them in the GAC.
For mono, the common references only included mscorlib. For .NET Core, I'm getting the common references from the "Trusted Platform Assemblies" (TPA) of the test process. As a result, package references of the test project become references during testcase compilation. Explicit references specified via `ReferenceAttribute` in a testcase are looked for alongside the host process's implementations as well (since we have no way to resolve a reference from just the filename since there is no GAC).
Other changes:
- Roslyn outputs the `DebuggableAttribute` by default but mcs does not, so when running on .NET Core, we don't check for the `DebuggableAttribute`.
- Check for core types in `System.Private.CoreLib` instead of `mscorlib`.
- There are some more edge cases that I'm in the process of cleaning up and will update soon.
At the moment, I'm trying to get the tests up and running, and I'm using plenty of compile-time conditions to modify the behavior on core. It would be nice not to have to do this - I haven't given it much thought, but maybe we could provide an abstraction that contains knowledge of the platform libraries, so that we don't have to have a hard dependency on `"mscorlib.dll"` and `"System.Private.CoreLib.dll"` everywhere? @mrvoorhe please let me know if you have any thoughts. Does the approach I'm taking so far seem sane to you?
@marek-safar
|
|
|
|
|
|
This organizes the source and test projects as follows:
- source projects go in `src/project/projectfile.csproj`
- test projects go in `test/project/projectfile.csproj`
The uniform layout of projects is part of the arcade onboarding (see https://github.com/mono/linker/issues/452).
|