-
Notifications
You must be signed in to change notification settings - Fork 5k
Reapply "Create a single copy of stub templates #115749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Tagging subscribers to this area: @mangod9 |
There was a problem hiding this comment.
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 reverts previous changes that affected the creation of a single copy of stub templates and disables the feature on Apple platforms pending resolution of dotnet/diagnostics#5487.
- Reintroduces changes to use standard integer types (uint8_t and size_t) in function signatures.
- Integrates a new configuration structure (InterleavedLoaderHeapConfig) for loader heap initialization and code page generation.
- Disables the template mapping feature on Apple platforms and adds related conditional logic.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/coreclr/vm/precode.h / precode.cpp | Updated function signatures to use uint8_t* and size_t for improved clarity and consistency. |
src/coreclr/vm/loaderallocator.cpp | Replacing function pointer parameters with references to InterleavedLoaderHeapConfig for stub code page generation. |
src/coreclr/utilcode/interleavedloaderheap.cpp | Updated allocation paths to support template-based thunk allocation and proper configuration initialization. |
src/coreclr/inc/loaderheap.h | Added and updated the configuration structure and its usage in loader heap initialization. |
Other files | Adjustments in assembly and executable allocator files to integrate template-based thunk allocation. |
Comments suppressed due to low confidence (3)
src/coreclr/utilcode/interleavedloaderheap.cpp:542
- The integration of the new InterleavedLoaderHeapConfig and its initialization via CreateTemplate appears correct. Please ensure that the template-based allocation paths are covered by adequate tests to validate the behavior under both standard and template-enabled environments.
void InitializeLoaderHeapConfig(InterleavedLoaderHeapConfig *pConfig, size_t stubSize, void* templateInImage, void (*codePageGenerator)(uint8_t* pageBase, uint8_t* pageBaseRX, size_t size))
src/coreclr/vm/precode.h:228
- Changing function signatures from BYTE* and SIZE_T to uint8_t* and size_t improves type clarity and portability. Please ensure that all related pointer arithmetic and casting remain consistent across the codebase.
static void GenerateCodePage(uint8_t* pageBase, uint8_t* pageBaseRX, size_t size);
src/coreclr/vm/loaderallocator.cpp:1211
- Replacing the original code page generator function pointer with a reference to the global configuration ensures a centralized management approach. Verify that all caller sites of the loader heap allocation have been updated to work with the new configuration structure.
&s_stubPrecodeHeapConfig
I think we need to merge and release dbgshim, then bump it everywhere and then merge here. What do you think @tommcdon @davidwrighton ? |
Ah okay, we should enable on Mac after doing what I asked :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a minor comment change for your review
Co-authored-by: Tom McDonald <tommcdon@microsoft.com>
/ba-g The build break is unrelated and has already been fixed by #115767 |
Revert PR #115665 which reverted #114462