dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.47k stars 4.76k forks source link

crossgen2 `--pdb` does not work when the output PDB is not named the way MSDiaReader expects #78230

Open rickbrew opened 2 years ago

rickbrew commented 2 years ago

Description

Specifying --pdb on the crossgen2 command line results in an error after writing the new DLL(s):

2>System.Runtime.InteropServices.COMException (0x80004005)
2>   at ILCompiler.Diagnostics.PdbWriter.CreateNGenPdbWriter(String, String, IntPtr&) + 0xce
2>   at ILCompiler.Diagnostics.PdbWriter.WritePDBDataHelper(String, IEnumerable`1) + 0x195
2>   at ILCompiler.Diagnostics.PdbWriter.WritePDBData(String, IEnumerable`1) + 0x1b
2>   at ILCompiler.DependencyAnalysis.ReadyToRunObjectWriter.EmitPortableExecutable() + 0x9ce
2>   at ILCompiler.ReadyToRunCodegenCompilation.Compile(String) + 0x18d
2>   at ILCompiler.Program.RunSingleCompilation(Dictionary`2, InstructionSetSupport, String, Dictionary`2, HashSet`1, CompilerTypeSystemContext) + 0x17b0
2>   at ILCompiler.Program.Run(String[]) + 0xd6b
2>   at ILCompiler.Program.Main(String[]) + 0x32

Reproduction Steps

Add --pdb to your crossgen2 command line args

Expected behavior

It works

Actual behavior

It does not work

Regression?

No; this was also bugged in .NET 6

Known Workarounds

I need the PDBs so that plugin developers can set breakpoints and do debugging. For now, they can set DOTNET_ReadyToRun=0.

Configuration

No response

Other information

No response

EgorBo commented 2 years ago

Probably related https://github.com/dotnet/runtime/issues/13134 and https://github.com/dotnet/runtime/pull/70407 cc @trylek

am11 commented 2 years ago

On Unix, should --pdb be a no-op? Currently, it throws an unhanded exception: System.DllNotFoundException: Unable to load shared library 'Microsoft.DiaSymReader.Native.arm64.dll' or one of its dependencies. (since Microsoft.DiaSymReader.Native is Windows-only for non-portable PDBs)

trylek commented 2 years ago

Hmm, @rickbrew - can you please double-check what exact version of Crossgen2 you're using? I believe I routinely use the --pdb option in my local testing without any issues and I also think that the runtime repo builds the System.Private.CoreLib.dll with the --pdb option on Windows so we should see if it regressed completely. Also please share what architecture you're using, I do admit I most often build for x64 locally. It's probably needless to say that the option --pdb only works on Windows as the PDB writer is a proprietary Microsoft technology that is not open source and not available on Unix. I have a PR open for adding broader PDB validation but I'm still having trouble making it work in the Helix lab so it hasn't yet been merged in.

trylek commented 2 years ago

It should probably issue a warning along the lines of my previous response and silently skip PDB generation, I believe I own an issue for that.

rickbrew commented 2 years ago

Hmm, @rickbrew - can you please double-check what exact version of Crossgen2 you're using?

I'm using 7.0.0 -- I pull down the latest version via nuget with respect to the version of .NET that I'm targeting. (specifically, crossgen2 -v prints 7.0.0-rtm.22518.5+d099f075e45d2aa6007a22b71b45a08758559f80)

Also please share what architecture you're using, I do admit I most often build for x64 locally.

x64 on Windows 11 build 22623.885. It happens for arm64 as well (on x64 -- I'm not building on arm64)

jakobbotsch commented 2 years ago

@rickbrew I have hit this problem when specifying a "non-default" output file name. For some reason the native pdb writer is picky that the output file name matches the input file name. Does it work if you specify -o <input file name>.r2r.dll?

trylek commented 2 years ago

Thanks @jakobbotsch for your comment, that's actually quite interesting, @davidwrighton who originally implemented PDB support in R2RDump I subsequently ported to Crossgen2 had to make some non-trivial hoops to satisfy the MSDiaSymReader expectations regarding PDB file naming. Having said that, Crossgen2 should internally emit the PDB under the name DIA is fine with and subsequently rename it as necessary, if that logic is broken, that's certainly worrisome and worth fixing.

jakobbotsch commented 2 years ago

Yeah, actually the .r2r.dll suffix I suggested above is not ok either. It has to match exactly:

❯ C:\dev\dotnet\runtime4\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\crossgen2\crossgen2.exe -r:*.dll .\playground.dll --pdb -o .\foobar\playground.dll
Emitting R2R PE file: .\foobar\playground.dll
Emitting PDB file: .\foobar\playground.ni.pdb

❯ C:\dev\dotnet\runtime4\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\crossgen2\crossgen2.exe -r:*.dll .\playground.dll --pdb -o .\foobar\playground.r2r.dll
Emitting R2R PE file: .\foobar\playground.r2r.dll
Emitting PDB file: .\foobar\playground.r2r.ni.pdb
Unhandled exception. System.Runtime.InteropServices.COMException (0x80004005): Error HRESULT E_FAIL has been returned from a call to a COM component.
   at ILCompiler.Diagnostics.PdbWriter.CreateNGenPdbWriter(String ngenImagePath, String pdbPath, IntPtr& ngenPdbWriterPtr)
   at ILCompiler.Diagnostics.PdbWriter.WritePDBDataHelper(String dllPath, IEnumerable`1 methods) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.Diagnostics\PdbWriter.cs:line 212
   at ILCompiler.Diagnostics.PdbWriter.WritePDBData(String dllPath, IEnumerable`1 methods) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.Diagnostics\PdbWriter.cs:line 141
   at ILCompiler.PEWriter.SymbolFileBuilder.SavePdb(String pdbPath, String dllFileName) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\ObjectWriter\SymbolFileBuilder.cs:line 31
   at ILCompiler.DependencyAnalysis.ReadyToRunObjectWriter.EmitPortableExecutable() in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\CodeGen\ReadyToRunObjectWriter.cs:line 404
   at ILCompiler.DependencyAnalysis.ReadyToRunObjectWriter.EmitObject(String objectFilePath, EcmaModule componentModule, IEnumerable`1 inputFiles, IEnumerable`1 nodes, NodeFactory factory, Boolean generateMapFile, Boolean generateMapCsvFile, Boolean generatePdbFile, String pdbPath, Boolean generatePerfMapFile, String perfMapPath, Int32 perfMapFormatVersion, Boolean generateProfileFile, CallChainProfile callChainProfile, Int32 customPESectionAlignment) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\CodeGen\ReadyToRunObjectWriter.cs:line 547
   at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\Compiler\ReadyToRunCodegenCompilation.cs:line 356
   at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, CompilerTypeSystemContext typeSystemContext) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Program.cs:line 598
   at ILCompiler.Program.Run() in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Program.cs:line 289
   at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass184_0.<.ctor>b__0(InvocationContext context) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Crossgen2RootCommand.cs:line 290
   at System.CommandLine.Invocation.AnonymousCommandHandler.Invoke(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass4_0.<<BuildInvocationChain>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass17_0.<<UseParseErrorReporting>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass12_0.<<UseHelp>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass23_0.<<UseVersionOption>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Invocation.InvocationPipeline.<Invoke>g__FullInvocationChain|3_0(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.Invoke(IConsole console)
   at System.CommandLine.Parsing.ParseResultExtensions.Invoke(ParseResult parseResult, IConsole console)
   at System.CommandLine.Parsing.ParserExtensions.Invoke(Parser parser, String[] args, IConsole console)
   at ILCompiler.Program.Main(String[] args) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Program.cs:line 874

❯ C:\dev\dotnet\runtime4\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\crossgen2\crossgen2.exe -r:*.dll .\playground.dll --pdb -o .\foobar\somethingelse.dll
Emitting R2R PE file: .\foobar\somethingelse.dll
Emitting PDB file: .\foobar\somethingelse.ni.pdb
Unhandled exception. System.Runtime.InteropServices.COMException (0x80004005): Error HRESULT E_FAIL has been returned from a call to a COM component.
   at ILCompiler.Diagnostics.PdbWriter.CreateNGenPdbWriter(String ngenImagePath, String pdbPath, IntPtr& ngenPdbWriterPtr)
   at ILCompiler.Diagnostics.PdbWriter.WritePDBDataHelper(String dllPath, IEnumerable`1 methods) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.Diagnostics\PdbWriter.cs:line 212
   at ILCompiler.Diagnostics.PdbWriter.WritePDBData(String dllPath, IEnumerable`1 methods) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.Diagnostics\PdbWriter.cs:line 141
   at ILCompiler.PEWriter.SymbolFileBuilder.SavePdb(String pdbPath, String dllFileName) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\ObjectWriter\SymbolFileBuilder.cs:line 31
   at ILCompiler.DependencyAnalysis.ReadyToRunObjectWriter.EmitPortableExecutable() in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\CodeGen\ReadyToRunObjectWriter.cs:line 404
   at ILCompiler.DependencyAnalysis.ReadyToRunObjectWriter.EmitObject(String objectFilePath, EcmaModule componentModule, IEnumerable`1 inputFiles, IEnumerable`1 nodes, NodeFactory factory, Boolean generateMapFile, Boolean generateMapCsvFile, Boolean generatePdbFile, String pdbPath, Boolean generatePerfMapFile, String perfMapPath, Int32 perfMapFormatVersion, Boolean generateProfileFile, CallChainProfile callChainProfile, Int32 customPESectionAlignment) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\CodeGen\ReadyToRunObjectWriter.cs:line 547
   at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\ILCompiler.ReadyToRun\Compiler\ReadyToRunCodegenCompilation.cs:line 356
   at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, CompilerTypeSystemContext typeSystemContext) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Program.cs:line 598
   at ILCompiler.Program.Run() in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Program.cs:line 289
   at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass184_0.<.ctor>b__0(InvocationContext context) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Crossgen2RootCommand.cs:line 290
   at System.CommandLine.Invocation.AnonymousCommandHandler.Invoke(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.<>c__DisplayClass4_0.<<BuildInvocationChain>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass17_0.<<UseParseErrorReporting>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass12_0.<<UseHelp>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.CommandLineBuilderExtensions.<>c__DisplayClass23_0.<<UseVersionOption>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at System.CommandLine.Invocation.InvocationPipeline.<Invoke>g__FullInvocationChain|3_0(InvocationContext context)
   at System.CommandLine.Invocation.InvocationPipeline.Invoke(IConsole console)
   at System.CommandLine.Parsing.ParseResultExtensions.Invoke(ParseResult parseResult, IConsole console)
   at System.CommandLine.Parsing.ParserExtensions.Invoke(Parser parser, String[] args, IConsole console)
   at ILCompiler.Program.Main(String[] args) in C:\dev\dotnet\runtime4\src\coreclr\tools\aot\crossgen2\Program.cs:line 874
rickbrew commented 2 years ago

I've been using paintdotnet.ni.dll for my DLL name -- do I have to use paintdotnet.dll? Won't that overwrite the real paintdotnet.dll?

jakobbotsch commented 2 years ago

The way the SDK does it is by emitting it with the same name but into another directory, so I suspect that's going to be the easiest workaround until we fix the actual issue.

rickbrew commented 2 years ago

It does successfully generate a pdb file if I use paintdotnet.dll as the name. However, then the rest of my build process fumbles -- how can I get the name to not collide with the original DLL's name?

trylek commented 2 years ago

Thanks Jakob and Rick for sharing your additional findings. While in theory this bit of code is supposed to handle this case,

https://github.com/dotnet/runtime/blob/28cf1719704d33a677990cf487af5719b6389f2a/src/coreclr/tools/aot/ILCompiler.Diagnostics/PdbWriter.cs#L194

for some reason it's apparently not kicking in. I'll self-assign this issue and I'll investigate what the problem is.

davidwrighton commented 2 years ago

@rickbrew the naming issue I would recommend working around, by either emitting the R2R binary to a separate directory in crossgen2.

We do this in 2 basic ways in our systems.

  1. The publish pipeline emits the R2R images into the publish directory which is separate from the build directory.
  2. Our test system works differently, when it decides to run crossgen2, it copies all of the IL versions of the code to an IL directory and then runs crossgen2 over each of those, emitting the R2R binary back to the original build location.
teo-tsirpanis commented 1 year ago

(setting to Future since it has no millestone and the untriaged label was removed, please change it if you plan to do it in 8)

trylek commented 10 months ago

I have clarified the naming of this issue to avoid its confusion with the recent regression in overall PDB generation tracked under https://github.com/dotnet/runtime/issues/96917.