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.

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

Option containing heap drop types in static mut acts strangley #46089

Closed
Ryan1729 opened this issue Nov 19, 2017 · 4 comments
Closed

Option containing heap drop types in static mut acts strangley #46089

Ryan1729 opened this issue Nov 19, 2017 · 4 comments

Comments

@Ryan1729
Copy link
Contributor

The following code use the recently stabilized, (but not in the current stable release yet,) feature drop_types_in_const, so it needs a recent nightly version to be compiled.

struct Foo {
    bar: String,
}

static mut FOO: Option<Foo> = None;

fn main() {
    unsafe {
        FOO = Some(std::mem::zeroed());
    }
    if let Some(_) = unsafe { FOO.as_mut() } {
        println!("Some(Foo)");
    } else {
        println!("None");
    }
}

This prints out "None", instead of the expected "Some(Foo)". However, if I change FOO = Some(std::mem::zeroed()); to FOO = Some(std::mem::uninitialized()); then it prints "Some(Foo)", (at least on my system, but that could clearly be just "luck".) The assignment also works as expected if I explicitly initialize FOO, that is, something like FOO = Some("Hello World!".to_string());, or if I use a non-drop type. If I use a different built-in drop type than String, like Vec<u32> or Box<u32> then it still prints "None". However, if I define my own stack-allocated drop type like so:

struct Baz{
    qux: u32
}

impl Drop for Baz {
    fn drop(&mut self) {}
}

and make bar an zeroed instance of that type then it prints "Some(Foo)".

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Nov 19, 2017

Creating a mem::zeroed'd String is Undefined Behavior, because String contains a pointer that must be non-null. So this program has UB and may thus exhibit arbitrary weirdness. The same is true for Box and Vec. This has nothing to do with Drop, it also applies to references. Some types have also have invalid values other than the all-zero bit pattern (see e.g. the nomicon for an incomplete list of examples), and producing an invalid value is UB in all those cases.

To explain the specific weirdness you're seeing: The non-nullness is used to store the Option<Foo> more efficiently than (bool, Foo) – a null pointer in the place where a String would have a non-null pointer is used to indicate. In contrast, your custom Baz type contains an u32, for which no layout optimizations apply (because every bit pattern of 32 bits is a representation of a Baz value).

@Mark-Simulacrum
Copy link
Member

I'm going to close as not a bug; let us know if you feel differently and we can reopen.

@Ryan1729
Copy link
Contributor Author

I was under the impression that only reading from a zeroed String was UB, so I thought using the Option state without reading was fine. But you're saying creating one whatsoever is UB? Is that documented anywhere? mem::zeroed only mentions destructors and suggests reading mem::uninitialized which gives limits on how uninitialized memory may safely be written to but suggests there are safe ways to do it. Option doesn't seem to mention this at all.

@hanna-kruppe
Copy link
Contributor

hanna-kruppe commented Nov 19, 2017

My apologies, you''ve stumbled upon a corner of the language that was (and still is to some degree) fuzzily-defined and contradictory. I'm pretty sure we're moving towards even just creating the invalid value being UB (and as part of that, deprecating mem::uninitialized in favor of a union). This is somewhat reflected in the Nomicon and also the reference, if we want to get all rules-lawyery. But more importantly, even if creation s not insta-UB, taking the uninitialized or zeroed value, putting it into a Some, and then inspecting the Option is reading it (for certain types), due to the null pointer optimization which is mentioned in Option's docs.

You make a good point about the docs of zeroed and uninitialized, they should be updated. I'll file an issue for that.

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

No branches or pull requests

3 participants