[v632] Add support for operator new signature introduced in C++17 in libNew (need for TMapFile) #15343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note both the new (catastrophically) or the old operator new are return an address that are misaligned. The old version was (and still) is returning a memory address aligned for sizeof(std::max_align_t) when it is supposed to returned a memory address aligned with __STDCPP_DEFAULT_NEW_ALIGNMENT__ (those value are usually respectively 8 and 16). The lame implementation of operator new for std::align_val_t is currently ignoring the passed argument (see next commits).
Meta data is extended to include the requested alignment (in case we need to call re-alloc) and the offset from the actual start of the allocated memory since this is no longer calculatable from just the address of the data and the alignment. Remove specialization from not R__B64, assume size_t is always the right type
…sss. Introduce new callback ROOT::Internal::gGetMapFileMallocDesc
The indicates the displacement between the requested location and the actual memory location (Essential once we remove the dreaded MAP_FIXED
MAP_FIXED is evil. The mmap documentation states: ... the mapping established by mmap() replacesany previous mappings for the process' pages in the range from addr to addr + len. and ** literaly ** it will silenty free-up and reuse any allocation done by malloc/new within the requested range. In bygone days, this worked out okay by simply specifying a "high enough" memory location to avoid overlap. However we the advent of memory randomization finding an area of memory that is "a priori" guaranteed to not be used by something else is essentially impossible. In previous implementation of TMapFile (a long long time ago) having a matching memory location for the memory map in the client and server was necessary as data pointers (it was even tried to re-used virtual function table!) were used. This is no longer the case and thus relaxing the actual location will not affect the functionality.
This insures that only the intended part of being allocated within the memory mapped file
This is no longer needed
The previous code in R__ListSlowClose and thus TROOT::CloseFiles was incorrectly using TDirectory::Close on the unrelated class TMapFile :(
In Optimized build it appears that libNew is not used (when it should) during the execution of dictionary code. Given than Mac 12/Monterey will be (likely) end of life in 6 months, there is no benefit in investigating this issue
This was referenced Apr 25, 2024
Closed
Test Results 11 files 11 suites 2d 2h 20m 11s ⏱️ Results for commit b3fae7a. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #14792