| Age | Commit message (Collapse) | Author |
|---|
|
This scans compiler-generated methods together as a group when marking
the corresponding user code.
There are two scans that we do:
1. An initial scan to determine whether we need to run the dataflow
scanner. It is also what marks static dependencies of the method IL.
2. The full dataflow scan (only if the initial scan says we need
to). This produces warnings and marks reflection dependencies.
Both scans are now done for the group of compiler-generated methods
when marking the user code. For now, we only do the dataflow scan once
per method, but in a later change, we will need to allow re-scanning
compiler-generated callees (as part of the full scan for the
corresponding user code) to properly track captured state.
When compiler-generated code is accessed via reflection, we now don't
do a dataflow scan because we don't have the context that might be
captured from user code. This also means that reflection-dependencies
of the compiler-generated code aren't kept, and dataflow warnings
aren't produced, unless the code is reached through the corresponding
user method. To guard against this, there are new warnings on
reflection access to compiler-generated code. The reflection access
warnings are only for compiler-generated code which would normally
require the reflection method body scanner. This is a heuristic meant
to conservatively approximate "this compiler-generated code would
produce dataflow warnings if invoked by reflection with an unknown
context".
|
|
* Don't crash analyzer for ref-return assignment
* Same for flow capture references
* Adjust tests after merge
* PR feedback
Add issue links to tests
|
|
Warn instead of throwing on unhandled reference store
I scoped the exception down to the case of a typed value which is of interesting type. Currently we don't think this can every happen and it's basically the only case where there's a potential for analysis hole.
Added a test case which actually reproes the failure from the runtime - not exactly (it seems the compiler in runtime is slightly different or something else is going on and the exact same code from runtime produces different IL in the linker tests), but modified so that it does hit the exception in unchanged code.
Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com>
|
|
|
|
Use RUC warnings to validate correct marking caused by attribute data flow.
The analyzer side of these tests doesn't work yet as they use string->type marking and the analyzer is missing a type name parser for now.
|
|
Type parameters for Roslyn-generated types are all alpha renames, so
there should be a unique original type parameter to map back to, which
can be used to get the original user attributes applied to the type
parameter. This change tries to build a map back from types to
constructor calls, which should map to the original type parameter annotations.
Only handles async and iterator state machines fully right now. Full support for
lambdas and local functions is left as future work.
|
|
[main] Update dependencies from dotnet/arcade
- Update the linker repo to .NET 7
Some small refactoring of the build files to avoid having to update multiple places with the new TFM.
Note that not all can be updated as they are used verbatim in the NuGet package, so can't rely on repo-only properties.
Also currently I left the source code to repeat these. Eventually we might investigate generating `.cs` files in the msbuild to "inject" the TFM and other constants from the MSBuild to the compiled code.
For the Roslyn tests, I hardcoded a new 7.0.0-preview.2 ref package reference, but this feels really weird - note that so far we've been testing against 6.0.0-preview.5 version. Ideally there would be some way to deduce the ref package version from the currently used SDK, and use that in the tests.
The formatting changes are induced by running `lint`. I assume this is because of the SDK version change as well, but I don't know for sure.
- Merge branch 'darc-main-3a65fa7f-262c-4578-97fd-670249162fc8' of https://github.com/dotnet/linker into darc-main-3a65fa7f-262c-4578-97fd-670249162fc8
- Merge remote-tracking branch 'mono/main' into darc-main-3a65fa7f-262c-4578-97fd-670249162fc8
- Update to preview 3 SDK
- Update to .NET 7 Preview 4 which should have the necessary changes.
- Formatting
|
|
Added a test based on array analysis which uses integers as indeces to the array.
|
|
This means the value of the fields is tracked as a const value instead of a field reference.
This is to support some additional code patterns as well as align the behavior with the linker. Compiler will inline the const fields into the IL effectively removing the field refernce in these cases and linker only sees the constant. So ideally the analyzer should have a similar behavior.
|
|
This shared pretty much all of the remaining intrinsics except for `object.GetType` (type hierarchy marking).
The linker move is straightforward and the only functional changes are:
* `Type.GetType()` with unsupported case insensitive search will not produce warnings from its return value
* Removed intrinsics handling of `Activator.CreateInstance<T>` - it's not needed, annotations do the same thing, and avoid generating duplicate warnings. This fixes: https://github.com/dotnet/linker/issues/1483
In the analyzer there are two behavioral differences from linker:
* Searching for assembly name + type name is completely disabled (no warnings, ignored) - see the comment added, but there's no good way to do it in most cases and probably not common. We can improve if really necessary.
* `Type.GetType` doesn't actually resolve any types since we don't have a type resolver implementation in the analyzer. This change makes all the rest work as appropriate, so once we do have a type resolver, it should all just light up.
I added test to cover at least basic type resolver cases in a way that it shows that analyzer doesn't handle it.
Other small things:
* Analyzer's ability to handle boolean constants
* Minor cleanup
|
|
|
|
Moves the intrinsic handling of `MakeGenericType`, `MakeGenericMethod` and `Expression.Call` into the shared code.
Other changes:
* Some small refactorings on the shared type system and value system.
* Expose generic parameters on type system proxies
* Fix a bug in analyzer when we're recording patterns, the values must be cloned (as they can mutate during analysis after the record is taken)
* Fix some problems in Nullable<T> handling in MakeGenericType - specifically if we don't know what the T is going to be, we now return just Nullable<> known type (but unknown T).
* Add couple new tests - update existing ones (mainly due to limitation in the analyzer)
Co-authored-by: Sven Boemer <sbomer@gmail.com>
|
|
Moving some simple method handle related intrinsics and unifying the values related to it.
|
|
|
|
|
|
* Support assignment to flow capture references
This uses code copied from Roslyn to detect whether a flow capture is used as an r-value or l-value (or both).
L-value captures are tracked in a separate dictionary which is used to look up captured values when assigning to
l-value capture references. The capture references are not first-class dataflow values under SingleValue, in
order to keep the general dataflow logic separated from the trim-analysis-specific values.
|
|
* Move parameter annotation checks to shared code for analyzer
Add GetMethodThisParameterValue
Make GetParameterAnnotations return a nonnull
DynamicallyAccessedMemberTypes
* Use the shared code for parameter annotation checks in the linker
* Remove redundant handling from analyzer
* Use ReturnsVoid() extension method
* Fall back to annotations for unimplemented intrinsics in analyzer
|
|
- The analyzer now has a logic that handles array blocks independently when an if statement is used, see the following example code:
Type[] arr = new Type[] { null };
if (i == 1)
arr[0] = GetMethods ();
else
arr[i] = GetFields ();
arr[0].RequiresAll ();
Gets translated to a structure similar to:
------[B1]------
-----/----\-----
--[B2]----[B3]--
-----\----/-----
------[B4]------
* The first block (B1) contains the array initialization
* Given that there could be different values depending on the result of the if statement, block 2 (B2) and block 3 (B3) handle the different results. Notice that they both start with a copy of the information on B1 which is their predecessor. B2 and B3 do not interact between them and generate their output based on merging the value of B1 and their own generated value.
* B2 executes GetMethods(); which has the return value PublicMethods, then we merge the values in index 0 which are: empty array from B1 and the return value in B2 (PublicMethods)
* B3 executes GetFields(); which has the return value PublicFields, then tries to access the array but the index is unknown so we return UnknownValue
* Block 4 (B4) executes RequiresAll(); in order to verify if arr[0] fulfills the requirements, we read the different values in the array index 0, meaning the result from B2 and B3. At that moment we attempt to merge the result from the different arrays, we verify that one of the results is unknown and therefore we cannot know if arr[0] fulfills the requirements. Generating a single warning stating that the value is unknown.
- Add tests
|
|
Add check for cref in generic parameter dataflow to avoid failing with a nullreference exception
Refactor code to reuse the while loop for both INamedType and Method symbols
Add tests
Fixes #2724
|
|
The main change is sharing of the GetConstructor intrinsic and related tests.
To make those tests work this also implements Type.EmptyTypes and String.Empty and Array.Empty intrinsics.
These are all treated as special cases:
* Type.EmptyTypes and String.Empty are actually field accesses, so we don't have infra to share these yet, no need to add one for now
* Array.Empty is a generic method where we need to know the generic instantiation. The current sharing doesn't propagate this information
so this forced the implementation to NOT be shared but instead done as a fallback which the shared one doesn't handle.
The rest of the changes are all about fixing how the shared intrinsic handling propagates return value from intrinsics.
Lot of small subtle bugs around null or emtpy inputs and forgetting to add return value and so on. The main changes are in GetMethod and GetNestedType intrinsics which had lot of issues.
Added tests for all of the corner cases.
NestedTypesUsedViaReflection test has been reworked as previously it didn't work correctly. Lot of the [Kept] attributes were fulfilled by some of the negative tests and thus the positive tests didn't have coverage (for example calling GetNestedType(unknownName) will mark all nested types on the type, so a subsequent call to GetNestedType("knownname") will not have an observable effect).
|
|
|
|
of the file (#2469)
Some of the files that hold a license from a third party now have the two licenses as header files
Linker now uses the sdk format of license header instead of the runtime one
Added having the license header as warning in the editor config
Added THIRD-PARTY-NOTICES.TXT file to the repo root
|
|
* Fix array dataflow test for passing ref to element
|
|
* Add test which mimics what is done in Type.ImplementInterface helper in runtime
There's nothing new in this test, but it's better to have coverage for the pattern as it's used in runtime.
* Add tests for properties
* Simplify
Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com>
|
|
|
|
Add intrinsic support for Nullable.GetUnderlyingType and support for MakeGenericType with Nullables
Adds ArrayCreationOperation visitors to create ArrayValue's in the analyzer, and adds start of dataflow analysis for array values.
Adds tests to validate dataflow in Arrays.
Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com>
|
|
* Warn on DAM mismatch between overrides
|
|
|
|
|
|
* Fix behavior of intrinsics with null or empty inputs
Add tests for intrinsics receiving null or empty values, and tweak a few
intrinsics to avoid warnings for these cases. This fixes some unnecessary
warnings, and also fixes a crash in the linker.
Some of the new shared intrinsics that don't produce type values need
to have some tracked value. Presumably this should really
be Unknown, but currently they fall back on the shared logic
to track a value with annotations (which for these intrinsics
will be None).
A few unnecessary warnings are left as-is to avoid changing the linker
behavior.
|
|
Other than sharing more code and adapting it so that it works on both linker and analyzer, this change brings simple analysis of constant integer values in the analyzer. This is necessary to make most reflection API calls recognize binding flags. For example `GetMethods(BindingFlags.Public | BindingFlags.Static)`. So this change adds analysis of constant values (as recognized by the Roslyn's operation tree) and the OR binary operator for integers and enums.
Added some new tests for the binding flags handling.
Reenabled some disabled tests for analyzer.
Moved the main affected tests from the generated source files to the hardcoded one and force them to exact match of warnings for both linker and analyzer.
|
|
|
|
|
|
The damAnalyzer branch has actually fixed the array access bug in the linker.
The analyzer doesn't understand this pattern though, so it doesn't produce a warning.
|
|
|
|
The tests use fake generated backing fields, which the linker recognizes as the real thing, but the analyzer is not fooled. So it's expected the analyzer would produce warnings here - and the developer who writes code like this (hard to imagine why) should fix those warnings anyway.
Co-authored-by: Tlakaelel Axayakatl Ceja <tlakaelel.ceja@microsoft.com>
|
|
|
|
Analyzer intentionally "ignores" intrinsics which linker recognizes, but analyzer doesn't yet. Meaning it will not produce any warnings due to them.
Some tests didn't reflect this change correctly.
|
|
This is a test for https://github.com/dotnet/linker/issues/2624
|
|
This adds support for exception filters in the analyzer. Now the exception state will flow from from try blocks to any catch blocks without filters, or to the filter regions of any catch blocks that do have filters. Additionally, the dataflow state flows from a filter region to the next filter or catch. We don't check whether the exception type of the filter regions match, but conservatively assume that control can flow out of any filter to the next filter or catch.
This also assumes that control leaves the filter region from its last block - I don't know if there are cases where a filter region can contain more than one basic block, so if this is possible the logic might need to be updated.
I added a few testcases with dataflow and side effects in filter regions. This fixes https://github.com/dotnet/linker/issues/2415 and allows enabling two more of the previously failing tests.
|
|
* suppressing analyzer for compiler generated methods
* FB
* fixing some tests
|
|
And fix invocations of GetValueUsageInfo
|
|
This tracks null values and adjusts the handling of null values in a
few intrinsics which either take structs that can never be null, or
don't return when the input is null.
MethodBodyScanner needed a small adjustment to correctly deal with the
case where an intrinsic returns TopValue.
|
|
The value of a property set expression is the value, not the return value of the set method (void).
|
|
Produce warnings for annotations on invalid types using a symbol analyzer
Add code to not produce invalid types warnings on dataflow analysis, and reduce the scope to only interesting types
Add extension file to ask if a type is interesting for dataflow
|
|
GetDisplayName (#2601)
|
|
* No Analyzer warnings for intrinsics
* FB
* FB2
* FB3
|
|
Shares several more intrinsics, the most notable one is `Type.GetInterface`.
Changes:
* Change TypeProxy to represent a resolved type on the linker side (no change to analysis)
This means that it can't represent generic instantiations, pointers and so on. Which is OK for the current usage.
Generic instantiations will need to be handled separately anyway due to analyzer
and the way they work in the linker is not ideal either (produce duplicate warnings in places).
Currently only Activator.CreateInstance<T> requires knowledge of generic instantiations
and that one is going to be removed as intrinsic (already has annotation which works the same).
* Add a type system proxy for generic parameter - this helps with representing `typeof(T)` like expressions
* `RequireDynamicallyAccessedMembersAction` is now part of `HandleCallAction` - enables the more interesting intrinsics like `GetInterface`
* Simplify some values to only-shared (no partial) due to type system proxy improvements
* Enable tests for the newly shared intrinsics
* Added some small improvements to tests
* Add one more shared intrinsic RunClassConstructor
Had to add some new state to HandleCallAction since this is the first intrinsic which generates diagnostics directly from the HandleCallAction.
|
|
This is necessary for correct data flow processing since we need to check the arguments to ctor the same way we do any other method.
Added couple of simple tests.
|
|
(#2418)
Add IL2110 and IL2111 to shared Diagnostic ids
Delete the use of context in the binder since the context is never used
Add ReflectionAccessAnalyzer which makes the work of ReflectionMethodBodyScanner without marking and only producing diagnostics
Produce diagnostics for SystemTypeValue that have annotations in its members
Enable the AnnotatedMembersAccessedViaReflection test
Add RequiresUnreferencedCode changes to be able to produce RUC from DAMAnalyzer
Disable intrinsics GetMethod, GetEvent, GetProperty, GetConstructor and GetField
Add diagnosticContext as part of the ReflectionAction
Add Struct Layout in ReflectionAction
Co-authored-by: vitek-karas <10670590+vitek-karas@users.noreply.github.com>
|
