Segfault due to using return pointer directly

[alexcrichton]

First reported at rust-lang-deprecated/rustc-serialize#154, it looks like the code in that issue will segfault on all platforms and isn't related to the allocator in question, it's just a bad free. It looks like the expansion of #[derive(RustcDecodable)] is the function that's segfaulting, specifically:

extern crate rustc_serialize;

use rustc_serialize::*;

pub struct Request {
    pub id: String,
    pub arg: String,
}

impl Decodable for Request {
    fn decode<D: Decoder>(d: &mut D) -> Result<Request, D::Error> {
        d.read_struct("Request", 2, |d| {
            Ok(Request {
                id: match d.read_struct_field("id", 0, Decodable::decode) {
                    Ok(v) => v,
                    Err(e) => return Err(e),
                },
                arg: match d.read_struct_field("arg", 1, Decodable::decode) {
                    Ok(v) => v,
                    Err(e) => return Err(e),
                },
            })
        })
    }
}

In this function the closure should detect that it has nested returns and therefore the closure should write intermediate results into a local alloca of what to return. Instead, though, no alloca is created an results are stored directly into the return pointer.

The fault looks like it happens when:

• The decode function is called, read_struct is called, then the closure is called.
• The first call to read_struct_field succeeds, so the string is stored in the id field directly into the return pointer
• The second call to read_struct_field fails, returning an error
• The compiler writes the result (Err(D::Error)) into the return pointer
• The compiler then attempts to free the string stored to id

Basically at that point it's freeing arbitrary data because the string has already been paved over. I have been unable to reproduce this with a smaller example because everything I do seems to trigger usage of an alloca for the return value (which should be happening here). Maybe someone else knows though how to trigger this with a smaller example.

In any case though this segafult happens on nightly/beta/stable, but would be very good to fix!

cc @rust-lang/compiler

also nominating

[alexcrichton]

Oh also worth pointing out that this is only a problem for old trans, MIR trans does not exhibit this segfault.

[arielb1]

trans has way too many of these bugs, but we are moving to MIR-by-default soon anyway.

[luqmana]

Here is a smaller test case. Seems like it has to be a nested return in a closure in a generic cross crate fn.

pub struct Request {
    pub id: String,
    pub arg: String,
}

pub fn decode<T>() -> Result<Request, ()> {
    (|| {
        Ok(Request {
            id: "hi".to_owned(),
            arg: match Err(()) {
                Ok(v) => v,
                Err(e) => return Err(e)
            },
        })
    })()
}

extern crate test;

fn main() {
    assert!(test::decode::<()>().is_err());
}

I think I have a fix for it and I'll try to put it up soon.

[posborne]

@alexcrichton I'm running into this on 1.10 and it also fails on rustc 1.11.0-beta.1 (8dc253bcf 2016-07-05) (but not nightly). For me, my code crashes if I (or a user) provides invalid JSON that correct code tries to parse using . I would very much like to see a fix in 1.11 if possible. Any chance of that?

This bug would seems to breaks the canonical way of doing JSON parsing for the majority of users.

[alexcrichton]

@posborne sure! I've nominated the PR for a beta backport at #34658 (comment)