Badly behaved Encode and Decode impls for FileMap

[dtolnay]

The impls added in #22235 violate some assumptions in libserialize by trying to serialize multiple values for the "lines" field. When decoding, it ends up hitting this unwrap.

#![feature(rustc_private)]

extern crate syntax_pos;
extern crate serialize;

use std::cell::RefCell;
use syntax_pos::{FileMap, BytePos};
use serialize::json;

fn main() {
    let file_map = FileMap {
        name: "rustc".to_owned(),
        name_was_remapped: false,
        crate_of_origin: 0,
        src: None,
        start_pos: BytePos(0),
        end_pos: BytePos(0),
        lines: RefCell::new(vec![BytePos(1), BytePos(2), BytePos(3)]),
        multibyte_chars: RefCell::new(vec![]),
    };

    let j = json::as_pretty_json(&file_map).to_string();
    println!("{}", j);

    // panic in libserialize
    let _ = json::decode::<FileMap>(&j);
}

{
  "name": "rustc",
  "name_was_remapped": false,
  "start_pos": 0,
  "end_pos": 0,
  "lines": 31111,
  "multibyte_chars": []
}
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335

This may not be important if it has worked for this long, but there may be other variations of this bug lurking elsewhere.

[dtolnay]

Side note: this unreachable is reachable.

#![feature(rustc_private)]

extern crate syntax_pos;
extern crate serialize;

use syntax_pos::FileMap;
use serialize::{opaque, Decodable};

fn main() {
    let bytes = [0, 0, 0, 0, 2, 0, 0];
    let _ = FileMap::decode(&mut opaque::Decoder::new(&bytes, 0));
}

thread 'main' panicked at 'internal error: entered unreachable code', /checkout/src/libsyntax_pos/lib.rs:481

[pnkfelix]

The tests here need to be updated; e.g. PR #52953 renamed FileMap to SourceFile, so the second example now needs to be written like (play):

#![feature(rustc_private)]

extern crate syntax_pos;
extern crate serialize;

use syntax_pos::SourceFile as FileMap;
use serialize::{opaque, Decodable};

fn main() {
    let bytes = [0, 0, 0, 0, 2, 0, 0];
    let _ = FileMap::decode(&mut opaque::Decoder::new(&bytes, 0));
}

(or you can rename it in the code itself rather than use use ... as ...;)

When you run it today, it does this:

thread 'main' panicked at 'assertion failed: position <= slice.len()', src/libserialize/leb128.rs:79:1

which seems better than what @dtolnay was previously seeing.

[bjorn3]

Updated test case:

#![feature(rustc_private)]

extern crate rustc_span;
extern crate rustc_serialize;

use rustc_span::SourceFile as FileMap;
use rustc_serialize::{opaque, Decodable};

fn main() {
    let bytes = [0, 0, 0, 0, 2, 0, 0];
    let _ = FileMap::decode(&mut opaque::Decoder::new(&bytes, 0));
}

This now panics with

thread 'main' panicked at 'assertion failed: sentinel == STR_SENTINEL', /rustc/70b3681bf621bc0de91ffab711b2350068b4c466/compiler/rustc_serialize/src/opaque.rs:656:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/70b3681bf621bc0de91ffab711b2350068b4c466/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/70b3681bf621bc0de91ffab711b2350068b4c466/library/core/src/panicking.rs:142:14
   2: core::panicking::panic
             at /rustc/70b3681bf621bc0de91ffab711b2350068b4c466/library/core/src/panicking.rs:48:5
   3: <rustc_serialize::opaque::Decoder as rustc_serialize::serialize::Decoder>::read_str
   4: <alloc::string::String as rustc_serialize::serialize::Decodable<D>>::decode
   5: <std::path::PathBuf as rustc_serialize::serialize::Decodable<D>>::decode
   6: rustc_span::_DERIVE_rustc_serialize_Decodable_D_FOR_RealFileName::<impl rustc_serialize::serialize::Decodable<__D> for rustc_span::RealFileName>::decode
   7: rustc_span::_DERIVE_rustc_serialize_Decodable_D_FOR_FileName::<impl rustc_serialize::serialize::Decodable<__D> for rustc_span::FileName>::decode
   8: <rustc_span::SourceFile as rustc_serialize::serialize::Decodable<D>>::decode
   9: rust_out::main
  10: core::ops::function::FnOnce::call_once
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

as it detected an invalid string serialization.

JSON deserialization support of rustc_serialize has been removed somewhat recently. Furthermore I don't think it is necessary for us to be robust against all corrupt metadata as we have to abort some way or another anyway. I think this issue should be closed.

[saethlin]

I agree.