Use no-op MD Importer for DIA-backed stacktraces by hoyosjs · Pull Request #129866 · dotnet/runtime

hoyosjs · 2026-06-25T19:18:31Z

When resolving line numbers and sequence points for classic PDB-backed modules, Module::GetISymUnmanagedReader on main passes a real metadata importer into DIA. Materializing that importer can switch metadata access to the RW-backed variant, and later metadata reads can contend on its lock-heavy path.

This change avoids that transition by passing a process-wide no-op importer (NoopMetadataImport) that only satisfies the binder's non-null COM contract. Since it contains no module-linked state, it is a singleton and has no collectible-lifetime coupling.

NoopMetadataImport behavior:

QI supports IUnknown, IMetaDataImport, IMetaDataImport2 and does not expose IID_IGetIMDInternalImport.
AddRef/Release are process-lifetime no-ops.
Metadata APIs assert in Checked/Debug and return E_NOTIMPL in Release.

The binder requires a non-null metadata import interface, but the StackTrace/source-line in-proc path does not need metadata signatures from that importer. A no-op importer keeps the binder contract while avoiding the RW swap.

Performance tests

Same repro from issue: https://gist.github.com/jkotas/617109b64d9b0ad52b491cdf021b2f8a

Machine:

6 physical cores / 12 logical processors

Experiment:

2x2x2 matrix: baseline vs change, full vs portable PDB, PDB present vs deleted
5 repetitions per cell
20 seconds per repetition
Throughput metric: exceptions/sec in the harness

Results (avg exceptions/sec):

full + PDB present: 865 -> 855 (0.99x)
portable + PDB present: 14,115 -> 14,264 (1.01x)
portable + no PDB: 35,843 -> 36,545 (1.02x)
full + no PDB: 10,848 -> 26,381 (2.43x)

This matches the target scenario (full + no PDB) and keeps non-target cells effectively unchanged.

Fixes #90563

Copilot

Pull request overview

This PR changes CoreCLR’s symbol-reader setup in Module::GetISymUnmanagedReader to avoid materializing a module’s real public metadata importer when constructing DIA-backed symbol readers, by instead passing a process-wide inert IMetaDataImport2 implementation.

Changes:

Introduces NoopMetadataImport, a singleton IMetaDataImport2 implementation intended only to satisfy the binder’s non-null metadata-import parameter.
Updates Module::GetISymUnmanagedReader to pass the noop importer to ISymUnmanagedBinder::GetReaderFromStream / GetReaderForFile instead of using the module’s importer.
Wires the new files into the VM build via src/coreclr/vm/CMakeLists.txt.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/coreclr/vm/noopmetadataimport.h	Declares `NoopMetadataImport` singleton implementing `IMetaDataImport2` as an inert binder placeholder.
src/coreclr/vm/noopmetadataimport.cpp	Implements singleton creation, COM QI/AddRef/Release behavior, and E_NOTIMPL stubs for metadata APIs.
src/coreclr/vm/CMakeLists.txt	Adds the new noop importer source/header to VM build lists.
src/coreclr/vm/ceeload.cpp	Switches symbol-reader binder calls to use `NoopMetadataImport::GetInstance()` instead of module metadata importers.

Co-authored-by: Aaron R Robinson <arobins@microsoft.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

+    <!-- DIA isn't used under Native AOT at runtime, but ensure that ILC does create
+     line number blobs from full PDB that can be used in runtime stack traces. -->
+    <StackTraceLineNumberSupport>true</StackTraceLineNumberSupport>
+    <PreservePdbInHelixPayload>true</PreservePdbInHelixPayload>
+  </PropertyGroup>


Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

+    <!-- DIA isn't used under Native AOT at runtime, but ensure that ILC does create
+     line number blobs from full PDB that can be used in runtime stack traces. -->
+    <StackTraceLineNumberSupport>true</StackTraceLineNumberSupport>
+    <PreservePdbInHelixPayload>true</PreservePdbInHelixPayload>


Co-authored-by: Jan Kotas <jkotas@microsoft.com>

jkotas · 2026-07-02T02:42:23Z

/azp run runtime-nativeaot-outerloop

azure-pipelines · 2026-07-02T02:42:32Z

Azure Pipelines successfully started running 1 pipeline(s).

jkotas · 2026-07-02T02:42:42Z

/azp run runtime-extra-platforms

azure-pipelines · 2026-07-02T02:42:54Z

Azure Pipelines successfully started running 1 pipeline(s).

jkotas

LGTM if the new tests pass in CI everywhere

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

+        // C++/CLI IJW varargs is Windows-only and unavailable on Nano Server.
+        public static bool IsSupported => TestLibrary.Utilities.IsWindows && !TestLibrary.Utilities.IsWindowsNanoServer;
        [ActiveIssue("C++/CLI, IJW not supported on Mono", TestRuntimes.Mono)]
-        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        [Fact]        
        public static int TestEntryPoint()


+    <!-- Tests opt into keeping their managed pdbs in the Helix payload with PreservePdbInHelixPayload=true, which drops a
+         <AssemblyName>.PreservePdb marker beside the pdbs in the per-test output directory. The merged wrapper directory that
+         seeds _MergedPayloadFiles is a flattened copy of every test's assemblies and pdbs but omits those marker files, so
+         probe the original per-test output tree ($(TestBinDir)) for the markers and collect the names of the pdbs that sit
+         next to them. A pdb with one of these names is kept below; because each pdb name only exists in its own merge group's
+         flattened payload, this preserves exactly the opted-in pdbs and lands them next to their dll. -->
+    <ItemGroup Condition="'@(_MergedPayloadFiles)' != ''">
+      <_PreservePdbMarkers Include="$(TestBinDir)**\*.PreservePdb" />
+      <_PreservePdbMarkerDirectories Include="@(_PreservePdbMarkers->'%(RootDir)%(Directory)')" KeepDuplicates="false" />
+      <_PreservePdbSourceFiles Include="%(_PreservePdbMarkerDirectories.Identity)**\*.pdb" Condition="'@(_PreservePdbMarkerDirectories)' != ''" />
+    </ItemGroup>
+    <PropertyGroup Condition="'@(_MergedPayloadFiles)' != ''">
+      <_PreservePdbFileNames>;@(_PreservePdbSourceFiles->'%(Filename).pdb');</_PreservePdbFileNames>
+    </PropertyGroup>


+        if (riid == IID_IUnknown ||
+            riid == IID_IMetaDataImport ||
+            riid == IID_IMetaDataImport2)
+        {
+            *ppvObject = static_cast<IMetaDataImport2*>(this);
+            return S_OK;
+        }


Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

+        {
+            *ppvObject = static_cast<IMetaDataImport2*>(this);
+            return S_OK;
+        }


+            Type testType = ijwNativeDll.GetType("TestClass");
+            MethodInfo throwMethod = testType.GetMethod("ThrowFromManaged");


+            StackFrame ijwFrame = null;
+            foreach (StackFrame frame in new StackTrace(thrown, fNeedFileInfo: true).GetFrames())
+            {


+  <PropertyGroup>
+    <!-- A classic Windows PDB (DebugType=Full) is required to exercise the diasymreader path,
+         and it can only be emitted on Windows. The test itself is Windows-only. -->
+    <DebugType Condition="'$(TargetOS)' == 'windows'">Full</DebugType>
+    <DebugSymbols>true</DebugSymbols>
+    <Optimize>false</Optimize>
+    <!-- DIA isn't used under Native AOT at runtime, but ensure that ILC does create
+     line number blobs from full PDB that can be used in runtime stack traces. -->
+    <StackTraceLineNumberSupport>true</StackTraceLineNumberSupport>
+    <PreservePdbInHelixPayload>true</PreservePdbInHelixPayload>
+  </PropertyGroup>


jkotas · 2026-07-02T16:15:31Z

fullpdbstacktrace is failing on CoreCLR, and the tests need to be disabled on Mono

Use noop IMDImporter in dia

9caad3e

Copilot AI review requested due to automatic review settings June 25, 2026 19:18

github-actions Bot added the area-VM-coreclr label Jun 25, 2026

hoyosjs requested review from AaronRobinsonMSFT, davidwrighton and jkotas June 25, 2026 19:19

Copilot started reviewing on behalf of hoyosjs June 25, 2026 19:19 View session

dotnet-policy-service Bot assigned hoyosjs Jun 25, 2026

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.h Outdated

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

AaronRobinsonMSFT reviewed Jun 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.h

AaronRobinsonMSFT reviewed Jun 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

Rect to aaron's feedback

45679f8

AaronRobinsonMSFT approved these changes Jun 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

Apply suggestion from @AaronRobinsonMSFT

57c848b

Co-authored-by: Aaron R Robinson <arobins@microsoft.com>

Copilot AI review requested due to automatic review settings June 25, 2026 23:28

hoyosjs enabled auto-merge (squash) June 25, 2026 23:28

Copilot started reviewing on behalf of hoyosjs June 25, 2026 23:28 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

Comment thread src/coreclr/vm/ceeload.cpp Outdated

Comment thread src/coreclr/vm/ceeload.cpp Outdated

Fix feedback

5514c7f

AaronRobinsonMSFT approved these changes Jun 26, 2026

View reviewed changes

jkotas reviewed Jun 26, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

Comment thread src/coreclr/inc/corpriv.h Outdated

Comment thread src/coreclr/md/runtime/noopmetadataimport.cpp

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

jkotas reviewed Jun 26, 2026

View reviewed changes

Comment thread src/coreclr/vm/noopmetadataimport.cpp Outdated

React to feedback

5ac1296

Copilot AI review requested due to automatic review settings June 28, 2026 06:46

Copilot started reviewing on behalf of hoyosjs June 28, 2026 06:47 View session

Copilot AI reviewed Jun 28, 2026

View reviewed changes

Comment thread src/coreclr/md/runtime/noopmetadataimport.cpp

jkotas reviewed Jun 28, 2026

View reviewed changes

Comment thread src/coreclr/md/runtime/noopmetadataimport.cpp Outdated

Nit fix

073de0a

davidwrighton approved these changes Jun 29, 2026

View reviewed changes

AaronRobinsonMSFT reviewed Jun 29, 2026

View reviewed changes

Comment thread src/coreclr/vm/ceeload.cpp Outdated

Fix tests and feedback

f1557f0

Copilot AI review requested due to automatic review settings July 2, 2026 00:21

Copilot started reviewing on behalf of hoyosjs July 2, 2026 00:21 View session

Remove unnecessary guard

d76f4a3

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 2, 2026 00:27

Copilot started reviewing on behalf of hoyosjs July 2, 2026 00:28 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

jkotas reviewed Jul 2, 2026

View reviewed changes

Comment thread src/tests/Interop/IJW/NativeVarargs/NativeVarargsTest.cs Outdated

Comment thread src/tests/Interop/IJW/NativeVarargs/NativeVarargsTest.cs Outdated

Apply suggestions from code review

0267dd5

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

Copilot AI review requested due to automatic review settings July 2, 2026 02:38

Copilot started reviewing on behalf of jkotas July 2, 2026 02:39 View session

jkotas reviewed Jul 2, 2026

View reviewed changes

Comment thread src/tests/Interop/IJW/NativeVarargs/NativeVarargsTest.cs Outdated

jkotas added 3 commits July 1, 2026 19:40

Apply suggestion from @jkotas

787536c

Update NativeVarargsTest.cs

0ed1727

Fix formatting of Fact attribute in NativeVarargsTest

e38aa9f

jkotas approved these changes Jul 2, 2026

View reviewed changes

Copilot AI reviewed Jul 2, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 2, 2026 02:45

Copilot started reviewing on behalf of hoyosjs July 2, 2026 02:46 View session

Copilot AI reviewed Jul 2, 2026

View reviewed changes

		Type testType = ijwNativeDll.GetType("TestClass");
		MethodInfo throwMethod = testType.GetMethod("ThrowFromManaged");

Uh oh!

Conversation

hoyosjs commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas commented Jul 2, 2026

Uh oh!

azure-pipelines Bot commented Jul 2, 2026

Uh oh!

jkotas commented Jul 2, 2026

Uh oh!

azure-pipelines Bot commented Jul 2, 2026

Uh oh!

jkotas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jkotas commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

hoyosjs commented Jun 25, 2026 •

edited

Loading

jkotas left a comment •

edited

Loading