Badly behaved Encode and Decode impls for FileMap

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.

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

@dtolnay

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.

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.

I agree.

Navigation Menu

Repository navigation

Badly behaved Encode and Decode impls for FileMap #42374

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Badly behaved Encode and Decode impls for FileMap #42374

Description

Activity

dtolnay commented on Jun 2, 2017

pnkfelix commented on Apr 19, 2019

bjorn3 commented on May 15, 2022

saethlin commented on Sep 12, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions