dotnet / dotnet/linker

Branch removal can't remove all branches in compiler generated state machine

Open
#3,087 2 comments 0 reactions 0 assignees View on GitHub
area-Linker: Steps
Dominant language
C#
Stars
392
Forks
128
Avg merge
2d 10h
Merged PRs (30d)
2

Description

Take for example this code:

```C#
static IEnumerable TestBranchWithYieldBefore ()
{
if (AlwaysFalse) { // Property which returns constant false
yield return 1;
RemovedMethod (); // This is unreachable code and should be removed, but it's not
} else {
yield return 1;
UsedMethod ();
}
}
```

The problem is that the compiler generated code produces the `if (AlwaysFalse)` where its body is just some state machine setup because there's a yield there. And so it means that even if that branch is removed, the call to `RemovedMethod` is still kept because it's part of a different state in the state machine.

```C#
private bool MoveNext()
{
switch (<>1__state)
{
default:
return false;
case 0:
<>1__state = -1;
if (AlwaysFalse) // This is the branch which can be optimized
{
<>2__current = 1; // This will be removed...
<>1__state = 1;
return true;
}
<>2__current = 1;
<>1__state = 2;
return true;
case 1:
<>1__state = -1;
RemovedMethod(); // But this is still kept, since there's not constant branch around it
goto IL_007a;
```

In order to correctly implement this, the analysis would have to understand at least basics of state machines. Detecting this in a general case is really complicated because the state is stored in a field, and so we would have to be able to prove that the field can only have a certain set of values - which is really hard in multi-threaded environments.

So in theory doable, but pretty complex.

The downside of this issue is that it's hard to diagnose what's happening for the user. If this is used in combination of feature switches then this can lead to unexpected warnings, since code which is technically hidden behind a feature switch is still marked as reachable by the linker.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.