search

dotnet / linker

388 stars 126 forks source link

Warn on static constructor reflection access #3108

Closed sbomer closed 1 year ago

sbomer commented 1 year ago

Introduces a warning for reflection access to static constructors.

Fixes https://github.com/dotnet/linker/issues/3104

MichalStrehovsky commented 1 year ago

We also have an intrinsic around this in RuntimeHelpers.RunClassConstructor. I assume it just falls out but calling it out just in case it doesn't.

vitek-karas commented 1 year ago

This does nothing about the RuntimeHelpers.RunClassConstructor per Michal's comment above. We should look into that - if it needs an annotation or RUC or something else.

marek-safar commented 1 year ago

Do module initializers have a similar problem?

vitek-karas commented 1 year ago

Do module initializers have a similar problem?

Sort of - it's not the same problem though. In the case of the module initializer there's no reflection access to the .cctor - there's no access at all anywhere in fact - it's a special runtime behavior - which the linker knows about, but it doesn't specifically mark the .cctor as accessed via reflection - just normal marking which would not produce a warning. So technically we should fix this by adding that.

That said, without IL hacking I don't see how one could have a <Module> type with RUC on it - so the priority of this is super low.

sbomer commented 1 year ago

We also have an intrinsic around this in RuntimeHelpers.RunClassConstructor. I assume it just falls out but calling it out just in case it doesn't.

Good catch, this required a small analyzer fix.

This does nothing about the RuntimeHelpers.RunClassConstructor per Michal's comment above. We should look into that - if it needs an annotation or RUC or something else.

With the change to treat cctors on a RUC type as requiring unreferenced code, it looked like this was already warning for RunClassConstructor (see testcase). But the analyzer was missing some logic. Does this address your point @vitek-karas?