| Age | Commit message (Collapse) | Author |
|---|
|
|
|
* Add a test for the `<namespace>` element. There were no tests.
* Expand `CanPreserveTypesUsingRegex` to help highlight how it behaves differently from `<namespace>`
* Add more examples to the docs
|
|
methods (#2868)
Fixes #2865
Also addresses marking of all static interface methods encompassing the changes from #2859, and updates the way that all interface methods are marked. Whether or not we mark an interface method due to its base method is now separated from marking other virtual methods and the marking is postponed to ProcessMarkedTypesWithInterface. In ProcessMarkedTypesWithInterfaces, interface implementations are marked, and methods that implement a marked/implemented interface are marked. Tests for static interface methods have also been updated.
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
Serialization discovery is an undocumented quirk that exists for
back-compat in xamarin scenarios. We turned it on by default in .NET
6, but would like to remove such quirks from the default options in
.NET 7.
This changes the (undocumented) command-line option to be
opt-in. Xamarin SDKs will be expected to pass this option by setting
<_ExtraTrimmerArgs>--enable-serialization-discovery</_ExtraTrimmerArgs>.
|
|
* Created first draft of unused suppressions detection doc.
* design doc improvements
* Fix redundant suppressions design doc wording
* Fix typos
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
* Apply suggestions from code review
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Changed example and added review suggestions.
* Apply suggestions from code review
Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>
* removed implementation details
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>
|
|
This reverts commit ff646e0ba4f5b919af854ec73dcee6c4903c3def.
|
|
Enables more precise removal of static abstract interface methods and adds more tests than previously.
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
* Revert "Trim static interfaces (#2741)"
This reverts commit a073a68561a7f45a2dba0976083ee3e9873bf423.
* Revert "Don't remove MethodImpl if overridden method is not in a link assembly (#2771)"
This reverts commit 44610681fb096cc2b38a18689e954af24049d390.
* Revert "Fix NullReferenceException when sweeping unused static interface (#2783)"
This reverts commit 00e9a154efeb6369b1345bdafeebb686c0163841.
|
|
Don't mark static interface methods when called on a concrete type, and removed the MethodImpl / Override if the interface method is kept.
Co-authored-by: vitek-karas <vitek.karas@microsoft.com>
Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com>
|
|
* Add tests for nested functions called from nested state machines
The existing algorithm to recursively discover calls to local functions
from a user method was incorrect for the case where a local function
was called from a state machine local function's MoveNext method.
These methods were being treated as roots for the discovery, as if they
were user code, which caused a failure downstream when we assumed that
each local function belongs to a unique user method - since a local function
might be called from a user method _and_ a nested state machine local function.
The existing behavior also had the issue that a local function state machine's
attributes could suppress warnings from nested local functions, going against the
behavior for normal nested functions in the linker.
* Don't fail on multiple user methods for same nested function
* Correctly handle iterator local functions calling nested functions
This fixes the issues shown in the tests by letting the state machine
types participate in the call graph discovery instead of being considered
as roots. Now any local functions called from state machine local functions
will be associated with the user method, not with the state machine MoveNext
method.
* Fix test for analyzer
* Update src/linker/Linker/CompilerGeneratedState.cs
Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>
* Adjust comments and docs
* Fix docs
Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>
|
|
|
|
|
|
|
|
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
* Problem description and existing patterns
* Proposed solutions and some discussion of these
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
|
|
|
|
* Normalize warning messages
* Apply suggestions from code review
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Update expected logged message
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
Add new error code for cases in which the static constructor is
annotated with RUC
Add IL2116 to error codes
Move static constructor verification in MarkField before pushing a new
stack to print warnings in the method caller instead of fields
Don't treat static constructors annotated with RUC as dangerous
Add test for several static constructor calls
Warn on field instead of warning on .cctor
* Field should warn no matter the DependencyKind except
DynamicallyAccessedMemberOnType
* Add test for Access to field via reflection, dynamic dependency and
using fields on attributes
Co-authored-by: vitek-karas <vitek.karas@microsoft.com>
|
|
This warning mentions "field" but is about a method.
|
|
* Fix warning origin for DAM on types
* Update tests with link to suppression issue
Messages for these warnings can not be suppressed at the member level
due to issues described in https://github.com/mono/linker/issues/2163.
Also fix expected warnings for properties, revert ResultChecker changes,
and remove redundant test.
* PR feedback
- Include "Attribute" in attribute type names
- Add comments about why we want to report warnings
- Add comment about the interaction with RUC on types
* Add testcase for nested type with default ctor
* PR feedback
- Clean up some test attributes
- Add test with DAM field on annotated PublicMethods type
- Add link to issue about duplicate warnings
Also use replace DAMT with DAM in tests.
* Use member-level suppressions in tests
* Unify with behavior for DAM warnings
- Don't warn on non-virtual methods that don't have return annotations
- Use the helpers introduced in https://github.com/mono/linker/pull/2145
* PR feedback
Clarify that getter without annotated return value doesn't warn because
it's non-virtual
|
|
* Warn when accessing DAM annotated method via reflection
Any access to DAM annotated method must be validated by the trimmer. Normal calls and such are handled already, but direct reflection access or other indirect accesses can lead to trimmer allowing potential invocation of DAM annotated method without validating the annotations. This change will warn whenever such potential "leak" happens.
This applies to method parameter, method return value and field annotations.
Uses the same infra as RUC already just adds additional checks.
Tests for the new cases.
* Formatting
* Only warn on virtual methods with annotate return value
Annotated return value is mostly not problematic (the caller only "Reads" from it, which is always safe). The only case where it's problematic is if something would override the method at runtime (ref emit) in which case the trimmer can't validate the new code that it fulfills the return value requirements. But that can only happen if the method is virtual.
* PR feedback
* PR feedback
* Remove test code.
|
|
Add support for RequiresUnreferencedCode on classes
Added a method to retrieve the effective RequiresUnreferencedCode attribute, which means it will look for the attribute on the type or any of its declaring types
Added new warning IL2109 for when a type derives from a base type with effective RequiresUnreferencedCode, and the derived doesn't have RequiresUnreferencedCode on its effective type
Added logic to handle suppressions
Adds tests
|
|
* Check that global level suppressions do not specify any target
* Run lint
* Update test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/SuppressWarningsInModule.cs
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Add code snippet with an example of 2108.
* PR feedback
Add tests
* Lint
* Don't silently suppress warnings on module if scope is missing and a target was used.
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
Add analyzer logic to create a diagnostic if attributes don't match between all interface implementations or overrides
Use IL2046 to display errors for RUC
Use IL3003 to display errors for RAF
Add IL3003 to error-codes.md
Add tests
Adds support for nullable attribute operations
Add support for adding metadata information into the compilation
Divide the current warning message into 4 different more insightful diagnostic messages following the structure implemented in ObsoleteAttribute messages
Add IL2046 to diagnostic format to ILLink.Shared
|
|
This change implements the following high-level functionality:
* `UnconditionalSuppressMessage` applies to the entire method body even for iterators, async methods
* `RequiresUnreferencedCode` automatically suppresses trim analysis warnings from entire method body for iterators, async methods
Solution approach:
* Detect compiler generated code by using the `IteratorStateMachineAttribute`, `AsyncStateMachineAttribute` and `AyncIteratorStateMachineAttribute`.
* When a method which is compiler generated (detected by looing for `<` in its name) is processed the original "user method" is determined by looking for a method with the "state machine" attribute pointing to the compiler generated type.
* This information is cached to avoid duplication of relatively expensive detection logic.
* Looks for `UnconditionalSuppressMessage` and `RequriesUnreferencedCode` in:
* If the warning origin is not a compiler generated code - simply use the warning origin (method) - existing behavior
* If the warning origin is compiler generated use both the origin as well as the user defined method (non-compiler-generated) which is the source of the code for the compiler generated origin
This is done by storing additional `SuppressionContextMember` on `MessageOrigin` which should always point to non-compiler-generated item.
Added lot of new tests for these scenarios.
This implements warning suppression part of https://github.com/mono/linker/issues/2001 for state machines.
|
|
Also added a small fix in the test infra
|
|
* Flow cppcli assemblies into the linker
* Add tests
* Add C++/CLI assembly
* Update docs/error-codes.md
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
* PR feedback
* Change to copy action.
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
|
|
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
|
|
Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
|
|
* Preserve types for XML serializer
* Move TypePreserve.All
* Add doc about serialization handling
* Adjust comments
* Remove dataflow type discovery
* Add more tests, update doc
* Add logic for DataContractSerializer
* Remove IsActiveFor check
* Fix comments
* Update docs
* PR feedback
- Remove redundant usings
- Make MarkSubStepsDispatcher non-abstract, make ctor public
- PreserveSerialization -> PreserveSerializationSubStep
- SerializationHelper -> SerializationMarker
* Update heuristics to be less conservative
Instead of trying to conservatively match the xamarin-android behavior,
this now does something more "correct" without building in too much
serializer-specific logic. Specifically:
- Don't preserve methods
- Don't preserve static members
- Don't preserve private members
* Only scan types that are already marked
* Fix test
* Activate serialization logic only for ctor calls
With this change the serialization heuristics will mark types only if
there was a call to a relevant serialization constructor.
* PR feedback
Update docs
* PR feedback
Change flag to --disable-serialization-discovery, and add a test
to check that disabling works as expected.
* Add missing changes
* Update docs/serialization.md
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
* Update docs/serialization.md
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
* Update src/linker/Linker.Steps/DiscoverSerializationHandler.cs
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
* Update src/linker/Linker.Steps/DiscoverSerializationHandler.cs
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
* PR feedback
To ensure private attributed members get marked, this changes the "root"
tracking to track members rather than types. Marking a root member for
serialization will now ensure that the member and its type
(for fields/properties) also get marked recursively.
Similar behavior applies for static members, methods, and events.
Also address other feedback:
- Update docs to describe .ctor-based activation
- Clarify kept vs considered root
- RecursiveType -> SerializedRecursiveType
- Add comment about array type handling
* Handle DataContractJsonSerializer
* Mention DataContractJsonSerializer in docs
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
|
|
behavior (#1920)
* Rebase
* Re-add tests and error code doc
|
|
* Support collapsing warnings
* Add tests
* Add documentation, usage, and another test
* Fix formatting
* Fix test, cleanup
* Change MSBuild metadata to "CollapseTrimWarnings"
* Add documentation
* PR feedback
* Update message to include link
* PR feedback
Rename WarningAssemblies to AssembliesWithGeneratedCollapsedWarnings
* Change "collapse" to "singlewarn", adjust wording
* Check subcategory, assert for MemberDefinition
|
|
* Extend attributes removal to support values matching
This is useful for key/value pair like custom attributes removal
* PR feedback
|
|
|
|
|
|
MakeGenericMethod (#1880)
Adds the ability to track some basic patterns around `MethodBase`/`MethodInfo`. Linker now recognizes `ldtoken` for a method as well as tracks return value of `Type.GetMethod`.
With this, the change implements two improvements:
#1814 - Mark property as accessed via reflection when using Expression.Property(Expression,MethodInfo)
Added a new warning to report cases where the intrinsic handling of the Property call doesn't work.
#1727 - Ability to handle simple MakeGenericMethod calls without generating a warning
If the generic method has no annotations on the generic arguments, linker will no longer warn.
Added tests for all the new behaviors.
Improved tests for MakeGenericMethod and MakeGenericType.
Implement checking for new constraint in MakeGenericType - it now behaves the same as if the generic argument is annotated.
|
|
* Support attribute opt-in
* Update docs
* Rename test attributes to match
* Don't pass native SDK assemblies to tests
on Windows
* Update LinkTask
* PR feedback
- Don't warn on duplicate attributes
- Remove comment
- Fix typos and wording
- Use static array
- Rename CheckIsTrimmable -> IsTrimmable
* PR feedback: rename options
- --trim-action -> --trim-mode
-- --default-action -> --action
* Fix ILLink.Tasks test
* PR feedback
- Add plenty of comments
- RootNonTrimmableAssemblies -> ProcessReferencesStep
- Make -reference order stable using List instead of HashSet
* PR feedback
Make GetInputAssemblyPaths private
|
|
* Extend root mode options with library mode
Fixes #1610
* PR feedback
* lint
|
|
* Change resource file to have a default message and accept arguments
* Add test for RequiresAssemblyFilesAttribute with Url named argument only
* Add IL3XXX warning codes to the master error code registry
* Throw whenever a named argument is passed in ExpectedWarning
|
|
Linker has intrinsic handling for some methods which are annotated with `RequiresUnreferencedCodeAttribute`. The intrinsic handling will sometimes recognize the pattern and avoid generating a warning, at other times it will generate a better more specific warning. In cases where this happens the generic handler for `RequiresUnreferencedCodeAttribute` should not kick in, as it would still produce a warning.
The change:
* For direct calls, virtual calls and `.ctor` calls rely on `ReflectionMethodBodyScanner` to handle all warnings caused by `RequiresUnreferencedCodeAttribute`. It has knowledge about intrinsic handling and can handle all non-intrisic calls as well.
* Limit the generic handler in `MarkMethod` to all other cases (typically reflection or other indirect references to the method)
Test changes:
* We already have test cases which trigger these conditions, but we don't have a way to validate that extra warnings were not produced
* Added `ExpectedNoWarnings` test attribute which validates no warnings other than those explicitly stated by the test are produced.
* Used it on two test cases which trigger the interesting condition - and fixed the tests to correctly expect all warnings
|
|
|
|
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
fixed a typo on the customs-steps.ms url
|
|
Changes how the constant propagation works. Instead of iterating over all methods multiple times, use a stack to emulate recursive processing of methods as they are needed.
The doc in this change contains lot of details about the motivation and the algorithm used so please refer to that.
This essentially makes constant propagation and branch removal per-method and not global step. It's still left as a global step which walks all methods and processes them, but that part is now not required by the algorithm.
Interesting statistics (on a console hello world app):
* Before this change we scanned over all methods 3 times - in total more than 217K method body scans). After the change we only scan over a total of 73K method bodies (total number of methods is 72K, so we scan most methods only once)
* Since there are no loops in framework, the maximum number of times we scan any method is 2
* The maximum depth of the processing stack is 16 (but it's a simple app)
The great savings in CPU are accompanied with a larger memory usage. The step now stores a dictionary of all methods (and one reference as a value, the values are almost all non-unique so the storage necessary for values is tiny). This will be necessary anyway for once we move this into `MarkStep` as it has to keep a cache of already processed methods.
|
|
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
|
|
Co-authored-by: Sven Boemer <sbomer@gmail.com>
Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>
|
|
* Add design doc for trimming opt-in/opt-out
This includes some background on what we added for .NET5, and a proposal for more options in .NET6 :
- An assembly-level attribute to opt into trimming
- Simplified trimming opt-in from the SDK
- Simplified "trim all assemblies" flag
This is a summary of the discussion in https://github.com/mono/linker/issues/1269 and https://github.com/dotnet/sdk/issues/14642, and the .NET 5 discussion in https://github.com/dotnet/sdk/issues/12035.
* Add existing AssemblyMetadata example
* PR feedback
* Fix typo
* Update docs/design/trimmed-assemblies.md
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
* Apply suggestions from code review
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
* PR feedback
* Add notes about opt-out
* Use present tense
* Add more opt-out notes
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
|
|
* Restrict embedded XML
Embedded attributes and substitutions may only modify the containing assembly.
* Allow specifying resource name in referenced assembly
In the test infrastructure.
* Fix formatting
* PR feedback
- Allow '*' in corelib embedded attribute XML
- Reword warning messages
- Avoid calling virtual method from ctor
* Fix test on mono
Macking the core library causes problems for PEVerify:
- Removed reference to corelib creates invalid typerefs
- System types like Object aren't found
* PR feedback
* PR feedback
|
|
Some additional description of the various formats and when/how they can be used.
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
* Update warning messages
* Add resource strings
* Keep xlf
* Update warning message
Remove GetDidNotMatchRequirementsWarningMessageArgs
* Update test
* Update src/linker/Resources/Strings.resx
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Update messages
* Update messages
* Unused using
Don't modify .gitignore
* Fix mono build
* Update sorted warnings
* Mono build
* Update src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Add special case f PublicConstructors
* Fix bitwise op
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Remove unnecessary parentheses
* Update sortedwarnings
* Update error-codes
* Fix error number in doc
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
