Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings depend on order of constant prop and compiler-generated code detection #2937

Open
Tracked by #101149
sbomer opened this issue Aug 1, 2022 · 0 comments
Open
Tracked by #101149

Comments

@sbomer
Copy link
Member

sbomer commented Aug 1, 2022

See tests added in #2928 and #2931.

When constant propagation removes calls to a local function, the compiler-generated state may or may not detect the user method which owns the local function. It depends on whether it gets a chance to inspect the user method before constant propagation.

#2931 shows a case where this can produce different warning behavior when RUC on the user method may or may not suppress a warning from the local function.

vitek-karas added a commit to vitek-karas/linker that referenced this issue Oct 31, 2022
…e constant prop/branch removal happened on the method.

This does have one possibly negative impact: the issue described in dotnet#2937 is now consistent and happens always.

Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar.

Basically in such case the analysis can't figure out to which method the local function belongs (since the call site has been removed).
vitek-karas added a commit that referenced this issue Nov 1, 2022
Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking

This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method.

The change guarantees that all accesses to Body are after the constant prop/branch removal happened on the method.

This does have one possibly negative impact: the issue described in #2937 is now consistent and happens always.

Added tests.

Note that there's still a whole in analysis of compiler generated code around state machines, see #3087

Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar.

In such case the analysis can't figure out to which method the local function belongs (since the call site has been removed).
tlakollo pushed a commit to dotnet/runtime that referenced this issue Nov 4, 2022
Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking

This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method.

The change guarantees that all accesses to Body are after the constant prop/branch removal happened on the method.

This does have one possibly negative impact: the issue described in dotnet/linker#2937 is now consistent and happens always.

Added tests.

Note that there's still a whole in analysis of compiler generated code around state machines, see dotnet/linker#3087

Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar.

In such case the analysis can't figure out to which method the local function belongs (since the call site has been removed).

Commit migrated from dotnet/linker@e502e72
agocke pushed a commit to dotnet/runtime that referenced this issue Nov 16, 2022
Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking

This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method.

The change guarantees that all accesses to Body are after the constant prop/branch removal happened on the method.

This does have one possibly negative impact: the issue described in dotnet/linker#2937 is now consistent and happens always.

Added tests.

Note that there's still a whole in analysis of compiler generated code around state machines, see dotnet/linker#3087

Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar.

In such case the analysis can't figure out to which method the local function belongs (since the call site has been removed).

Commit migrated from dotnet/linker@e502e72
vitek-karas added a commit to vitek-karas/linker that referenced this issue Dec 13, 2022
Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking

This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method.

The change guarantees that all accesses to Body are after the constant prop/branch removal happened on the method.

This does have one possibly negative impact: the issue described in dotnet#2937 is now consistent and happens always.

Added tests.

Note that there's still a whole in analysis of compiler generated code around state machines, see dotnet#3087

Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar.

In such case the analysis can't figure out to which method the local function belongs (since the call site has been removed).
sbomer pushed a commit that referenced this issue Jan 18, 2023
* Fix branch removal in compiler generated code (#3088)

Changes to processing of compiler generated methods lead to a state where we don't call constant prop and branch removal in all cases before we mark instructions of the method. This can lead to overmarking

This change fixes this by making sure that the branch removal executes on the method in all cases before we mark instructions of the method.

The change guarantees that all accesses to Body are after the constant prop/branch removal happened on the method.

This does have one possibly negative impact: the issue described in #2937 is now consistent and happens always.

Added tests.

Note that there's still a whole in analysis of compiler generated code around state machines, see #3087

Basically if there's a local function which is going to be removed due to branch removal and if the body of that method contains code which produces a warning due to generic parameter validation, such warning will always be generated even though it's "dead" code and even if it's suppressed via RUC or similar.

In such case the analysis can't figure out to which method the local function belongs (since the call site has been removed).

* PR feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant