dotnet / linker

389 stars 127 forks source link

Locals are not treated as independent blocks in linker logic #2746

Open tlakollo opened 2 years ago

tlakollo commented 2 years ago

Analyzer has a logic that handles blocks independently depending on their predecessor value meaning that the following 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]------

Linker on the other hand has a concept of a global 'locals' structure, the structure would look similar to:

---[ l o c a l s ]---
---/----/---\----\---
-[B1]-[B2]-[B3]-[B4]-

This is only one example of the problems with the linker although there are many others that could surface, the linker already handles different stacks to behave like the analyzer when there are branches due to if conditions that allow to have a separation between stacks and then merge them together (see code) the solution should be to have a similar system that threats locals independently for each of the blocks and then something that merges the different local values.

vitek-karas commented 2 years ago

This is not entirely accurate about the linker. It does track values per basic block - all locals are stored with ValueBasicBlockPair which stores the basic block identification + the value. And in some cases when it stores locals it will store multiple values each with a different basic block ID. But it's not 100% consistent about doing this and it unfortunately completely ignores the basic block ID when reading the values in some cases. So the end result is that it's as if it doesn't track basic blocks, even though it tries, but fails.

I would not try to fix this in isolation. I think that once we start using CFG in linker a fix for this will be a side-effect of that change.