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

Trimming locals from a method doesn't remove the locals from the pdb #1260

Closed
eerhardt opened this issue Jun 9, 2020 · 6 comments
Closed
Assignees
Milestone

Comments

@eerhardt
Copy link
Member

eerhardt commented Jun 9, 2020

With change dotnet/runtime#37615, we are introducing some ILLink.Substitutions.xml files that direct the linker to trim branches in IL that will never be called. For example, when building for x64, any if (AdvSimd.IsSupported) branches can be trimmed away, since ARM intrinsics will never be true for x64.

However, Pdb2Pdb is failing in this PR:

  System.Private.CoreLib -> F:\workspace\_work\1\s\artifacts\bin\coreclr\Windows_NT.x64.Checked\IL\System.Private.CoreLib.dll
  PDB0004: token 0x06001C30: Method containing local variables has no local signature
  PDB0004: token 0x06001C30: Method containing local variables has no local signature
  PDB0004: token 0x06001C31: Method containing local variables has no local signature
  PDB0004: token 0x06001C31: Method containing local variables has no local signature
  Error HRESULT E_FAIL has been returned from a call to a COM component.
F:\workspace\_work\1\s\.packages\microsoft.dotnet.arcade.sdk\5.0.0-beta.20280.1\tools\SymStore.targets(70,5): error MSB3073: The command ""F:\workspace\_work\1\s\.packages\microsoft.diasymreader.pdb2pdb\1.1.0-beta2-19575-01\tools\Pdb2Pdb.exe" "F:\workspace\_work\1\s\artifacts\bin\coreclr\Windows_NT.x64.Checked\IL\System.Private.CoreLib.dll" /out "F:\workspace\_work\1\s\artifacts\SymStore\Checked\System.Private.CoreLib\netcoreapp2.1\x64\System.Private.CoreLib.pdb" /srcsvrvar SRC_INDEX=public" exited with code 2. [F:\workspace\_work\1\s\src\coreclr\src\System.Private.CoreLib\System.Private.CoreLib.csproj]
##[error].packages\microsoft.dotnet.arcade.sdk\5.0.0-beta.20280.1\tools\SymStore.targets(70,5): error MSB3073: (NETCORE_ENGINEERING_TELEMETRY=Build) The command ""F:\workspace\_work\1\s\.packages\microsoft.diasymreader.pdb2pdb\1.1.0-beta2-19575-01\tools\Pdb2Pdb.exe" "F:\workspace\_work\1\s\artifacts\bin\coreclr\Windows_NT.x64.Checked\IL\System.Private.CoreLib.dll" /out "F:\workspace\_work\1\s\artifacts\SymStore\Checked\System.Private.CoreLib\netcoreapp2.1\x64\System.Private.CoreLib.pdb" /srcsvrvar SRC_INDEX=public" exited with code 2.

Debugging into Pdb2Pdb.exe, I see the first problem is that we are trimming away locals from this method:

https://github.com/dotnet/runtime/blob/bd6cbe3642f51d70839912a6a666e5de747ad581/src/libraries/System.Private.CoreLib/src/System/Numerics/BitOperations.cs#L233-L251

The if (AdvSimd.Arm64.IsSupported) branch is getting trimmed, but it looks like the local variables are not getting removed from the portable .pdb. Here is the code after trimming:

        public static int PopCount(uint value)
        {
            if (Popcnt.IsSupported)
            {
                return (int)Popcnt.PopCount(value);
            }
            !AdvSimd.Arm64.IsSupported;
            return BitOperations.<PopCount>g__SoftwareFallback|9_0(value);
        }

The error

PDB0004: token 0x06001C30: Method containing local variables has no local signature

Is being emitted because Pdb2Pdb is finding local variables in the pdb, but the method doesn't have a "local signature" (since there are no locals in the resulting method).

The ILLinker needs to be removing the locals from the pdbs as well, so tools like Pdb2Pdb still work.

eerhardt added a commit to dotnet/runtime that referenced this issue Jun 10, 2020
* Add ILLink.Substitutions.xml files for System.Private.CoreLib.

Ported the settings from https://github.com/mono/mono/blob/eaa32d13659f0a6b6b5e62ddb49af68b1f9efb6c/sdks/wasm/src/linker-subs.xml and split them out as appropriate to reduce duplication across the different platform builds.

Contributes to #31785

* Remove all Platforms other than wasm to workaround linker bug.

See dotnet/linker#1260
@marek-safar marek-safar added this to the .NET5.0 milestone Jun 11, 2020
@vitek-karas
Copy link
Member

There are actually two issues here

This issue will be about the local variable debug information.

The problem is in CleanRemovedVariables. Linker will remove the local variables from the method body definition, but it will not remove them from debug information. Some tools consuming the PDB will then get confused when reading it as the information in PDB doesn't match the method body.

Fix is to walk all local scopes (even nested ones) and remove variables which are also removed from the body (not replaced with System.Object ones). Since the debug info for local variables only stores name and some special flags, rewriting the variable to be System.Object doesn't change the debug info for it.

@vitek-karas
Copy link
Member

@vitek-karas
Copy link
Member

Cleaner way to fix this would be in Cecil itself. VariableDefinitionCollection.OnRemove is the right place to go and cleanup debug info for the variable being removed. There's no real counterpart for OnAdd as VariableDefinition doesn't carry any debug info (name), so there's nothing to add to the debug info.

@eerhardt
Copy link
Member Author

@vitek-karas - I see the underlying Cecil fixes have been merged. Is this issue now fixed? Or do we need to take the latest Cecil in mono/linker in order to get the changes?

Once this is fixed, we can fix dotnet/runtime#31785.

@vitek-karas
Copy link
Member

We need to get a new build of https://github.com/jbevain/cecil published. It should be in the works.. so hopefully soon.

@marek-safar
Copy link
Contributor

Fixed in master and 5.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants