[5.3] [metadata prespecialization] Zero trailing flags field. #31420
Conversation
This file contains hidden or 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
The struct and enum metadata scanners were both adding a pointer for the trailing flags field. That was only correct for 64-bit platforms. Instead, add an Int64.
Added an assertion that a generic metadata record that is instantiated at runtime is not a prespecialized metadata record.
Previously, the trailing flags field of runtime instantiated generic metadata for types which had prespecialization enabled were not zeroed. Consequently, the field always contained garbage. Often, metadata was instantiated on new (and so, zeroed) pages, so the garbage happened to be zero as is appropriate. However, when the metadata was instantiated on pages which had previously been dirtied, the garbage value would sometimes indicate that the metadata was canonical statically specialized. When that occurred, swift_checkMetadataState would incorrectly return a metadata state of complete, despite the fact that the metadata might not in fact be complete. As a result, the runtime was trafficking in incomplete metadata as if it were complete, resulting in various crashes that would arise from for example missing a witness table or a value witness table. Here the problem is corrected. The trailing flags field of structs and enums that have the field is set to 0. For structs, this is accomplished by modifying the extra data pattern to exist not only when there is fixed type info for the type but also when the type is being prespecialized. Specifically, the extra data for a struct is, rather than an i32 array of field offsets, a struct consisting of none, one, or both of the following: (1) the array of field offsets, (2) the trailing flags. Similarly, enums now have an extra data pattern which consists of none, one, or both of the following: (1) the payload size, (2) the trailing flags. Enum metadata extra data setting was previously achieved by customizing the metadata allocation function; that customization is now eliminated, being replaced with the shared runtime code for copying extra data into place, a modest code size savings. rdar://problem/61465515
To facilitate debugging metadata records which are not properly initialized, upon allocation fill them with a regular byte pattern (0xAA) so that on subsequent inspection it is obvious if part of the record is not initialized.
|
@swift-ci please test |
|
@swift-ci please test windows platform |
nate-chandler
deleted the
generic-metadata-prespecialization-components/rdar61465515
branch
AnthonyLatsis
added
swift 5.3
🍒 release cherry pick
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.
The important changes here are
and
Together, those two commits unlock reenabling metadata prespecialization for the stdlib.
Additionally, improved the debuggability of bad metadata records by first memsetting them to 0xAA on allocation in debug builds and secondly asserting that newly created records are not prespecialized. Mike Ash pointed out that the memsetting could be done in release builds as well by so long as the work if the MallocScribble environment variable is set; that change will be done in a follow-on commit.