Skip to content

Remove calli target tagging from CoreCLR interop stubs - #131642

Merged
jkotas merged 22 commits into
mainfrom
copilot/delete-target-shifting-call
Aug 3, 2026
Merged

Remove calli target tagging from CoreCLR interop stubs#131642
jkotas merged 22 commits into
mainfrom
copilot/delete-target-shifting-call

Conversation

CopilotAI commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

This change removes the (target << 1) | 1 encoding previously used for marshaling-required unmanaged calli paths. MethodDesc and unmanaged target handling now follow their actual storage semantics instead of relying on low-bit tagging.

Fixes#131606

CopilotAIand others added 3 commits July 31, 2026 12:34
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 31, 2026 12:56

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 6 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Comment threadtest.cs Outdated
Comment threadsrc/coreclr/vm/frames.h Outdated
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 31, 2026 13:38

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Comment threadsrc/coreclr/jit/lower.cpp Outdated
Comment threadsrc/coreclr/jit/lower.cpp Outdated
Comment threadsrc/coreclr/vm/frames.h Outdated
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 31, 2026 19:31

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
CopilotAI review requested due to automatic review settings August 2, 2026 14:09

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Comment threadsrc/coreclr/jit/lower.cpp
CopilotAI requested a review from jkotasAugust 2, 2026 14:11
@jkotas
jkotas marked this pull request as ready for review August 2, 2026 14:11
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 7 pipeline(s).
9 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Comment threadsrc/coreclr/vm/frames.h Outdated
CopilotAI review requested due to automatic review settings August 2, 2026 14:26

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

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

Suppressed comments (1)

src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameHelpers.cs:402

  • UsesInlinedCallFrameStackSizeSentinel() re-parses Constants.Globals.Architecture directly, duplicating RuntimeInfo_1.ReadArchitecture and bypassing the existing RuntimeInfo contract caching/normalization. FrameHelpers already depends on _target.Contracts.RuntimeInfo elsewhere (GetFirstArgRegisterName), so this can be simplified and made consistent by querying IRuntimeInfo instead of re-reading globals here.
 private bool UsesInlinedCallFrameStackSizeSentinel()
{
if (_target.PointerSize != sizeof(uint))
{
return false;
}
if (_target.TryReadGlobalString(Constants.Globals.Architecture, out string? arch)
&& Enum.TryParse(arch, ignoreCase: true, out RuntimeInfoArchitecture runtimeArchitecture))
{
return runtimeArchitecture == RuntimeInfoArchitecture.X86;
}
return false;

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@jkotas

Copy link
Copy Markdown
Member

@copilot Fix cdac test failures

Generated cDAC usage documentation is stale: StackWalk.md.\nTo update it, run from the repository root:\n pwsh .\src\native\managed\cdac\tools\CdacUsageGraph\generate-docs.ps1\nReview and commit the generated changes. If the generated dependencies are incorrect, fix the usage-graph analysis or add a narrowly scoped entry to docs\design\datacontracts\data-descriptor-overrides.json.

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

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.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

CopilotAI commented Aug 2, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Fix cdac test failures

Generated cDAC usage documentation is stale: StackWalk.md.\nTo update it, run from the repository root:\n pwsh .\src\native\managed\cdac\tools\CdacUsageGraph\genera...

Fixed in 7d87b30.

I regenerated the StackWalk cDAC docs and updated the source meaning for InlinedCallFrame.Datum, so the generated StackWalk.md is back in sync.

Note

This reply was generated by GitHub Copilot.

@janvorlijanvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

@jkotas

Copy link
Copy Markdown
Member

/ba-g build analysis stuck

Sign up for freeto 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.

Visual Studio Debugger randomly not showing native call stack due to .NET 10 CALLI stack walk omits native frames for odd target addresses

4 participants

@jkotas@janvorli