Skip to content

[IRGen] Pack extra data pattern structs. #34822

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

Merged
merged 3 commits into from
Nov 19, 2020

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Nov 19, 2020

Previously, the extra data pattern structs for struct and enums were not packed.

On 32 bit, this resulted in an extra data pattern struct which was 4 bytes too large whenever there was an odd number of fields in the struct. The result was writing past the end of the allocated struct. That bug only caused occasional crashes because (1) for the most part there was additional space beyond the end of the allocation intended for the struct metadata in the bump allocator and (2) while half of the trailing flags field would be overwritten, those bits of the trailing flags being nonzero did not have an observable effect since those bits of the trailing flags field are not yet used.

Here, the structs are marked packed, resulting in the appropriate size for the extra data pattern structs on 32 bit platforms.

rdar://problem/68997282

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test asan

@nate-chandler
Copy link
Contributor Author

@swift-ci please clean test windows platform

@nate-chandler
Copy link
Contributor Author

@swift-ci please asan test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ea93c3d1b97edaf28a73600c9e7211b08e3fdb02

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please asan test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 264dcf297a976f21c5616431633e25b34c39711e

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please asan test

1 similar comment
@nate-chandler
Copy link
Contributor Author

@swift-ci please asan test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a39d91672819a405f9189090c11bdcdfedd723c4

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - a39d91672819a405f9189090c11bdcdfedd723c4

Previously, the extra data pattern structs for struct and enums were not
packed.

On 32 bit, this resulted in an extra data pattern struct which was 4
bytes too large whenever there was an odd number of fields in the
struct.  The result was writing past the end of the allocated struct.
That bug only caused occasional crashes because (1) for the most part
there was additional space beyond the end of the allocation intended for
the struct metadata in the bump allocator and (2) while half of the
trailing flags field would be overwritten, because those bits of the
trailing flags being nonzero did not have an observable effect since
those bits of the trailing flags field are not yet used.

Here, the structs are marked packed, resulting in the appropriate size
for the extra data pattern structs on 32 bit platforms.

rdar://problem/68997282
To prevent the fix for rdar://problem/68997282 from regressing, assert
that the size of the extra data patterns for generic enums and structs
combined with the corresponding offsets matches the size passed from the
generic metadata instantiation function to
swift_allocateGenericValueMetadata.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please asan test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 3b13850d88e700f1e33dde1c6a6a4e954e72ca98

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 3b13850d88e700f1e33dde1c6a6a4e954e72ca98

To prevent rdar://problem/68997282 from regressing, verify at runtime in
debug builds that in calls to swift_allocateGenericValueMetadata the
extraDataSize argument matches the OffsetInWords and SizeInWords
specified by the GenericMetadataPartialPattern available within the
pattern argument.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler nate-chandler deleted the rdar68997282 branch November 19, 2020 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants