Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Don't allow reinterprets that would expose padding #27877

Merged
merged 1 commit into from
Jul 7, 2018
Merged

Don't allow reinterprets that would expose padding #27877

merged 1 commit into from
Jul 7, 2018

Commits on Jul 6, 2018

  1. Don't allow reinterprets that would expose padding 

    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 have `a || !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` showing
up in a user-visible way. There are several ways to fix this:
 1. Wait until LLVM comes up with a safer `undef` and have the value merely
    be arbitrary, but not dangerous.
 2. Always guarantee that padding bytes will be 0.
 3. For contiguous-memory arrays, guarantee that we end up with the underlying
    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:
 - Reinterpreting a vector or matrix of values to StaticVectors of the same
   element type. These should generally always have compatiable padding (if
   not, reinterpret was likely the wrong API to use).
 - Reinterpreting from a Vector{UInt8} to a vector of structs (that may have padding).
   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
    @Keno
    Keno committed Jul 6, 2018
    Configuration menu
    Copy the full SHA
    85dced1 View commit details
    Browse the repository at this point in the history