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

C# PrivateImplementationDetails class can contain module name with '.', confusing tools which expect to parse into namespaces #1430

Closed
agocke opened this issue Mar 20, 2015 · 2 comments · Fixed by #1546
Assignees
Labels
Area-Compilers Bug Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. Verified
Milestone

Comments

@agocke
Copy link
Member

agocke commented Mar 20, 2015

Commit 38aa68d adds the module name to the PrivateImplementationDetails class to prevent #1228. If that module name contains '.', tools which consume metadata and attempt to reverse engineer C# namespaces can become confused and believe that the dots represent namespace separators. For example, this is the result in refasmgen:

.namespace '<PrivateImplementationDetails><Maui.Core'
{
    .class private sealed 'dll>'
        extends [mscorlib]System.Object
    {
        .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = { }
        .method assembly static 
            uint32 ComputeStringHash(string s)
        {
            ret
        }
        .field static assembly initonly int64 '7E50BDC673C53A3E19E50BC237337FA9902ABB30'
    }
}
@agocke agocke added Area-Compilers Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. labels Mar 20, 2015
@gafter gafter added this to the 1.0 (stable) milestone Mar 20, 2015
@gafter gafter self-assigned this Mar 20, 2015
@gafter gafter added 2 - Ready Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. labels Mar 20, 2015
@gafter
Copy link
Member

gafter commented Mar 23, 2015

See #1428 for a proposed solution.

@gafter
Copy link
Member

gafter commented Mar 23, 2015

@VSadov I believe you might be looking at this.

@theoy theoy added the Bug label Mar 24, 2015
@VSadov VSadov assigned VSadov and unassigned gafter Mar 24, 2015
VSadov added a commit to VSadov/roslyn that referenced this issue Mar 25, 2015
…ame only when dealing with netmudules.

The goal of module name apending is to avoid clashes when combining multiple netmodules into multifile assembly. When building a regular assembly, appending module name is not serving any purpose and just causes unnecessary metadata differences.

Also in this change - when we do apend the module name, replace '.' with '_' when that happens. For example when we build a netmodule and its name is Foo.Bar.dll
More complicated name mangling schemes were discussed, but at this point we will do a simple '.' --> '_' as the least destabilizing change which is still sufficient in the most common case of having dots in the module name.

Fixes dotnet#1430
@gafter gafter added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 3 - Working labels Apr 19, 2015
@tmat tmat added the Verified label Jun 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Concept-Determinism The issue involves our ability to support determinism in binaries and PDBs created at build time. Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented Tenet-Compatibility Violation of forwards/backwards compatibility in a design-time piece. Verified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants