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

For .net 5 preview1, the CoreFX symbols are missing for IL symbols. #33097

Closed
yunwei00q opened this issue Mar 3, 2020 · 34 comments · Fixed by #33521
Closed

For .net 5 preview1, the CoreFX symbols are missing for IL symbols. #33097

yunwei00q opened this issue Mar 3, 2020 · 34 comments · Fixed by #33521

Comments

@yunwei00q
Copy link

yunwei00q commented Mar 3, 2020

Affected Build: Master/29827.154 (16.6.0 preview 1.0) + CLI/SDK:5.0.100-preview.1.20125.9

Steps to reproduce
Testing on Windows with VS -

  1. Goto Tools -> options -> Debugging
    Under General
    a. Uncheck 'Enable Just My Code'
    b. Check 'Suppress JIT optimization on module load (Managed only)'
    Under Symbols
    a. Check 'Microsoft Symbol Servers'

  2. Create a new ASP.NET Razor project by doing
    mkdir razor
    cd razor
    dotnet new razor

  3. Double click on the ‘Startup.cs’ file in the ‘Explorer’ pane to bring up that source code.

  4. Add the following line to the Startup.cs file at the beginning of the ‘Configure’ method (at the bottom of the file). When using VS, you can add this line of code on Program.cs file

    Console.WriteLine("x " + 3);

  5. Place a breakpoint on that line (F9).

  6. Run the application until it hit that breakpoint (F5). It will load symbols from the Microsoft symbol server, and may take a few seconds to hit the breakpoint. You should see 20-30 lines of ‘Loaded XXX Symbols loaded’.

  7. Step into the concatination method (F11). Select 'Download Source and Continue Debugging' from the popup
    This tests CoreCLR symbols AT THIS POINT IT SHOULD SHOW YOU THE SOURCE CODE FOR Concat().

  8. Step out of Concat (Shift F11). Then step into the WriteLine method(F11). You may see the pop up as above, select the first option from it.
    This tests CoreFX symbols AT THIS POINT IT SHOULD SHOW YOU THE SOURCE CODE FOR Console.WriteLine().

Expected result:

CoreFX symbols are displayed in step8.

Actual result:

CoreFX symbols are missing in step8.
image

Dotnet –info:

.NET Core SDK (reflecting any global.json):
 Version:   5.0.100-preview.1.20125.9
 Commit:    12e88ed2e5

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.100-preview.1.20125.9\

Host (useful for support):
  Version: 5.0.0-preview.1.20120.5
  Commit:  3c523a6a7a

.NET Core SDKs installed:
  3.1.200-preview-014977 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.1.20125.9 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0-preview.1.20124.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0-preview.1.20120.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0-preview.1.20122.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
To install additional .NET Core runtimes or SDKs:
https://aka.ms/dotnet-download

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Mar 3, 2020
@yunwei00q yunwei00q changed the title For .net 5 preview1, The CoreFX symbols are missing for IL symbols. For .net 5 preview1, the CoreFX symbols are missing for IL symbols. Mar 3, 2020
@danmoseley
Copy link
Member

@markwilkie didn't we add built-in validation that packages contained symbols (same as we added validation for symbols containing sourcelink)?

@danmoseley
Copy link
Member

Actually, re-reading it sounds like symbols resolved but did not point to sources - sounds like sourcelink perhaps.

@markwilkie
Copy link
Member

Any insight @tmat ?

@tmat
Copy link
Member

tmat commented Mar 3, 2020

I don't know what the current status of Source Link validation is. I think it was disabled at some point.
@JohnTortugo

@JohnTortugo
Copy link
Contributor

The SourceLink validation has a toggle parameter to enable/disable it. Currently it is disabled for dotnet/runtime and dotnet/[email protected]

I think we missed re-enabling it when this issue was closed: #30551

@danmoseley
Copy link
Member

Ouch. Maybe we need a validation step for the validation step ☹️

@ericstj not sure when @safern is back, or maybe @Anipik could look? We should fix for Preview 2

@danmoseley
Copy link
Member

I also see enableSymbolValidation: false on the line above....

@JohnTortugo
Copy link
Contributor

The symbol validation should be kept disabled. We were having some issues with that validation and decided to implement it as part of the release automation process: dotnet/arcade#3735

@JohnTortugo
Copy link
Contributor

I created a copy of the master branch and ran a build with SourceLink validation enabled. Some packages are failing the validation. Here are the results:

https://dnceng.visualstudio.com/internal/_build/results?buildId=545906&view=logs&j=a0031d5e-23bf-56e4-50ed-887ec07ba0df&t=dbd0eeef-38fc-5d8d-697e-bbcba6ebbed3&l=30

@ericstj
Copy link
Member

ericstj commented Mar 5, 2020

It looks like the commit hash used by the build doesn't exist in github: 2159123572fba25bc26b16baae8719c6432da2fa.

@danmoseley danmoseley added this to the 5.0 Preview 2 milestone Mar 10, 2020
@safern
Copy link
Member

safern commented Mar 10, 2020

It looks like the commit hash used by the build doesn't exist in github: 2159123572fba25bc26b16baae8719c6432da2fa.

I believe that was a commit added directly into a test branch by @JohnTortugo to run the validation.

@JohnTortugo if validation should be kept disabled then how should we make sure that this is not broken?

@JohnTortugo
Copy link
Contributor

@safern - symbol validation should preferably be disabled. You can download the scripts from Arcade and run them locally though. The issue with that validation is due to caching in the symbol server.. that prevents us from running the validation immediately after we publish the symbols; but we can run it a few minutes after that.

I believe that was a commit added directly into a test branch by @JohnTortugo to run the validation.

Yeah. You're probably right. To do a better validation you need to have the same commit in the open.

@danmoseley
Copy link
Member

aside: whatever the fix is here we should ideally get into preview 2, which already branched off master.

@safern
Copy link
Member

safern commented Mar 10, 2020

@safern - symbol validation should preferably be disabled. You can download the scripts from Arcade and run them locally though. The issue with that validation is due to caching in the symbol server.. that prevents us from running the validation immediately after we publish the symbols; but we can run it a few minutes after that.

Oh sorry there was a mixup here. You and @danmosemsft were speaking about the symbol validation, which makes sense why it is disabled. I believe the source link validation was initially disabled because we were hitting out of memory errors with the way validation was implemented and I believe you fixed that but we missed to update it.

I think we should enable it and fix the issues and then port it to preview2. I can look into it today if @Anipik doesn't have bandwidth to do it (as he's work on this in the past).

@JohnTortugo
Copy link
Contributor

Testing the sourcelink enabled again here: https://dnceng.visualstudio.com/internal/_build/results?buildId=554520&view=results

@JohnTortugo
Copy link
Contributor

@safern @Anipik - The validation failed again. Looks like all it's complaining is about AssemblyInfo.cs files under artifacts\obj folder.

@tmat - can you please remember me what's the appropriate fix for these cases? Is it to set EmbedUntrackedSources to true in the projects?

@tmat
Copy link
Member

tmat commented Mar 11, 2020

We are setting EmbedUntrackedSources here: https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/ProjectDefaults.props#L18
So that shouldn't be the problem, unless it's changed somewhere. What file is this exactly? What task generated it?

@JohnTortugo
Copy link
Contributor

They are all AssemblyInfo.cs files. Eg:

artifacts/obj/System.Text.Json/netcoreapp3.0-Release/_AssemblyInfo.cs
artifacts/obj/System.Text.Json/netcoreapp3.0-Release/System.Text.Json.AssemblyInfo.cs
artifacts/obj/System.Threading.Tasks.Dataflow/netstandard1.1-Release/_AssemblyInfo.cs

I'm not sure if some are auto generated or not. Perhaps @safern or @Anipik can assert that.

@safern
Copy link
Member

safern commented Mar 11, 2020

Yes AssemblyInfo.cs files are autogenerated.

@tmat
Copy link
Member

tmat commented Mar 11, 2020

@safern What target generates them?

@tmat
Copy link
Member

tmat commented Mar 11, 2020

Could dotnet/arcade#4859 be causing this?

@safern
Copy link
Member

safern commented Mar 11, 2020

The task that generates it is the Microsoft.NET.Sdk task that generates all the assembly info files when GenerateAssemblyInfo is set to true. We set the path to the file here: https://github.com/dotnet/runtime/blob/master/eng/versioning.targets#L7

Could dotnet/arcade#4859 be causing this?

Based on the fact that we use IntermediateOutputPath. It could be it, yes, but the paths seem correct for the files. However, when adding them to UntrackedSources the path/tfm could be different.

@tmat
Copy link
Member

tmat commented Mar 11, 2020

Seems like you shouldn't need to set the path, the SDK should be doing that.

@safern
Copy link
Member

safern commented Mar 11, 2020

It seems like we do it because we add more information to the AssembliInfo and then we write it ourselves: https://github.com/dotnet/runtime/blob/master/eng/versioning.targets#L70 -- So we use the SDK targets and then add more info and write the file.

@Anipik
Copy link
Contributor

Anipik commented Mar 11, 2020

This target is not getting executed

 <Target Name="_SetEmbeddedFilesFromSourceControlManagerUntrackedFiles"
          DependsOnTargets="BeforeCompile;SetEmbeddedFilesFromSourceControlManagerUntrackedFiles"
          BeforeTargets="CoreCompile"
          Condition="'$(EmbedUntrackedSources)' == 'true' and '$(EmbedAllSources)' != 'true' and '$(DebugType)' != 'none' and '$(EnableSourceControlManagerQueries)' == 'true'" />

this is due to EnableSourceControlManagerQueries being false. it is being se to false
https://github.com/dotnet/runtime/blob/master/eng/DisableSourceControlManagement.targets

@safern
Copy link
Member

safern commented Mar 11, 2020

We no longer build for RHEL6 so we might be able to just remove that file. Also, if we want to disable it we should be more granular on how we disable it and for which platforms IMHO.

@tmat
Copy link
Member

tmat commented Mar 11, 2020

_WriteNonStringAssemblyInfoAttributes is missing BeforeTargets="BeforeCompile;CoreCompile".

This is somewhat confusing though. I suggest renaming the AssemblyInfoFile and the file itself to indicate that the file is actually not the same one generated by the SDK.

@safern
Copy link
Member

safern commented Mar 12, 2020

_WriteNonStringAssemblyInfoAttributes is missing BeforeTargets="BeforeCompile;CoreCompile".
This is somewhat confusing though. I suggest renaming the AssemblyInfoFile and the file itself to indicate that the file is actually not the same one generated by the SDK.

Thanks @tmat -- @Anipik is working on it.

@Anipik can you confirm if you local run for symbol validation passed? Just so that it is in the record of the discussion that the only problem is source link?

@Anipik
Copy link
Contributor

Anipik commented Mar 12, 2020

@Anipik can you confirm if you local run for symbol validation passed?

The local validation passes for preview1 and preview2

The sourcelink is broken for packages produced in installer as well. Installer doesnot run _WriteNonStringAssemblyInfoAttributes.

GenerateAssemblyInfo and GenerateInternalsVisibleToFile are getting run before we run SetEmbeddedFilesFromSourceControlManagerUntrackedFiles
Fixing the traversal problem fixes the sourcelink.

just as note:- i used the symbol-validation script from arcade to do the symbol validation.

I still need to debug the original issue that the author is facing.

@yunwei00q
Copy link
Author

Not repro on SDK: 5.0.100-preview.2.20161.13 + D16.6/29911.98 (16.6.0 preview 1.0)

@danmoseley
Copy link
Member

This is not fixed yet.

@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label Mar 14, 2020
@ericstj
Copy link
Member

ericstj commented Mar 14, 2020

@danmosemsft can you summarize what is not fixed?

I believe the original issue (missing symbols) is no-repro.

We found another issue, which is we are missing embedded sources for build-time-generated files like assemblyinfo.cs and generated resource accessors and the like. An issue we are fixing -- but not the original posted issue, and not something likely to block someone debugging.

Let me know if I'm missing something here.

@danmoseley
Copy link
Member

I didn't verify the bug. If it's already fixed, great. My only concern was step-in works. (Don't care about assemblyinfo). If this is actually fixed, great. Hopefully we have sufficient validation to protect it or that will be added.

@danmoseley
Copy link
Member

I didn't realize only the assembly info issue was left.

@karelz karelz modified the milestones: 5.0 Preview 2, 5.0.0 Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants