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

PGO Instrumented SDK fails to load coreclr.dll #95534

Closed
DrewScoggins opened this issue Dec 1, 2023 · 6 comments
Closed

PGO Instrumented SDK fails to load coreclr.dll #95534

DrewScoggins opened this issue Dec 1, 2023 · 6 comments

Comments

@DrewScoggins
Copy link
Member

When trying to run dotnet runtime startup fails and prints the following error message

Failed to load the dll from [D:\git\dotnet-optimization\artifacts\out\sdk\x64\shared\Microsoft.NETCore.App\9.0.0-alpha.1.23577.7\coreclr.dll], HRESULT: 0x8007007E

You can obtain a copy of the instrumented SDK from here. I took a quick look at this is windbg, but all I could find is that we were getting an access violation when this error is printed. I couldn't find any symbols so I don't know where we were in startup.

Tagging @jkotas for routing.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 1, 2023
@jkotas
Copy link
Member

jkotas commented Dec 2, 2023

This was broken by #89311. The problem is that the PGO instrumented coreclr.dll depends on pgort140.dll. pgort140.dll is copied locally next to coreclr.dll. The flag added by #89311 enforces that coreclr.dll can only depend on system .dlls. Locally copied pgort140.dll is not a system .dll and that leads to the coreclr.dll load failure.

@DrewScoggins @AustinWise @elinor-fung Do you have opinions about the best way to proceed?

I can think about two options:

  • Try work to work around it by copying pgort140.dll to %WINDIR%\System32. It requires admin access and modifies global machine state on the machine that runs the optimization workloads that comes with a new set of problems. @DrewScoggins What do you think about that?
  • Revert Windows executables: only load imported DLLs from System32 #89311 and ask the C++ team to ignore the DEPENDENTLOADFLAG mismatch between instrumented and non-instrumented builds. Try again once the C++ toolset is fixed.

@AustinWise
Copy link
Contributor

AustinWise commented Dec 2, 2023

Personally, I like the second option if it is available. The linker should not be affected by the /DEPENDENTLOADFLAG flag, both when collecting and applying PGO data. This would help our immediate problem in .NET as well as help anyone else using PGO.

In terms of risks of reverting #89311, the risk is low. This is a defense-in-depth type protection. Reverting it does not expose any known security problems. I don't believe BinSkim currently checks this flag, so CI should not complain.

@jkotas
Copy link
Member

jkotas commented Dec 2, 2023

Ok, I have opened https://devdiv.visualstudio.com/DevDiv/_queries/edit/1924842/ (internal MS link) to get a fix in the linker. Let's revert #89311 for now to get the PGO data flow unblocked.

@DrewScoggins
Copy link
Member Author

This all sounds good to me. We are still waiting on this change to flow through to the installer, but once it makes it in I will make sure we get another run going to make sure this fixed the issue.

@jkotas
Copy link
Member

jkotas commented Dec 4, 2023

Fixed by #95540

@jkotas jkotas closed this as completed Dec 4, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Dec 4, 2023
@jkotas
Copy link
Member

jkotas commented Dec 4, 2023

I have opened #95598 to revert the revert once the PGO collection issue is resolved.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants