Skip to content

Conversation

leculver
Copy link
Contributor

@leculver leculver commented Jan 5, 2023

A lot of changes are required for this upgrade. Nearly all of them were originally crafted with the end in mind of going with NativeAOT, but that doesn't support win-x86, so we're sticking with DNNE, but otherwise most of the changes @leculver prepared are good and forward-looking.

- Update the dbgeng extension to use NativeAOT.
- Update the dbgeng extension to use the new debugger plugin paradigm in the most recent ClrMD.
@leculver
Copy link
Contributor Author

leculver commented Jan 5, 2023

@AArnott (From our email discussion.) Unfortunately I was not able to build or test this change. So, if you decide this is the direction you want to go in, you’ll need to build/test the changes I’ve made. I’m about to go on vacation so I’m unfortunately out of time on this project. :(

@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

Thanks. I'll take a look. Unfortunately it appears you based your changes on a commit from before #1138 merged, so you not only have merge conflicts, but that may be why the diff in this PR wasn't conveying what you expected it to. I'd like to try out NativeAOT though, so I'll see what I can make of your PR.

@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

@leculver, I merged main into your branch. It doesn't build, I think because you didn't add/update any PackageReference or PackageVersion items, so the new work you did in another repo doesn't show up here. We're getting errors such as:

1>C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\DumpAsyncCommand.cs(8,47,8,53): error CS0234: The type or namespace name 'DbgEng' does not exist in the namespace 'Microsoft.Diagnostics.Runtime.Utilities' (are you missing an assembly reference?)
1>C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\SOSLinkedCommand.cs(4,7,4,22): error CS0246: The type or namespace name 'DbgEngExtension' could not be found (are you missing a using directive or an assembly reference?)
1>C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\SOSLinkedCommand.cs(5,47,5,53): error CS0234: The type or namespace name 'DbgEng' does not exist in the namespace 'Microsoft.Diagnostics.Runtime.Utilities' (are you missing an assembly reference?)
1>C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\SOSLinkedCommand.cs(9,35,9,48): error CS0246: The type or namespace name 'DbgEngCommand' could not be found (are you missing a using directive or an assembly reference?)

Do you have time to push another commit?

Unfortunately I was not able to build or test this change

From your email it sounds like you simply don't have the required SDK installed. This repo requires the .NET 7.0.101 SDK (allowing for higher patch versions). You can either install that SDK (which our init.ps1 script will do for you) or you can change the rollForward property in global.json to allow someting later that you might be using (e.g. major).

@leculver
Copy link
Contributor Author

leculver commented Jan 6, 2023

Oh, that fixed it. The init.ps1 script did fix it, but I'm not sure how I didn't have that .Net 7 (or greater) build on my machine. I was just testing something using that SDK on Monday! Strange...

The errors you see are due to the Microsoft.Diagnostics.Runtime.Utilities package not being fully updated to this one: https://www.nuget.org/packages/Microsoft.Diagnostics.Runtime.Utilities/2.3.405501 which was shipped yesterday.

I attempted to update to that version, but I'm getting NuGet errors. In particular, this feed seems to have Microsoft.Diagnostics.Runtime.Utilities packages for versions I've never shipped:
https://pkgs.dev.azure.com/azure-public/3ccf6661-f8ce-4e8a-bb2e-eff943ddd3c7/_packaging/36a629e1-6c5b-4bcd-aa2e-6018802d6b99/nuget/v3/flat2/microsoft.diagnostics.runtime.utilities/index.json

Are you using a different M.D.R.U library? Can you update that package feed to point to the real one on NuGet?

@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

The feed we consume from uses nuget.org as an upstream, so all I had to do was switch the Directory.Packages.props file over to the new versions and the compilation worked. I've pushed that change to your branch. I'm testing out your changes now.

@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

The native code generation step fails for Debug builds:

dotnet publish -r win-x64 -f net7.0-windows -c debug

C:\_nugetpackages\runtime.win-x64.microsoft.dotnet.ilcompiler\7.0.1\framework\System.Net.Http.dll : warning IL3053: Assembly 'System.Net.Http' produced AOT analysis warnings. [C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\SosThreadingTools.csproj::TargetFramework=net7.0-windows]
CVTRES : fatal error CVT1103: cannot read file [C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\SosThreadingTools.csproj::TargetFramework=net7.0-windows]
LINK : fatal error LNK1123: failure during conversion to COFF: file invalid or corrupt [C:\Users\andarno\source\repos\vs-threading\src\SosThreadingTools\SosThreadingTools.csproj::TargetFramework=net7.0-windows]

The release configuration builds just fine (with the System.Net.Http warning notwithstanding), and it appears to work, so that's great.

@leculver
Copy link
Contributor Author

leculver commented Jan 6, 2023

That is unfortunately expected, and should be ignored.

ClrMD does use System.Net.Http, but not in any way that triggers a NativeAOT failure (it doesn't go down codepaths using reflection).

I'm not clear on whether the Http library simply hasn't been updated to fully support NativeAOT (not all libraries have). Or whether there's a real problem with Http and we should be using something else when making symbol server requests.

Either way, I haven't had any issues with ClrMD+NativeAOT, despite what the warning says.

@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

Any idea why the debug build fails to produce native code?

I'm also curious about the dev inner loop, considering any change to SosThreadingTools made locally will have to be run through dotnet publish before a binary that can actually be loaded by windbg is produced.

@AArnott AArnott marked this pull request as draft January 6, 2023 17:57
@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

Apparently the debug config failure was an incremental build issue. It worked after I cleaned the repo.

@leculver
Copy link
Contributor Author

leculver commented Jan 6, 2023

Any idea why the debug build fails to produce native code?

Ah glad you figured that out.

I'm also curious about the dev inner loop

So I actually build the library as a pure IL library too. I load up DbgEng from disk, load a crash dump, then call my extension directly. Here's the startup path: https://github.com/microsoft/clrmd/blob/main/src/Samples/DbgEngStandalone/Program.cs#L20-L35

Then for my DbgEng commands I simply initialize with that DbgEng instance and run them directly (unit tests would work similarly): https://github.com/microsoft/clrmd/blob/main/src/Samples/DbgEngStandalone/Program.cs#L20-L35

This leads to a really good inner-dev loop...it's all C# code (not compiled to NativeAOT), and my F5 loads dbgeng as a dll...you don't have to interact with windbg/cdb just to test.

I didn't add that constructor override to your code, one sec I'm going to push a change to add that.

@AArnott
Copy link
Member

AArnott commented Jan 6, 2023

Ooh... I just saw this error:

Native compilation does not support targeting win-x86 yet.

We build a win-x86 binary so it can be loaded into x86 WinDbg for x86 process debugging. It looks like this is still required, as WinDbgX's debug engine crashes when we try to load 64-bit SosThreadingTools for a 32-bit dump file.

@leculver
Copy link
Contributor Author

leculver commented Jan 6, 2023

Ah shoot. I've been doing pure x64 debugging, so I never ran into that. I suppose this won't work for you after all. =(

@AArnott AArnott changed the title Update plugin to use NativeAOT Update Microsoft.Diagnostics.Runtime.Utilities to first publicly stable version Jan 6, 2023
@AArnott AArnott added this to the v17.6 milestone Jan 6, 2023
This is because AOT doesn't support win-x86.
@AArnott AArnott marked this pull request as ready for review January 6, 2023 23:51
@AArnott AArnott merged commit 80a9b39 into microsoft:main Jan 7, 2023
@leculver leculver deleted the AOT branch January 7, 2023 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants