Trimming: Reflection access to compiler generated methods produces warnings #85042
Comments
vitek-karas
added
the
area-Tools-ILLink
ghost
added
the
|
Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas Issue DetailsTechnically using reflection to access compiler generated methods could cause holes in trim analysis. This is because we don't analyze CG methods on their own, but only in the context of the user defined method which "owns" them. If they're invoked outside such context, the analysis may miss some incompatible behavior. ILLink tried to mitigate this by producing warnings when we detect reflection access to CG method. Originally it produced warning always, but that was way too noisy. Modern C# produces lot of CG methods and using DAM annotations, it's really easy to accidentally "access" some of the CG methods "via reflection". So ILLink also implemented a heuristic, where it tried to detect if the method in question contains potentially dangerous code, this was done by looking for access to annotated members (mostly). The end result of this is that ILLink can produce warnings We considered implementing these in NativeAOT, but it proved to be problematic. The behavior of NativeAOT compiler is different enough, that the heuristic produces different results and there are cases were it produces warnings while ILLink doesn't. These places are also deep in internal structures of the NativeAOT runtime code, and suppressing these is problematic. After a discussion we decided that we should not try to produce these warnings, and leave this as a known analysis hole. Reflection access to compiler generated methods is per C# "undefined behavior" and so it's already very problematic and should not be used. Combine this with trimming which will only make it slightly worse. NativeAOT will not implement the necessary logic to produce
|
…s warnings between ILLink and NativeAOT (#84770) This enables the last of the disabled `RequiresCapability` tests for NativeAOT. The implementation basically copies the logic from ILLink on how to produce warnings when method or field is marked due to reflection access. We decided that producing warnings due to reflection access to compiler generated methods is more trouble than benefit and thus we won't do that anymore. See reasoning in #85042. Otherwise the behavior in NativeAOT fully copies the one in ILLink (small exceptions due to better method overload resolution in NativeAOT). High-level behavior changes in NativeAOT: * Finally allow the NativeAOT compiler to produce IL2110, IL2111, IL2113 and IL2115 * Avoid generating warnings for override methods if the warning is already generated on the base method (for DAM marking) * Implement correct marking (and warnings) for "DAM on type" when the derived class has broader annotation than the base class/interface (this is mostly to avoid noise warnings) * Rename a set of methods and classes in data flow scanner to "Token", because they're only used when a given entity is accessed through its token (so `ldtoken` and similar). This allows for more precise handling of the cases above. Testing changes: * Allow specifying tool exceptions for `SkipKepItemsValidationAttribute` - this allows to enable one of the reflection tests for its diagnostics coverage, avoiding the necessity to fix everything around Kept behavior yet. * Couple of fixes in the test validator to correctly compare expected/actual diagnostics This fixes final bits of #68786 Fixes #68688 This helps with #82447
dotnet-policy-service
bot
added
the
in-pr
Technically using reflection to access compiler generated methods could cause holes in trim analysis. This is because we don't analyze CG methods on their own, but only in the context of the user defined method which "owns" them. If they're invoked outside such context, the analysis may miss some incompatible behavior.
ILLink tried to mitigate this by producing warnings when we detect reflection access to CG method. Originally it produced warning always, but that was way too noisy. Modern C# produces lot of CG methods and using DAM annotations, it's really easy to accidentally "access" some of the CG methods "via reflection". So ILLink also implemented a heuristic, where it tried to detect if the method in question contains potentially dangerous code, this was done by looking for access to annotated members (mostly).
The end result of this is that ILLink can produce warnings
IL2118,IL2119andIL2120. These warnings are currently a "best effort" and there are cases where they don't need to be produced (overly aggressive heuristic in the ILLink). But there are also cases where they're not generated even though they should be (ILLink analysis doesn't catch everything in this case).We considered implementing these in NativeAOT, but it proved to be problematic. The behavior of NativeAOT compiler is different enough, that the heuristic produces different results and there are cases were it produces warnings while ILLink doesn't. These places are also deep in internal structures of the NativeAOT runtime code, and suppressing these is problematic.
After a discussion we decided that we should not try to produce these warnings, and leave this as a known analysis hole. Reflection access to compiler generated methods is per C# "undefined behavior" and so it's already very problematic and should not be used. Combine this with trimming which will only make it slightly worse.
NativeAOT will not implement the necessary logic to produce
IL2118,IL2119andIL2120. We should probably remove these from ILLink eventually as well.The text was updated successfully, but these errors were encountered: