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

Unexpected undefined behavior when assigning a function pointer to std::mem::zeroed() #51615

Closed
archshift opened this issue Jun 18, 2018 · 3 comments · Fixed by #73425
Closed
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@archshift
Copy link
Contributor

archshift commented Jun 18, 2018

Note: the following behavior only exists when compiling with optimizations (a.k.a. release mode).

Example 1:

pub fn bar() -> usize {
    let foo: fn() = unsafe { ::std::mem::zeroed() };
    foo as usize
}

This emits (as of 1.26.0):

  push rbp
  mov rbp, rsp
  ud2

Whereas it used to have expected behavior up until 1.19.0:

  push rbp
  mov rbp, rsp
  xor eax, eax
  pop rbp
  ret

Example 2:

A bit of an odd case I discovered, which results in what seems to be some stack corruption:

pub struct Foo<T: Copy> {
    map_keys: [u64; 64],
    map_vals: T,
}

impl<T: Copy> Foo<T> {
    pub fn new() -> Self {
        Foo {
            map_keys: [!0; 64],
            map_vals: unsafe { ::std::mem::zeroed() }
        }
    }
}

fn main() {
    Foo::<fn()>::new();
}

Playground link


What I think is striking here is that this behavior occurs while never even dereferencing the pointer!. This allows one to shoot oneself in the foot when trying to, for example, set a default value for a function pointer member.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jun 18, 2018

"Function pointers" are actually "function references", so they can't be null (and optimizer can rely on that), so Option<fn()> needs to be used for nullable function pointers (as explained in the documentation).

@hanna-kruppe
Copy link
Contributor

Yeah, this might surprise people, but it's working as intended: creating a bit pattern that is invalid for a type and unsafely claiming it to be of that type is instant UB, and has to be to enable a variety of optimizations. One more reason to be extremely careful with mem::zeroed (e.g., the Foo type in example 2 is unsound for many Copy types T, not just fn types).

We might want to explicitly call this out in more places (e.g., in the reference and in the nomicon) since quite a few people trip over this, but it's not going to change.

@kennytm kennytm added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Jun 18, 2018
@steveklabnik steveklabnik added the P-medium Medium priority label Dec 27, 2018
@steveklabnik
Copy link
Member

Let's put this stuff in mem::zeroed's docs.

@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 6, 2020
RalfJung added a commit to RalfJung/rust that referenced this issue Jun 18, 2020
…ers, r=dtolnay

Mention functions pointers in the documentation

Fixes rust-lang#51615.

This mentions function pointers in the documentation for `core::mem::zeroed`, adding them to the list of types that are **always** wrong when zeroed, with `&T` and `&mut T`.

@rustbot modify labels: T-doc, C-enhancement, T-libs
@bors bors closed this as completed in 9f8f994 Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants