Synthesized name clashes with existing vars in scope

[tamlin-mike]

LINQ-generation needs to look at all vars in scope when synthesizing a name, not only ilSpy-generated names, but also parameters. If not, you can end up with something like

void fn(int var1) {
 ... = from var1 in whatever ...
}

where the second var1 is a ilSpy-synthesized name, intended to be unique, making the code uncompilable.

Since this is yet another instance of the same problem that has been observed more than once, perhaps the code synthesizing a new local name should be refactored into a single place, to solve this problem once and for all?

EDIT: Upon close inspection it's not only a LINQ problem, it pops up for nested vars even when turning off LINQ-generation, such as in parameter names for nested lambda's.

[siegfriedpammer]

This is strange: It will be hard to create a test-case for this, because the variable names in LINQ from and let clauses are derived from the lambda parameter names, which are usually embedded into the compiler-generated display class.

If you happen to have access to a binary sample, this would help a lot. Thank you!

[tamlin-mike]

The sample is from a deobfuscation-attempt of an obfuscated assembly, so likely parameter names have been renamed using the same pattern for all lambdas etc., and ilSpy then blindly using those parameter names without checking if they already exist in scope.

I'll try to find an example and boil it down to IL that can be assembled.

[tamlin-mike]

I managed to whip up some plain C# to repro, and this surfaced yet another local-name-synthesis bug (invalid name generated). :-)

using System.Collections.Generic;
using System.Linq;
class Dummy { public int baz; public List<Dummy> more; }
[System.Runtime.CompilerServices.CompilerGenerated]
class Helper { internal bool HelpMe(Dummy dum) { return true; } }
class Foo {
  void Bar(Dummy dum) {
    var h = new Helper();
    Dummy localDummy = dum.more.Where(h.HelpMe).FirstOrDefault();
    ++localDummy.baz;
  }
}

decompiles into

using System.Linq;

internal class Foo
{
  private void Bar(Dummy dum)
  {
    Helper CS$<>8__locals0 = new Helper();
    Dummy dummy = (from dum in dum.more
      where true
      select dum).FirstOrDefault();
    dummy.baz++;
  }
}

So the first problem is that the identifier dum is taken from HelpMe without checking if the name clashes in the generated scope.

The second problem should be obvious - CS$<>8__locals0 is not a valid identifier.

A third problem surfaces if we comment out the CompilerGenerated attribute:

private void Bar(Dummy dum)
{
  Helper @object = new Helper();
  Dummy dummy = dum.more.Where(@object.HelpMe).FirstOrDefault();
  dummy.baz++;
}

I see no reason to synthesize a name such as @object, or actually any identifier starting with an at sign.

Anyway, these three examples IMO displays quite nicely why name-synthesis needs an overhaul in the suggested direction.

Cheers!

[siegfriedpammer]

I have started working on this issue in variable-naming, however I am facing the issue that LINQ Syntax decompilation requires that the variable names used by lambda parameters are consistent across multiple invocations.

items.Where(item => item != null).Select(item2 => item2.Data);

would be transformed to

from item in items where item != null select item2.Data;

So my current approach to eliminate all lambda/expression tree/local function parameters from the set of existing variable names is insufficient.

Another problem is that the query expression transforms depend on the parameter names of transparent identifiers to detect them.

[tamlin-mike]

Excellent news!

Provided item and item2 are unique in scope, wouldn't the following work?

items.Where(item => item != null).Select(item => item.Data);

Granted, I'm no master of C# scoping rules, but wouldn't those two lambdas be completely separate, and therefore reusing item would be well-formed?

If so, provided I understood the idea of what IL this could be generated from, the LINQ would also only use item and work as expected?

To borrow transformation-lingo from a different area, kindasorta a "phi" operation, but on identifier names.

[yyjdelete]

@siegfriedpammer
781e21e#diff-bfaf881eaeaff0a646a14dbce2e65182R469

The change in variable-naming will cause an NullReferenceException for the below code(.pdb file need to be removed)

System.NullReferenceException:
   at ICSharpCode.Decompiler.IL.Transforms.AssignVariableNames.SplitName(String name, Int32& number) in AssignVariableNames.cs:line 439
   at ICSharpCode.Decompiler.IL.Transforms.AssignVariableNames.GenerateNameForVariable(ILVariable variable) in AssignVariableNames.cs:line 325
   at ICSharpCode.Decompiler.IL.Transforms.AssignVariableNames.PerformAssignment(ILFunction function) in AssignVariableNames.cs:line 175
   at ICSharpCode.Decompiler.IL.Transforms.AssignVariableNames.Run(ILFunction function, ILTransformContext context) in AssignVariableNames.cs:line 120
   at ICSharpCode.Decompiler.CSharp.CSharpDecompiler.DecompileBody(IMethod method, EntityDeclaration entityDecl, DecompileRun decompileRun, ITypeResolveContext decompilationContext) 

public class Class1
{
    public enum Enum
    {
    }

    public void Method2()
    {
        Enum @enum = default;
        Console.WriteLine(@enum.ToString() + "," + @enum.ToString());
    }
}