Improve the UB situation in layout tests. #2064
Closed
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
|
|
russell-islam
added a commit
to russell-islam/cloud-hypervisor
that referenced
this pull request
Jun 22, 2021
This patch addresses this issue rust-lang/rust-bindgen#2064. While we access field of packed struct the compiler can generate the correct code to create a temporary variable to access the packed struct field. Access withing {} ensures that. Signed-off-by: Muminul Islam <[email protected]>
russell-islam
added a commit
to russell-islam/cloud-hypervisor
that referenced
this pull request
Jun 22, 2021
This patch addresses this issue rust-lang/rust-bindgen#2064. While we access field of packed struct the compiler can generate the correct code to create a temporary variable to access the packed struct field. Access withing {} ensures that. Signed-off-by: Muminul Islam <[email protected]>
likebreath
pushed a commit
to cloud-hypervisor/cloud-hypervisor
that referenced
this pull request
Jun 22, 2021
This patch addresses this issue rust-lang/rust-bindgen#2064. While we access field of packed struct the compiler can generate the correct code to create a temporary variable to access the packed struct field. Access withing {} ensures that. Signed-off-by: Muminul Islam <[email protected]>
|
Hi @emilio, is there anything blocking you from merging this? It would be super helpful for all the projects using bindgen :) |
|
Yeah, some tests are failing, I don't recall the specifics, need to look into that, will re-push. |
- Replaced dereferencing of null ptr with zero bit pattern + transmute + std::ptr::addr_of! to avoid UB. It affects only checks of fields offsets - Overwritten expected with BINDGEN_OVERWRITE_EXPECTED=1 - Overwritten test_multiple_header_calls_in_builder and test_mixed_header_and_header_contents manually because #2054 - Do not check the layout for repr(C) unions - Do not call the destructor of tmp struct to avoid other UB
emilio
force-pushed
the
layouttests-ub
branch
from
87097af to
284dd3c
Compare
|
Out of curiosity, why zero memory and not a (Using MaybeUninit like that is explicitly defined here: https://doc.rust-lang.org/beta/std/mem/union.MaybeUninit.html#initializing-a-struct-field-by-field) |
|
☔ The latest upstream changes (presumably 310f7f8) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
This is #2055 with some extra fixes