Force MetaDataGetDispenser to be linked into singlefilehost#127036
Force MetaDataGetDispenser to be linked into singlefilehost#127036elinor-fung wants to merge 3 commits intodotnet:mainfrom
Conversation
Add linker flags on Unix/macOS to force MetaDataGetDispenser as an undefined symbol, which makes the linker pull mscoree.o from coreclr_static. On Windows, the .def file already forces this. Add a test that validates the export is present in the singlefilehost binary using NativeLibrary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ensures singlefilehost continues to export MetaDataGetDispenser (regression in .NET 10 due to static-library dead-code elimination) by forcing the symbol to remain undefined at link time so the relevant object file is pulled in, and adds a regression test to validate expected exports are present.
Changes:
- Add Unix/macOS linker options to force
MetaDataGetDispenserto be pulled fromcoreclr_staticintosinglefilehost. - Add an installer test that validates key
singlefilehostexports (includingMetaDataGetDispenser) are present.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/native/corehost/apphost/static/CMakeLists.txt | Adds linker flags to force MetaDataGetDispenser to be linked into singlefilehost on non-Windows. |
| src/installer/tests/AppHost.Bundle.Tests/StaticHost.cs | Adds a test that checks singlefilehost exports on Windows (NativeLibrary) and non-Windows (nm). |
|
@copilot apply changes based on the comments in this thread |
| # This should be kept in sync with singlefilehost_unixexports.src. | ||
| if(CLR_CMAKE_TARGET_APPLE) | ||
| target_link_options(singlefilehost PRIVATE | ||
| -Wl,-u,_DotNetRuntimeInfo |
There was a problem hiding this comment.
Should we instead annotate these properly, e.g.:
--- a/src/coreclr/debug/runtimeinfo/runtimeinfo.cpp
+++ b/src/coreclr/debug/runtimeinfo/runtimeinfo.cpp
@@ -13,8 +13,9 @@
#include <runtime_version.h>
// Runtime information public export
-#ifdef HOST_UNIX
DLLEXPORT
+#ifdef HOST_UNIX
+__attribute__((used, retain))
#endif
RuntimeInfo DotNetRuntimeInfo = {
{and then cleanup from various scripts like:
src/native/corehost/apphost/static/singlefilehost.def:DotNetRuntimeInfo @4 data
src/native/corehost/apphost/static/singlefilehost_freebsdexports.src:DotNetRuntimeInfo
src/native/corehost/apphost/static/singlefilehost_unixexports.src:DotNetRuntimeInfo
There was a problem hiding this comment.
Maybe I'm missing something, but with just the attributes, MetaDataGetDispenser seems to no longer be retained?
--- a/src/coreclr/dlls/mscoree/mscoree.cpp
+++ b/src/coreclr/dlls/mscoree/mscoree.cpp
@@ -41,7 +41,11 @@ BOOL WINAPI DllMain(HANDLE hInstance, DWORD dwReason, LPVOID lpReserved)
// This function gets the Dispenser interface given the CLSID and REFIID.
// Exported from coreclr and used by external profilers.
// ---------------------------------------------------------------------------
-STDAPI DLLEXPORT MetaDataGetDispenser( // Return HRESULT
+STDAPI DLLEXPORT
+#ifdef HOST_UNIX
+__attribute__((used, retain))
+#endif
+MetaDataGetDispenser( // Return HRESULT
REFCLSID rclsid, // The class to desired.> nm -D --defined-only ./artifacts/bin/linux-x64.Release/corehost/singlefilehost
0000000000bb9340 D DotNetRuntimeInfo@@V1.0
0000000000000000 A V1.0
0000000000bc6708 B g_dacTable@@V1.0
There was a problem hiding this comment.
Ah, this is using linker script, which overrides what to keep. I wonder why linker script isn't keeping MetaDataGetDispenser as it's already there?
This workaround looks good to me, I will take a look later what can be done to make these symbols always available. I was just wondering since we used a similar linker workaround in NativeAOT BuildIntegration
but it was only done for the API which wasn't owned by us. 🤔
Add linker flags on Unix/macOS to force MetaDataGetDispenser as an undefined symbol, which makes the linker pull mscoree.o from coreclr_static. On Windows, the .def file already forces this.
Add a test that validates the export is present in the singlefilehost binary using NativeLibrary.
See #126634. We'll want to backport to 10.0.
cc @dotnet/appmodel @dotnet/dotnet-diag