| Age | Commit message (Collapse) | Author |
|---|
|
Move RequiresAssemblyFiles from Analyzer to the linker RequiresCapability file
Move the new constraint code from RequiresUnreferencedCode to be on the base Requires file
Add Warning Id to messages from the analyzer to distinguish in a test which analyzer missed/found a warning
Add RequiresAssemblyFilesMismatchAttribute Title and Message to fix bug
Add testing in linker for mismatching attributes
Add testing in linker for Delegates in CompilerGenerated file
Add support for static constructor compiler-generated calls
Add additional warnings from DAM and RUC interaction. Not supported by the analyzer yet
|
|
Add logic to check some of the Ruc on type analysis warnings
Change field and event signature to match analyzer and linker to a single signature mode
Add class as a diagnostic target
Fix issue where attributes ProducedBy.Trimmer/ProducedBy.All are skipped if they are surrounded by ProducedBy.Analyzer attributes
Bring IsConstructor and IsStaticConstructor from Roslyn source
Remove special case for printing warnings in the event instead of the caller of the event accessor
Scenarios of Ruc on type that are not addressed by this PR:
Dealing with interfaces with RUC and having a class implementing it
DAM interaction with RUC
Reflection patterns, some of the tests work because they use typeof() but in general, they are not supported
|
|
* Warn on usage of attributes with annotated ctors
* PR feedback
Add ProducedBy from Test Restructure
Add support for SetupCompileBefore in the analyzer
Run analyzer tests on all members (not only methods)
Move tests to RequiresCapability
* Enable analyzer tests for attributes which use RUC annotated properties
* Lint
* Rename CheckAttributeCtor
Check for instantiations that set annotated properties
* Apply suggestions from code review
Co-authored-by: Andy Gocke <angocke@microsoft.com>
* Update comment
Rename Linker => Trimmer in ProducedBy
* Fix applied suggestions
* Lint
Co-authored-by: Andy Gocke <angocke@microsoft.com>
|
|
We are migrating our unit test projects to run on net5. However, we will continue to run our linker tests against a mono like profile (since that is what Unity is based on).
Eliminating PeVerifier entirely causes us problems. We want to continue running it on the linker test executables.
This PR refactors how PeVerifier is made to do nothing when compiling with NETCOREAPP defined. This approach lets the illink tests continue to operate as they do now. While also giving us the ability to run our `Unity.Linker.Tests` project on net5, our test case assemblies compiled against a .NET Framework profile, and have our test framework continue to run PeVerifier
|
|
* Validate unrecognized patterns against logged warnings
This changes the linker validation to check for unrecognized
reflection access patterns against logged messages. The plan is to do
the same for the analyzer tests, allowing us to make progress as we
replace UnrecognizedReflectionAccessPatternAttribute with
ExpectedWarningAttribute.
The UnrecognizedReflectionAccessPatternAttribute is now checked
against both the logged messages and recorded unrecognized reflection
patterns. We check that it matches the same number of patterns as
logged warnings for consistency. This required some changes to the
signature formats used for validation, since the signatures used in
this attribute now need to use the same format as the one we use for
logged warnings.
Changing the signature formats here uncovered some places where we
were still using the cecil signature format even for user-visible
warnings. These have been changed to use the format from
GetDisplayName.
Generic parameter dataflow validation treats the
UnrecognizedReflectionAccessPatternAttribute differently, using the
member name to refer to a generic type parameter. This inconsistency
has been lifted into a separate custom attribute argument that
specifies when the attribute refers to a generic
parameter. Non-generic method parameters had a similar problem, but
this was less common, so those usages were just replaced by
ExpectedWarningAttribute.
* PR feedback
- Continue returning cecil signature as fallback
(replace exception with a debug assert)
- Remove left-over logic from debugging
* Fix more tests
- Use display name signature format for MethodReference
- Revert changes that expect attributes to match only one warning
- Remove debug output
- Also add missing using
|
|
* Support assembly as message origin
* Add tests
* Fix test failures
* Add missing file
* Update src/linker/Linker.Steps/MarkStep.cs
Co-authored-by: Mateo Torres-Ruiz <mateoatr@users.noreply.github.com>
Co-authored-by: Mateo Torres-Ruiz <mateoatr@users.noreply.github.com>
|
|
The isinst optimization replaces that instruction with a pop, ldnull when the type is not instantiated. This changes the instruction and also changes the length of the instruction in that position. Cecil unfortunately doesn't update try/catch/filter references and they keep pointing to the old isinst instruction which not part of the method body anymore. When saving the assembly the offsets stores in the try/catch/filter records end up effective random and corrupted.
This is a short-term fix to unblock failures in runtime due to this problem.
Medium term fix would be to carefully handle all IL replacements in the linker with regard to try/catch/filter records.
Ideally the long term fix would be to do this in Cecil in such a way that IL replacements would be correctly handled on their own.
This fixes the Http3RequestStream failures mentioned in #2181, but I was not able to confirm if this fixes the CoreLib ArrayPool issues as well (I think it will not).
|
|
* 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
|
|
The core already recognized empty array as no args, but null means the same thing.
This change also:
* Fixes a bug when PublicConstructors was required but only PublicParameterlessConstructors were provided the warning message would not say what the requirement was.
* Simplifies some of the code
* Better matching for UnrecognizedReflectionAccessPattern and warnings in tests
* Added tests
|
|
* Fix writing of updated copyused scopes
Fixes https://github.com/mono/linker/issues/2138
* Fix using order
|
|
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.
|
|
Recent chaneg introduced a scope stack to marking which carries `MessageOrigin` around so that warnings can be reported with precise origin. The one exception to this was the `ReflectionMethodBodyScanner`.
This change integrates the scope stack with `ReflectionMethodBodyScanner`. The visible result is more precise reporting of some warnings. This change will be needed for the work around compiler generated code (since then MessageOrigin will carry even more information than it does currently).
The main API change is addition of the `MessageOrigin` parameter to the `IReflectionPatternRecorder.UnrecognizedReflectionAccessPattern`. There are plans to remove this interface completely, so there' not much need to clean it up right now (so I left the potentially duplicated `origin` and `source` arguments as is).
The rest of the change is just correctly flowing the information around.
I also improved the test infra to be able to correctly handle `ExpectedWarning` with source/line info on all warnings.
There are visible changes in the `SourceLines` test where some warnings changed location (more precise now).
|
|
Probably due to copy/paste error we've validated the attribute by looking at all warning messages at once. This means that potentially any warning can be matched to the attribute even if its location doesn't match.
This change fixes that but uncovers some small test issues:
* The expected warnings happen in different places - fixed that
* Some warnings can't be reported by the Roslyn Analyzer as they require global view - added a mechanism to mark these
Refactored a bit of the Roslyn analyzer validation code to support named properties in attributes.
|
|
* Pick up the DynamicallyAccessedMemberTypes.Interfaces enum from runtime
* Update branding of the linker assembly
* Fix nullref in tests - corelib now has an assembly level attribute with annotations (so the reported "source" is null - since we can't represent assembly as a source just yet).
|
|
* Stop rewriting scopes for copy assemblies with removed references
* Keep exported types in copy assemblies
Mark dynamically accessed forwarders
Add tests
* Transitive forwarders are linked
* Keep copyused behavior
* Tests
* Mark type if it's exported
* PR feedback
* Transitively mark forwarders when a facade is dynamically accessed and has copy action
* Transitively mark forwarders
* Fix formatting
* Update MarkExportedType
* Keep mscorlib assert
* Mark forwarders when there's a type ref to an exported type in a copy assembly
* Update DependencyKind
* Keep forwarders when member ref is resolved from a copy assembly
* Add more tests, mark forwarded CAs
* Mark forwarded interfaces
* Simplify logic for typerefs
* Clean
* Keep ProcessInternalsVisibleAttributes in process while-loop
* Fix whitespace formatting
* Remove unused param
* Feedback
* Whitespace formatting
* Remove unnecessary assembly
* Add IL2104 warning
* Remove unnecessary marking
* Update comment
* Remove ExportedTypeExtensions
* Remove formatDiagnostics and revert cecil subm change
* Remove warning
* Comment out check for removed warning
* Update more of existing calls
* Reproduce attribute issue
* Update scopes before sweeping forwarders
* Remove my enum from compilation
* Fix formatting
* Keep same behavior for scopes in copyused assemblies
Co-authored-by: Marek Safar <marek.safar@gmail.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
This reverts commit 1d96189abb1b2cfe00dff817c63b84b25534746a.
|
|
Co-authored-by: Marek Safar <marek.safar@gmail.com>
|
|
|
|
* Remove LoadReferencesStep
Introduce TryResolve helper, and avoid some calls to GetLoadedAssembly
* Load assemblies lazily
- Run TypeMap logic on-demand
- Register every resolved assembly
so that it gets an action
- Process embedded XML files lazily
- Run body substitutions lazily
- Introduce abstraction to allow XML processing
to run before or during MarkStep
- Iterate over reference closure when needed
instead of only looking at loaded assemblies
- Track applied preserve info
so that it may be changed by new XML
- Only mark assemblies when used
- Introduce test attributes
to check instructions in other assemblies
- Add a base class for per-assembly logic
Squashed commit with updates, cleanup, PR feedback
Cleanup
Clean up copy/save mark logic
Call TryResolve where we used to look for loaded assemblies without throwing
Process XML from type forwarder assemblies
PR feedback
- Prefix "assembly" field with underscore
- Change "context" field to a property
Separate per-assembly step processing
Embedded XML is now read only when needed, with separate caches for descriptors,
substitutions, attributes, removing unreachable blocks, and remaining
per-assembly steps. The attribute cache is kept on CustomAttributeSource.
BaseAssemblyStep -> BasePerAssemblyStep
And don't implement IStep. Instead, take an assembly and context in the ctor.
Some PR feedback
- Don't change GetType use in attribute XML
- Add clarifying comments
- Avoid an extra cache for marking entire assembly
PR feedback
- remove Delete case (this command-line option will be removed)
- change EmbeddedXmlStep -> EmbeddedXmlInfo static class
PR feedback
- Don't support xml in pure facades
- Update constant prop test after rebase
- Slightly clean up RemoveUnreachableBlocksStep
* Add tests for swept references
And fix up some existing tests
* Add lazy loading testcases
- RemoveAttributeInstances from lazy XML
- Changing TypePreserve from lazy XML
- Substitutions from lazy XML
- Constant propagation in lazy assemblies
* Keep copy/save behavior for lazy assemblies
* Avoid loading all references while sweeping
* Ensure that unused assemblies have references removed
We used to remove references to any resolved assembly that was unused.
Now, an unused assembly may not have been resolved at all, so we need to
iterate over references that might resolve to an unused assembly to ensure
that they are removed.
Iterating over all references from loaded assemblies accomplishes this,
but we might miss a resolved unused assembly that was never referenced,
and never set its action to Delete. (This can happen for type forwarders,
for example.) A simple pre-pass over loaded assemblies ensures we handle
this case.
* Remove DynamicDependencyLookupStep
* Change corlib search order
To avoid loading mscorlib in most cases. Otherwise we load mscorlib and
do a bunch of unnecessary work to resolve all of its exported types.
Fixes CanEnableReducedTracing, which was getting a lot of spew from
the exported type marking for mscorlib.
* Fix behavior of mono steps
- Run RemoveSecurityStep for lazy assemblies
- Resolve mscorlib in LoadI18nAssemblies
* Turn on tracking of lazy pending members
* Move MarkExportedTypesTargetStep
* Hide substitution processing inside helper class
This class mediates access to the method actions which can be set by
substitution xml.
Also fix and enable the constant prop testcases which depend on
substitutions.
* Load all references for IDynamicInterfaceCastable
This preserves the behavior which searches all referenced assemblies
for interfaces with DynamicInterfaceCastableImplementationAttribute,
and keeps them if they implement any marked interfaces.
* PR feedback
- Clean up dead code
- Clean up use of non-generic IDictionary
- Add file headers
- Use static classes for per-assembly logic
- Link to open github issues
- Avoid "step" terminology for per-assembly logic
* Move attribute/substitution processing to cache initialization
Instead of calling EnsureProcessed at key points, the attribute XML
processing is now done as part of cache initialization when it is first
accessed.
GetCustomAttributes, GetAction, and similar will no longer trigger
processing that adds attributes to the global store. Instead, we maintain
separate global and per-assembly stores, where the precedence
(global > per-assembly) is explicit in the getters.
In the case of attributes, the precedence is not important because they
are additive, but it does matter for the method actions which can be
mutated.
As part of the change, the attribute/substitution steps have been separated
into steps (which run on command-line XML) and parsers (which are run on
cache initialization for the embedded XML). The steps are simple wrappers
that call the parsers. The parsers can either return a new object representing
the parsed information, or they can modify a passed-in object. The latter is
used for the command-line XML steps which modify the global store of attributes
or substitutions.
* Pick better names
CustomAttributeSource.GlobalXmlInfo -> GlobalAttributeInfo
LinkAttributesParser._xmlInfo -> _attributeInfo
* PR feedback
- Rename "Global" to "Primary"
- Use TryGetValue
- Use properties instead of fields
* Rename more things
- SubstitutionInfo.SetAction -> SetMethodAction
- SubstitutionInfo.SetSubstitutedInit -> SetFieldInit
- ProcessLinkerXmlStepBase -> ProcessLinkerXmlBase
* Separate shared XML logic from IStep interface
- Make ProcessLinkerXmlBase not an IStep
- Separate descriptor marking logic from IStep implementation in
ResolveFromXmlStep
- Introduce DescriptorMarker helper similar to the XML parsers
- Remove parse overload that returns XML info
- Provide context to ctor of XML processing logic
|
|
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>
|
|
|
|
* 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
|
|
* Add supported diagnostics for when we use Url named argument
* Add support for Expected Warning
* Use single format for Resource file
* Dont execute test that used another diagnostic id that is not 2026
* Change formats in the linker for setters and getters to match C# format
* Use GetSignature in method instead of ToString()
|
|
handling (#1645)
Unresolved types should be handled as "unknown" value nodes. This is already the case when they appear in a method body. But when they're used either as generic arguments or in attribute instantiations linker doesn't handle this case correctly.
For generics linker would fail with an exception. For attributes linker would ignore the type.
Both cases are wrong since linker needs to make sure to issue a warning if it can't guarantee to meet the requirements posed by the data flow annotations. So ignoring unresolved types is also wrong.
The change fixes these behaviors to treat the value as "unknown" which leads into warnings when used somewhere with data flow requirements.
There's also a small change in where the attribute related warnings are reported - now they should consistently report the warning in the place where the attribute is used (and not on the attribute definition) and also correctly report which members are involved (it used to report the `this` for ctor arguments for example).
Added a new test infra capability to remove an assembly from linker input - to be able to emulate unresolved assembly.
|
|
|
|
Co-authored-by: vitek-karas <vitek.karas@microsoft.com>
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
|
|
|
|
unsupported type (#1431)
`DynamicallyAccessedMembersAttribute` is only supported on items of type `System.Type` or `System.String` (and derived classes) otherwise linker will ignore it. When it happens it's very likely not intentional and so linker should warn about it. Since it only affects analysis results, these are marked as TrimAnalysis.
|
|
* Don't error on non-IL warning codes
We pass NoWarn which may contain warning codes for other tools. This was
causing failures in https://github.com/dotnet/sdk/pull/12948.
Adds a check that testcases don't produce unexpected errors.
* Make the error check optional
And disable it for tests that expect errors due to global --warnaserror.
|
|
The most important change is to make warning codes and messages unique - that is for each code there's exactly one message. This means adding quite a few new codes as we were sharing them rather heavily.
We also try to avoid logic when constructing messages (to allow for better localization), so instead of having smaller pieces concatenated we introduce several similar warnings.
This change also adds more tests for some of the warnings.
Infra changes:
* Add the warning code parameter to unrecognized reflection call on the interface
* Add a new ExpectedWarning test attribute which validates the code, the location and optionally parts of the message
* Improved the unrecognized reflection pattern attribute to accept only parts of the message (and to accept more than one)
* Add the ability to validat the warning code for unrecognised reflection patterns
* Refactor source context for annotated values
Makes the source context required by making it a a .ctor argument for all annotated nodes. Make it
IMetadataTokenProvider. Fixes the problem with AnnotatedStringValue which was reported as "unknown" in warning
messages.
This also slightly changes what is stored in the SourceContext - now it's the actual thing (so ParameterDefinition,
MethodReturnType and so on) - so printing it out doesn't require knowing which value node it came from. This is cleaner
because the SourceContext describes where the value came from, not what the value is (that's the job of the ValueNode).
In warnings we typically don't want to print out what the value is since in most cases we don't know the actual value (we
just know things about it), so we can rely on SourceContext to simplify some code.
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
the type itself
If an attribute is marked for removal of instances, but the linker keeps the attribute type for other reasons (for example because it's used from a copy assembly) then there will be several other references to the attribute type from the code within the attribute type itself. These warnings are useless and should not be generated.
Other changes:
* Moved link attribute tests into their own separate folder - they don't belong to DataFlow tests
* The log verification logic now removes messages which where matched by LogContains attributes, so it's possible to validate that there are not other messages of the same kind by adding LogDoesntContain at the end
|
|
For the linker to work correctly all correctness annotations must be exactly the same on all methods in a single virtual method override hierarchy.
Currently this applies to DynamicallyAccessedMemberAttribute and RequiresUnreferencedCodeAttribute.
The validation is postponed after the MarkStep since it requires full knowledge of virtual method hierarchies (and interface method hierarchies). With TypeMapStep these are available during MarkStep as well, but we're trying to move away from that requirement, so the validation is done after MarkStep, this only adds a very small amount of memory (the list of methods to validate is currently cca 200 for the entire corelibe).
Refactorings to help with this change:
* Change how we store generic parameter annotations - it now copies the method parameter annotation storage - array indexed by the index of the generic parameter on the type/method. This makes it easy to map the annotation to a generic parameter on any method in the virtual hierarchy (as they all share the same index).
* FlowAnnotations are now stored directly on the Annotations class (since the new step needs them after MarkStep)
* Move some helpers into DataFlowUtilities - mostly around getting description of members for error message purposes
* Synchronize behavior of LogContains and LogDoesNotContain test attributes - both now support "contains" and "regex" versions
* Correctly detect persense of LogContains and LogDoesNotContain attributes on all members of a test, so that test logging is turned on when they're used
* Added a TestSystemTypeBase class - which is a class derived from System.Type and implements all abstract members. This is used to then write test classes which need to derive from System.Type (to test the "this" parameter annotations).
|
|
* Implement data flow annotations on generic parameters
The checks are done when linker calls MarkType(TypeReference) or MarkMethod(MethodReference). The check itself is straightforward as it uses existing functionality.
Added annotations in FlowAnnotations for generic parameters.
Added one parameter to MarkType to better track the location where the type reference comes from. This enables much more precise reporting of warnings.
On test side this makes the recognized/unrecognized attributes applicable to many more things as warnings can come from types, fields and so on.
* Recognize MakeGenericType/Method and warn on them
Data flow on generic parameters makes any generic instantiation potentially problematic. So we need to start marking MakeGenericType and MakeGenericMethod as problematic as well.
MakeGenericType is possible to only warn when there are generic parameters with annotations. For MakeGenericMethod we can't do anything special as we don't track MethodInfo instances.
|
|
* Warnings during propagation of data flow annotations in properties
When we propagate data flow annotations from the property to its getter/setter/backing field there are cases which the code doesn't handle. This change will issue warnings for most of those cases:
- If we can't detect backing field
- If there already is a data flow annotation in a place where we would try to apply it
Added tests for these cases
* Don't warn if we can't deduce backing fields
|
|
(#1219)
With the upcoming support for DynamicallyAccessedMembersAttribute on generic parameters, the source of the access pattern will no longer be any code, but it can be a type declaration or field definition and so on. In such cases trying to point back to some line of code is complicated and in lot of cases confusing.
This change allows more values to be passed as the source of a reflection access pattern. Currently there's no use case for it. The one which comes close is attribute fields, but those make sense to use the attribute ctor as the source of the problem and use the field as the "accessed" item.
Corresponding changes to the test infra to allow for wider range of values and to handle missing values better.
This is also to align with warning suppression code. The reported patterns must work with warning suppression and thus we can't report source which the warning suppression system would not be able to handle.
|
|
* `Type.GetNestedType` - should be straighforward
* `TypeDelegator..ctor` - this is used in CoreLib - we should just treat it as identity so that we don't lose dataflow annotations
* `object.GetType` - I specifically need the delegate part for `CoreLib`; the rest of the behavior is nice to have but I don't expect it to kick in too often.
|
|
Introduces a cache of "linker attributes", currently only RequiresUnreferencedCodeAttribute is supported, but it's highly likely we will need this for other attributes as well (DynamicDependency, future AOT/Single-file related attributes, UnconditionalSuppressMessage, ...). The cache is on Annotations class and should also act as a natural point to unify attribute and XML based inputs (once we have the XML based attribute injection).
RequiresUnreferencedCode now causes a linker warning whenever a method with it is called.
It also auto-suppresses warnings within the method with this attribute - it actually disables data flow analysis on the method right now.
In the future when we have AOT/Single-File related analysis we will need to modify this to selectively support a subset and only auto-suppress warnings related to the relevant subsets.
Extends the LogContains/LogDoesNotContain test attributes so that they can appear on anything within the test class. Functionality is still the same as if they're on the class, but it makes it easier to structure the tests and make them more readable. Once we have solid warning origins, we could also auto-validate that the warning came from the method it is on.
Added support for multiple attributes of the same type. Added logic to detect multiple instances of attribute where there should only be one - in this case linker will issue a warning and use the first instance only.
|
|
* Log errors via MessageContainer
* Use LinkerErrorExceptions for all throws. Don't stop linker on first error found.
* Move FoundErrors to LinkContext and clean exceptions.
* Use same code for errors with same message.
* Fix test and whitespace formatting
* Update exit message
Co-authored-by: Marek Safar <marek.safar@gmail.com>
* Add error for unexpected scenarios.
* Print all inner exceptions.
* Update FeatureSubstitutionsInvalid test.
* Update docs/error-codes.md
Co-authored-by: Sven Boemer <sbomer@gmail.com>
* Print all inner exceptions in LinkerFatalErrorException
* Whitespace formatting
Co-authored-by: Marek Safar <marek.safar@gmail.com>
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
The attribute changed namespace and the DynamicallyAccessedMemberTypes enum has been renamed and has differnet values in it now.
Most notable changes:
* The DynamicallyAccessedMemberTypes values are similar to binding flags in that they specify Public/NonPublic which are exclusive
* There's a new DynamicallyAccessedMemberTypes.All value which is supposed to be a special value (not a simple superset of all the other bits) and tells the linker to keep the type intact.
This also changes the behavior of the linker when recognizing calls to reflection methods like GetField/GetMethod. Previously the overloads which didn't take BindingFlags would consider all Public and NonPublic and Static/Instance members. That's not how the reflection APIs behave, these overloads only consider Public Static/Instance members. This change modifies the linker to do the same.
As such this changes the linker behavior to potentially include less, but to also potentially break some apps which by accident worked due to the linker including more than necessary. Note that the linker will not warn if the member being asked for was not found (or doesn't have the right binding flags). The reasoning is that without linker the app would get a null from the reflection API in such case, with the linker it will still get null, so the behavior of the app doesn't change and as such there's no reason to raise a warning.
Lot of test changes to adapt to the new enum.
Removed internal declaration of BindingFlags - relying on the one from framework.
Added internal declaration for the new attributes with the expectation that we're going to remove them (ifdef) once we move to the framework which already has them.
|
|
* Change IReflectionPatternRecorder to receive the Instruction where the reflection method is called instead of its MethodDefinition
* Enable representation of CA dataflow within the reflection pattern context
* Add the option to check for the return type of an unrecognized pattern
|
|
* Apply consistent formatting based on .editorconfig
```
dotnet format -f <path>
```
* Add lint step to the CI
* Use local tool
* Suppress publish logs warning
* Fix more style errors
* Fixes bad merge
* Write something to log dir
* Move lint job to global scope
So it doesn't get the arcade publish logs/test steps injected.
* Split sources and tests reporting
* Include also src folder in the run
* Exclude cecil sources
* Remove duplicate line
* Trigger notification
* Format more code
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
|
|
regenerating the assembly.
Fixes #1108
|
|
Merge master into feature/dataflow
|
|
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.
|
|
|
|
* 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
|
|
|
|
* 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>
|
|
Implements propagation of data flow annotations from/to method parameters and from/to method return value.
|
