1918186 - Panic while deserialising markers containing Rust structs with a boolean field

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Let's say we implement something like the following:

#[derive(serde::Serialize, serde::Deserialize, Debug)]
enum BooleanMetricAction {
    Create,
    Set(bool),
}

impl fmt::Display for BooleanMetricAction {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            BooleanMetricAction::Create => write!(f, "create"),
            BooleanMetricAction::Set(v) => write!(f, "set {}", v),
        }
    }
}

#[derive(serde::Serialize, serde::Deserialize, Debug)]
struct BooleanMetricMarker {
    id: MetricId,
    action: BooleanMetricAction,
}

impl gecko_profiler::ProfilerMarker for BooleanMetricMarker {
    fn marker_type_name() -> &'static str {
        "Glean Boolean Metric"
    }
    fn marker_type_display() -> gecko_profiler::MarkerSchema {
        use gecko_profiler::schema::*;
        let mut schema = MarkerSchema::new(&[Location::MarkerChart, Location::MarkerTable]);
        schema.set_chart_label("Name: {marker.name}");
        schema.add_key_label_format_searchable("id", "Id", Format::Integer, Searchable::Searchable);
        schema.add_key_label_format("action", "Action", Format::String);
        schema
    }
    fn stream_json_marker_data(&self, json_writer: &mut gecko_profiler::JSONWriter) {
        let action = self.action.to_string();
        json_writer.int_property("id", self.id.0.into());
        json_writer.string_property("action", action.as_str());
    }
}

When we run Firefox, and deserialise this, we get a panic while unwrapping the deserialised bincode in src/marker/mod.rs:

https://searchfox.org/mozilla-central/rev/3b59c739df66574d94022a684596845cd05e7c65/tools/profiler/rust-api/src/marker/mod.rs#303

let payload: T = bincode::deserialize(&payload_slice).unwrap();

Comment 1

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Okay, so I think I've figured this out: This occurs because we incorrectly "calculate" the deserialiser tags in Rust. We do this:

https://searchfox.org/mozilla-central/rev/4a8bd8efdfaa43dd14a16d3cb15bf86796fd1def/tools/profiler/rust-api/src/marker/deserializer_tags_state.rs#64

let unique_marker_tag = &T::marker_type_name as *const _ as usize;

which claims to take the address of the marker_type_name method of the struct that we're trying to serialise. In reality, however, it does something different (I'm unsure of the specific semantics), and always returns a constant 1.

Because of this, when we come to deserialise, we always choose the deserialiser function of the first marker that we recorded. In my case, that involved trying to deserialise two integers (rather than an integer and a bool), which then panicked.

To fix this, we need to make it so that the unique_marker_tag is, well, unique. This seems to work instead:

let unique_marker_tag = T::marker_type_name as *const () as usize;

Comment 2

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Attachment: Bug 1918186 - Correctly take the address of trait methods on structs when creating a unique_marker_tag r?mstange,canaltinova

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24b40e133a8a
Correctly take the address of trait methods on structs when creating a unique_marker_tag r=mstange,profiler-reviewers

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/24b40e133a8a