Skip to content

Commit

Permalink
Stacked borrows:
Browse files Browse the repository at this point in the history
Miri enforces that references passed to functions are valid during the entire execution of the function, even when those references are passed inside a struct.

See someguynamedjosh/ouroboros#88 for a similar issue and discussion on that topic.
  • Loading branch information
Ten0 committed Jan 15, 2024
1 parent addc42e commit ccfee76
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 23 deletions.
8 changes: 4 additions & 4 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub struct DeserializerState<'s, R> {
/// Schema + other configs for deserialization
#[derive(Clone)]
pub struct DeserializerConfig<'s> {
schema_root: &'s SchemaNode<'s>,
schema_root: NodeRef<'s>,
/// If a sequence turns out to be longer than this during deserialization,
/// we will throw an error instead.
///
Expand All @@ -115,7 +115,7 @@ impl<'s> DeserializerConfig<'s> {
pub fn new(schema: &'s Schema) -> Self {
Self::from_schema_node(schema.root())
}
pub(crate) fn from_schema_node(schema_root: &'s SchemaNode<'s>) -> Self {
pub(crate) fn from_schema_node(schema_root: NodeRef<'s>) -> Self {
Self {
schema_root,
max_seq_size: 1_000_000_000,
Expand All @@ -129,7 +129,7 @@ impl<'s, 'de, R: ReadSlice<'de>> DeserializerState<'s, R> {
Self::from_schema_node(r, schema.root())
}

pub(crate) fn from_schema_node(r: R, schema_root: &'s SchemaNode<'s>) -> Self {
pub(crate) fn from_schema_node(r: R, schema_root: NodeRef<'s>) -> Self {
Self::with_config(r, DeserializerConfig::from_schema_node(schema_root))
}

Expand All @@ -139,7 +139,7 @@ impl<'s, 'de, R: ReadSlice<'de>> DeserializerState<'s, R> {

pub fn deserializer<'r>(&'r mut self) -> DatumDeserializer<'r, 's, R> {
DatumDeserializer {
schema_node: self.config.schema_root,
schema_node: self.config.schema_root.as_ref(),
allowed_depth: AllowedDepth::new(self.config.allowed_depth),
state: self,
}
Expand Down
10 changes: 6 additions & 4 deletions src/object_container_file_encoding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,12 @@ struct Metadata<S, M> {
#[serde(flatten)]
user_metadata: M,
}
const METADATA_SCHEMA: &crate::schema::self_referential::SchemaNode =
&crate::schema::self_referential::SchemaNode::Map(
crate::schema::self_referential::NodeRef::from_static(
&crate::schema::self_referential::SchemaNode::Bytes,
const METADATA_SCHEMA: crate::schema::self_referential::NodeRef<'static> =
crate::schema::self_referential::NodeRef::from_static(
&crate::schema::self_referential::SchemaNode::Map(
crate::schema::self_referential::NodeRef::from_static(
&crate::schema::self_referential::SchemaNode::Bytes,
),
),
);

Expand Down
9 changes: 1 addition & 8 deletions src/object_container_file_encoding/reader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,18 +173,11 @@ where
.read_const_size_buf::<16>()
.map_err(FailedToInitializeReader::FailedToDeserializeHeader)?;

// Build a `&'static SchemaNode` from the `Arc<Schema>` (`'static` is fake)
// Safety: we don't drop the schema until this is dropped
// This is useful to be able to store a DeserializerState directly in here,
// which will avoid additional &mut levels, allowing for highest performance and
// ergonomics
let schema_root: &'static schema::self_referential::SchemaNode<'static> = unsafe {
let schema = &*(&*schema as *const Schema);
let a: *const schema::self_referential::SchemaNode<'_> =
schema.root() as *const schema::self_referential::SchemaNode<'_>;
let b: *const schema::self_referential::SchemaNode<'static> = a as *const _;
&*b
};
let schema_root = unsafe { schema.root_with_fake_static_lifetime() };

Ok((
Self {
Expand Down
4 changes: 3 additions & 1 deletion src/object_container_file_encoding/writer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ impl<'c, 's> WriterBuilder<'c, 's> {
codec: self.compression.codec(),
user_metadata: metadata,
})
.serialize(header_serializer_state.serializer_overriding_schema_root(METADATA_SCHEMA))
.serialize(
header_serializer_state.serializer_overriding_schema_root(METADATA_SCHEMA.as_ref()),
)
.map_err(|ser_error| {
<SerError as serde::ser::Error>::custom(format_args!(
"Failed to serialize object container file header metadata: {ser_error}"
Expand Down
24 changes: 19 additions & 5 deletions src/schema/self_referential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,26 @@ impl Schema {
/// [`Schema`].
///
/// The root node represents the whole schema.
pub(crate) fn root<'a>(&'a self) -> &'a SchemaNode<'a> {
pub(crate) fn root<'a>(&'a self) -> NodeRef<'a> {
// the signature of this function downgrades the fake 'static lifetime in a way
// that makes it correct
self.nodes
.first()
.expect("Schema must have at least one node (the root)")
assert!(
!self.nodes.is_empty(),
"Schema must have at least one node (the root)"
);
// SAFETY: bounds checked
unsafe { NodeRef::new(self.nodes.as_ptr() as *mut _) }
}

/// SAFETY: this `NodeRef` must never be dereferenced after the
/// corresponding schema was dropped.
pub(crate) unsafe fn root_with_fake_static_lifetime(&self) -> NodeRef<'static> {
assert!(
!self.nodes.is_empty(),
"Schema must have at least one node (the root)"
);
// SAFETY: bounds checked
unsafe { NodeRef::new(self.nodes.as_ptr() as *mut _) }
}

/// Obtain the JSON for this schema
Expand Down Expand Up @@ -492,7 +506,7 @@ impl TryFrom<super::safe::SchemaMut> for Schema {

impl std::fmt::Debug for Schema {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
<SchemaNode<'_> as std::fmt::Debug>::fmt(self.root(), f)
<SchemaNode<'_> as std::fmt::Debug>::fmt(self.root().as_ref(), f)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/ser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl<'c, 's, W: std::io::Write> SerializerState<'c, 's, W> {

pub fn serializer<'r>(&'r mut self) -> DatumSerializer<'r, 'c, 's, W> {
DatumSerializer {
schema_node: self.config.schema().root(),
schema_node: self.config.schema().root().as_ref(),
state: self,
}
}
Expand Down

0 comments on commit ccfee76

Please sign in to comment.