Don't allow reinterprets that would expose padding #27877
Merged
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 just the breaking part of #27213. Commit message:
In #25908 it was noted that reinterpreting structures with paddings
exposes undef LLVM values to user code. This is problematic, because
an LLVM undef value is quite dangerous (it can have a different value
at every use, e.g. for
a::Bool
undef, we can havea || !a == true
.There are proposal in LLVM to create values that are merely arbitrary
(but the same at every use), but that capability does not currently
exist in LLVM. As such, we should try hard to prevent
undef
showingup in a user-visible way. There are several ways to fix this:
undef
and have the value merelybe arbitrary, but not dangerous.
bytes from that array.
However, for now, I think don't think we should make a choice here. Issues
like #21912, may play into the consideration, and I think we should be
able to reserve making a choice until that point. So what this PR does
is only allow reinterprets when they would not expose padding. This should
hopefully cover the most common use cases of reinterpret:
element type. These should generally always have compatiable padding (if
not, reinterpret was likely the wrong API to use).
This PR allows this for reading (but not for writing). Both cases are generally
better served by the IO APIs, but hopefully this should still allow the
common cases.
Fixes #25908