Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

"Conditional jump or move depends on uninitialised value(s)" on feature(generator) #47253

Closed
dwrensha opened this issue Jan 7, 2018 · 3 comments

Comments

@dwrensha
Copy link
Contributor

dwrensha commented Jan 7, 2018

Valgrind reports use of uninitialized memory in the following program, which was reduced from tokio-rs/tokio-timer#36 and alexcrichton/futures-await#47.

// main.rs
#![feature(conservative_impl_trait, generators, generator_trait)]

pub type Poll<T, E> = Result<Async<T>, E>;

#[derive(Copy, Clone, Debug, PartialEq)]
pub enum Async<T> {
    Ready(T),
    NotReady,
}

pub trait Future {
    type Item;
    type Error;
    fn poll(&mut self) -> Poll<Self::Item, Self::Error>;
}

impl<F, T, E> Future for Option<F> where F: Future<Item=T, Error=E> {
    type Item = Option<T>;
    type Error = E;
    fn poll(&mut self) -> Poll<Option<T>, E> {
        match *self {
            None => Ok(Async::Ready(None)),
            Some(ref mut x) => match x.poll() {
                Ok(Async::Ready(t)) => Ok(Async::Ready(Some(t))),
                Ok(Async::NotReady) => Ok(Async::NotReady),
                Err(e) => Err(e),
            }
        }
    }
}

pub mod __rt {
    pub use std::ops::Generator;
    use super::Poll;
    use super::{Future, Async};
    use std::ops::GeneratorState;

    pub trait MyFuture<T: IsResult>: Future<Item=T::Ok, Error = T::Err> {}

    impl<F, T> MyFuture<T> for F
        where F: Future<Item = T::Ok, Error = T::Err > + ?Sized,
              T: IsResult
    {}

    pub trait IsResult {
        type Ok;
        type Err;

        fn into_result(self) -> Result<Self::Ok, Self::Err>;
    }
    impl<T, E> IsResult for Result<T, E> {
        type Ok = T;
        type Err = E;

        fn into_result(self) -> Result<Self::Ok, Self::Err> { self }
    }

    pub struct GenFuture<T>(pub T);

    pub enum Mu {}

    impl<T> Future for GenFuture<T>
        where T: Generator<Yield = Async<Mu>>,
              T::Return: IsResult,
    {
        type Item = <T::Return as IsResult>::Ok;
        type Error = <T::Return as IsResult>::Err;

        fn poll(&mut self) -> Poll<Self::Item, Self::Error> {
            match self.0.resume() {
                GeneratorState::Yielded(Async::NotReady)
                    => Ok(Async::NotReady),
                GeneratorState::Yielded(Async::Ready(mu))
                    => match mu {},
                GeneratorState::Complete(e)
                    => e.into_result().map(Async::Ready),
            }
        }
    }
}

pub struct Foo {
    _a: Option<usize>,
    _b: Option<String>,
}

fn ola() -> impl __rt::MyFuture<Result<(), String>> + 'static {
    __rt::GenFuture(move || -> Result<(), String> {
        let _f = Foo { _a: None, _b: None };
        yield Async::NotReady;
        return Ok(());
    })
}

fn main() {
    let ft = ola();
    let _ = Some(ft).poll();
}
$ rustc -v -V
rustc 1.25.0-nightly (6828cf901 2018-01-06)
binary: rustc
commit-hash: 6828cf90146c7fefc4ba4f16dffe75f763f2d910
commit-date: 2018-01-06
host: x86_64-unknown-linux-gnu
release: 1.25.0-nightly
LLVM version: 4.0
$ rustc main.rs
$ valgrind ./main
==2217== Memcheck, a memory error detector
==2217== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2217== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==2217== Command: ./main
==2217== 
==2217== Conditional jump or move depends on uninitialised value(s)
==2217==    at 0x10FAF4: <core::option::Option<F> as main::Future>::poll (in /home/dwrensha/Desktop/main)
==2217==    by 0x10F59A: main::main (in /home/dwrensha/Desktop/main)
==2217==    by 0x10F7E2: std::rt::lang_start::{{closure}} (in /home/dwrensha/Desktop/main)
==2217==    by 0x119E17: {{closure}} (rt.rs:59)
==2217==    by 0x119E17: _ZN3std9panicking3try7do_call17hffb5315d0c0f678dE.llvm.3A805FA6 (panicking.rs:480)
==2217==    by 0x123D9E: __rust_maybe_catch_panic (lib.rs:102)
==2217==    by 0x11034F: try<i32,closure> (panicking.rs:459)
==2217==    by 0x11034F: catch_unwind<closure,i32> (panic.rs:365)
==2217==    by 0x11034F: std::rt::lang_start_internal (rt.rs:58)
==2217==    by 0x10F7C6: std::rt::lang_start (in /home/dwrensha/Desktop/main)
==2217==    by 0x10F62D: main (in /home/dwrensha/Desktop/main)
==2217== 
==2217== Conditional jump or move depends on uninitialised value(s)
==2217==    at 0x10FB06: <core::option::Option<F> as main::Future>::poll (in /home/dwrensha/Desktop/main)
==2217==    by 0x10F59A: main::main (in /home/dwrensha/Desktop/main)
==2217==    by 0x10F7E2: std::rt::lang_start::{{closure}} (in /home/dwrensha/Desktop/main)
==2217==    by 0x119E17: {{closure}} (rt.rs:59)
==2217==    by 0x119E17: _ZN3std9panicking3try7do_call17hffb5315d0c0f678dE.llvm.3A805FA6 (panicking.rs:480)
==2217==    by 0x123D9E: __rust_maybe_catch_panic (lib.rs:102)
==2217==    by 0x11034F: try<i32,closure> (panicking.rs:459)
==2217==    by 0x11034F: catch_unwind<closure,i32> (panic.rs:365)
==2217==    by 0x11034F: std::rt::lang_start_internal (rt.rs:58)
==2217==    by 0x10F7C6: std::rt::lang_start (in /home/dwrensha/Desktop/main)
==2217==    by 0x10F62D: main (in /home/dwrensha/Desktop/main)
==2217== 
==2217== 
==2217== HEAP SUMMARY:
==2217==     in use at exit: 0 bytes in 0 blocks
==2217==   total heap usage: 6 allocs, 6 frees, 2,000 bytes allocated
==2217== 
==2217== All heap blocks were freed -- no leaks are possible
==2217== 
==2217== For counts of detected and suppressed errors, rerun with: -v
==2217== Use --track-origins=yes to see where uninitialised values come from
==2217== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
@Zoxc
Copy link
Contributor

Zoxc commented Jan 7, 2018

Here is a reduced test case:

#![feature(generators)]

pub struct Foo {
    _a: bool,
    _b: Option<String>,
}

fn main() {
    let g = ||  {
        let _f = Foo { _a: true, _b: None };
        yield ();
    };
    match Some(g) {
        None => (),
        Some(_) => (),
    }
}

It turns out that trans never stores the discriminant for the Some constructor here, and it remains uninitialized.

@Zoxc
Copy link
Contributor

Zoxc commented Jan 7, 2018

Relevant LLVM IR:

%"alloc::vec::Vec<u8>" = type { [0 x i64], { i8*, i64 }, [0 x i64], i64, [0 x i64] }
%"core::option::Option<alloc::allocator::Layout>" = type { [3 x i64] }
%"core::option::Option<alloc::allocator::Layout>::None" = type {}
%"core::option::Option<alloc::allocator::Layout>::Some" = type { [1 x i64], { i64, i64 }, [0 x i64] }
%"alloc::heap::Heap" = type {}
%"main::{{closure}}" = type { [0 x i64], %Foo, [0 x i32], i32, [1 x i32] }
%Foo = type { [0 x i64], %"core::option::Option<alloc::string::String>", [0 x i8], i8, [7 x i8] }
%"core::option::Option<alloc::string::String>" = type { [0 x i64], {}*, [2 x i64] }
%"core::option::Option<main::{{closure}}>" = type { [24 x i8], i8, [15 x i8] }
%"core::option::Option<main::{{closure}}>::Some" = type { [0 x i64], %"main::{{closure}}", [0 x i64] }
%"alloc::string::String" = type { [0 x i64], %"alloc::vec::Vec<u8>", [0 x i64] }
%"core::option::Option<alloc::string::String>::Some" = type { [0 x i64], %"alloc::string::String", [0 x i64] }

define internal void @_ZN6uninit4main17h832ef423163e0fb1E() unnamed_addr #1 {
start:
  %_3 = alloca %"main::{{closure}}", align 8
  %_2 = alloca %"core::option::Option<main::{{closure}}>", align 8
  %g = alloca %"main::{{closure}}", align 8
  %_0 = alloca {}, align 1
  %0 = getelementptr inbounds %"main::{{closure}}", %"main::{{closure}}"* %g, i32 0, i32 3
  store i32 0, i32* %0
  %1 = bitcast %"main::{{closure}}"* %g to i8*
  %2 = bitcast %"main::{{closure}}"* %_3 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %2, i8* %1, i64 40, i32 8, i1 false)
  %3 = bitcast %"core::option::Option<main::{{closure}}>"* %_2 to %"core::option::Option<main::{{closure}}>::Some"*
  %4 = bitcast %"core::option::Option<main::{{closure}}>::Some"* %3 to %"main::{{closure}}"*
  %5 = bitcast %"main::{{closure}}"* %_3 to i8*
  %6 = bitcast %"main::{{closure}}"* %4 to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %6, i8* %5, i64 40, i32 8, i1 false)
  %7 = getelementptr inbounds %"core::option::Option<main::{{closure}}>", %"core::option::Option<main::{{closure}}>"* %_2, i32 0, i32 1
  %8 = load i8, i8* %7, !range !3
  %9 = icmp eq i8 %8, 2
  %10 = select i1 %9, i64 0, i64 1
  switch i64 %10, label %bb3 [
    i64 0, label %bb1
    i64 1, label %bb2
  ]

bb1:                                              ; preds = %start
  br label %bb4

bb2:                                              ; preds = %start
  br label %bb4

bb3:                                              ; preds = %start
  unreachable

bb4:                                              ; preds = %bb1, %bb2
; call core::ptr::drop_in_place
  call void @_ZN4core3ptr13drop_in_place17h2c816151c807a52eE(%"core::option::Option<main::{{closure}}>"* %_2)
  br label %bb5

bb5:                                              ; preds = %bb4
  ret void
}

cc @eddyb

@eddyb
Copy link
Member

eddyb commented Jan 7, 2018

It turns out that trans never stores the discriminant for the Some constructor here, and it remains uninitialized.

That's because Option<T> is optimized in that case to the same size as T (where T is the generator state, I assume), which makes Some the identity function (relying on a valid T).

I guess generator fields don't have struct semantics but rather union/maybe-uninitialized semantics?

kennytm added a commit to kennytm/rust that referenced this issue Jan 8, 2018
Don't look for niches inside generator types. Fixes rust-lang#47253

r? @eddyb
@bors bors closed this as completed in 1a7b00d Jan 9, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants