Fix static virtual methods for delegate creation - #118470

Merged
davidwrighton merged 2 commits into
dotnet:mainfrom
davidwrighton:fix_svm_composite_build
Aug 7, 2025
Merged

Fix static virtual methods for delegate creation#118470
davidwrighton merged 2 commits into
dotnet:mainfrom
davidwrighton:fix_svm_composite_build

Conversation

@davidwrighton

@davidwrightondavidwrighton commented Aug 6, 2025

Copy link
Copy Markdown
Member
  • Notably for creation of delegates to SVM methods which do not require generic dictionary lookups
  • Also a drive by fix of the vtable handling for NativeImageLayout which was causing crashes of the DAC when used in debug builds

Fixes#109200

@davidwrighton
Fix static virtual methods for delegate creation
b2bac6e
- Notably for creation of delegates to SVM methods which do not require generic dictionary lookups
- Also a drive by fix of the vtable handling for NativeImageLayout which was causing crashes of the DAC when used in debug builds
Fixesdotnet#109200
@davidwrighton
Update issues.targets
707b5c0
CopilotAI review requested due to automatic review settings August 6, 2025 22:12
CopilotAI reviewed Aug 6, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes static virtual method (SVM) delegate creation that was causing crashes when creating delegates to SVM methods that don't require generic dictionary lookups. It also includes a drive-by fix for vtable handling in NativeImageLayout that was causing DAC crashes in debug builds.

Key changes:

  • Adds type handle setting for static methods during method decoding
  • Fixes vtable declaration for NativeImageLayout to work properly with DAC
  • Removes test exclusions that were previously failing due to the fixed issue

Reviewed Changes

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

FileDescription
src/tests/issues.targetsRemoves exclusions for StaticVirtualMethods tests that are now fixed
src/coreclr/vm/zapsig.cppAdds type handle assignment for static methods in DecodeMethod
src/coreclr/vm/peimagelayout.hFixes DACCESS_COMPILE conditional placement for NativeImageLayout
src/coreclr/inc/vptr_list.hAdds missing VPTR_CLASS entry for NativeImageLayout

@dotnet-policy-service

dotnet-policy-serviceBot commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

@davidwrighton

davidwrighton commented Aug 6, 2025

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2-composite

@azure-pipelines

azure-pipelinesBot commented Aug 6, 2025

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dotnetdotnet deleted a comment from azure-pipelinesBotAug 6, 2025
@davidwrighton
davidwrighton requested a review from kgAugust 6, 2025 22:14
kg
kg approved these changes Aug 6, 2025
View reviewed changes
@davidwrighton
davidwrighton merged commit ac85d6f into dotnet:mainAug 7, 2025
115 of 117 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Sep 7, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@davidwrightondavidwrighton

Labels

area-VM-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Crossgen outerloop regression: Failures in GenericContextTest / GenericContextTestDefaultImp

3 participants

@davidwrighton@kg
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); })();
Skip to content

Fix static virtual methods for delegate creation - #118470

Merged
davidwrighton merged 2 commits into
dotnet:mainfrom
davidwrighton:fix_svm_composite_build
Aug 7, 2025
Merged

Fix static virtual methods for delegate creation#118470
davidwrighton merged 2 commits into
dotnet:mainfrom
davidwrighton:fix_svm_composite_build

Conversation

@davidwrighton

@davidwrightondavidwrighton commented Aug 6, 2025

Copy link
Copy Markdown
Member
  • Notably for creation of delegates to SVM methods which do not require generic dictionary lookups
  • Also a drive by fix of the vtable handling for NativeImageLayout which was causing crashes of the DAC when used in debug builds

Fixes#109200

@davidwrighton
Fix static virtual methods for delegate creation
b2bac6e
- Notably for creation of delegates to SVM methods which do not require generic dictionary lookups
- Also a drive by fix of the vtable handling for NativeImageLayout which was causing crashes of the DAC when used in debug builds
Fixesdotnet#109200
@davidwrighton
Update issues.targets
707b5c0
CopilotAI review requested due to automatic review settings August 6, 2025 22:12
CopilotAI reviewed Aug 6, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes static virtual method (SVM) delegate creation that was causing crashes when creating delegates to SVM methods that don't require generic dictionary lookups. It also includes a drive-by fix for vtable handling in NativeImageLayout that was causing DAC crashes in debug builds.

Key changes:

  • Adds type handle setting for static methods during method decoding
  • Fixes vtable declaration for NativeImageLayout to work properly with DAC
  • Removes test exclusions that were previously failing due to the fixed issue

Reviewed Changes

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

FileDescription
src/tests/issues.targetsRemoves exclusions for StaticVirtualMethods tests that are now fixed
src/coreclr/vm/zapsig.cppAdds type handle assignment for static methods in DecodeMethod
src/coreclr/vm/peimagelayout.hFixes DACCESS_COMPILE conditional placement for NativeImageLayout
src/coreclr/inc/vptr_list.hAdds missing VPTR_CLASS entry for NativeImageLayout

@dotnet-policy-service

dotnet-policy-serviceBot commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

@davidwrighton

davidwrighton commented Aug 6, 2025

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2-composite

@azure-pipelines

azure-pipelinesBot commented Aug 6, 2025

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dotnetdotnet deleted a comment from azure-pipelinesBotAug 6, 2025
@davidwrighton
davidwrighton requested a review from kgAugust 6, 2025 22:14
kg
kg approved these changes Aug 6, 2025
View reviewed changes
@davidwrighton
davidwrighton merged commit ac85d6f into dotnet:mainAug 7, 2025
115 of 117 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Sep 7, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@davidwrightondavidwrighton

Labels

area-VM-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Crossgen outerloop regression: Failures in GenericContextTest / GenericContextTestDefaultImp

3 participants

@davidwrighton@kg