Skip to content

Avoid writing jitdump code for block data - #122274

Merged
tommcdon merged 7 commits into
dotnet:mainfrom
tommcdon:dev/tommcdon/jitmap_blockdata_fix
Mar 20, 2026
Merged

tommcdon merged 7 commits into
dotnet:mainfrom
tommcdon:dev/tommcdon/jitmap_blockdata_fix

Conversation

@tommcdon

@tommcdon tommcdon commented Dec 8, 2025

Copy link
Copy Markdown
Member

Fixes #120419. The implementation of JIT dumps in the linux perf tool creates dynamic shared objects representing JIT'd methods. In .NET 10, #113943 changed the default granularity for stubs, which now reports the block of memory that stores stubs instead of the individual methods. If the event is fired before the block memory is committed (only is reserved), PerfJitDumpState::LogMethod fails to read the memory from the block and then all subsequent JIT dump events will fail. The linux perf tool will favor jit dumps over perfmaps if jit dumps are available, which breaks DOTNET_PerfMapEnabled=1 or 2 settings. This fix addresses the issue by no longer reporting the code byte for block allocations, which is only enabled when DOTNET_PerfMapStubGranularity is set to 0 or 1.

Copilot AI 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 an issue (#120419) where JIT dumps fail when reporting stub blocks whose memory is reserved but not yet committed. The fix prevents writing code bytes for block-level allocations while maintaining backward compatibility for individual method reporting.

  • Adds a reportCodeBlock boolean parameter to PAL_PerfJitDump_LogMethod (defaults to true)
  • Updates the stub logging logic to skip code bytes for block-type allocations
  • Removes dead code check that would never execute

Reviewed changes

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

File Description
src/coreclr/pal/inc/pal.h Adds reportCodeBlock parameter with default value true to maintain backward compatibility
src/coreclr/pal/src/misc/perfjitdump.cpp Implements conditional code size reporting based on reportCodeBlock parameter; removes dead code check
src/coreclr/vm/perfmap.cpp Updates call site to pass false for block-type stub allocations

@tommcdon tommcdon added area-Diagnostics-coreclr and removed area-PAL-coreclr only for closed issues labels Dec 8, 2025
@tommcdon tommcdon added this to the 11.0.0 milestone Dec 8, 2025
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Comment thread src/coreclr/pal/src/misc/perfjitdump.cpp
Comment thread src/coreclr/vm/perfmap.cpp Outdated
Copilot AI review requested due to automatic review settings February 11, 2026 17:55

Copilot AI 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 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/pal/inc/pal.h Outdated
Copilot AI review requested due to automatic review settings February 11, 2026 21:52

Copilot AI 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 3 out of 3 changed files in this pull request and generated no new comments.

Comment thread src/coreclr/vm/perfmap.cpp

@noahfalk noahfalk 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.

👍

Copilot AI review requested due to automatic review settings March 19, 2026 16:21
@tommcdon
tommcdon enabled auto-merge (squash) March 19, 2026 16:21

Copilot AI 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 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/pal/src/misc/perfjitdump.cpp:268

  • reportedCodeSize controls how many bytes are appended to the record, but record.code_size remains set to codeSize even when reportCodeBlock is false. Since header.total_size will then not equal sizeof(record)+name+code_size, this looks inconsistent and can be confusing for future maintainers and for jitdump consumers. Consider adding an explicit comment here clarifying that code_size describes the code region size for symbolization while the native code payload is intentionally omitted (and total_size reflects the actual emitted payload size).
            size_t reportedCodeSize = reportCodeBlock ? codeSize : 0;

            size_t bytesRemaining = sizeof(JitCodeLoadRecord) + symbolLen + 1 + reportedCodeSize;

            record.header.timestamp = GetTimeStampNS();
            record.vma = (uint64_t) pCode;
            record.code_addr = (uint64_t) pCode;
            record.code_size = codeSize;
            record.header.total_size = bytesRemaining;

You can also share your feedback on Copilot code review. Take the survey.

@tommcdon

Copy link
Copy Markdown
Member Author

/ba-g failures are known

@steveisok

Copy link
Copy Markdown
Member

/backport to release/10.0

@github-actions github-actions Bot unlocked this conversation Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Started backporting to release/10.0 (link to workflow run)

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@steveisok backporting to release/10.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git cherry-pick fec510711cda38dc02759c5f03d01bf1249a6e60

Auto-merging src/coreclr/pal/inc/pal.h
Auto-merging src/coreclr/vm/perfmap.cpp
CONFLICT (content): Merge conflict in src/coreclr/vm/perfmap.cpp
error: could not apply fec510711cd... Avoid writing jitdump code for block data (#122274)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Avoid writing jitdump code for block data
Applying: Address PR feedback
Applying: Address PR feedback
Using index info to reconstruct a base tree...
M	src/coreclr/pal/inc/pal.h
M	src/coreclr/vm/perfmap.cpp
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/pal/inc/pal.h
Auto-merging src/coreclr/vm/perfmap.cpp
CONFLICT (content): Merge conflict in src/coreclr/vm/perfmap.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0003 Address PR feedback
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DOTNET_PerfMapEnabled behavior in .NET 10.0

6 participants