Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove panics from Artifact::deserialize #3130

Merged
merged 13 commits into from
Aug 25, 2022
4 changes: 2 additions & 2 deletions lib/cli/src/c_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl CType {
#[allow(clippy::borrowed_box)]
let ret: CType = return_value
.as_ref()
.map(|i: &Box<CType>| (&**i).clone())
.map(|i: &Box<CType>| (**i).clone())
.unwrap_or_default();
ret.generate_c(w);
w.push(' ');
Expand Down Expand Up @@ -183,7 +183,7 @@ impl CType {
#[allow(clippy::borrowed_box)]
let ret: CType = return_value
.as_ref()
.map(|i: &Box<CType>| (&**i).clone())
.map(|i: &Box<CType>| (**i).clone())
.unwrap_or_default();
ret.generate_c(w);
w.push(' ');
Expand Down
2 changes: 1 addition & 1 deletion lib/compiler/src/artifact_builders/artifact_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl ArtifactCreate for ArtifactBuild {
}

fn data_initializers(&self) -> &[OwnedDataInitializer] {
&*self.serializable.data_initializers
&self.serializable.data_initializers
}

fn memory_styles(&self) -> &PrimaryMap<MemoryIndex, MemoryStyle> {
Expand Down
94 changes: 80 additions & 14 deletions lib/compiler/src/engine/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,29 @@ impl Artifact {
"The provided bytes are not wasmer-universal".to_string(),
));
}
let bytes = &bytes[ArtifactBuild::MAGIC_HEADER.len()..];

let bytes = Self::get_byte_slice(bytes, 0, ArtifactBuild::MAGIC_HEADER.len()).ok_or(
DeserializeError::InvalidByteLength {
expected: ArtifactBuild::MAGIC_HEADER.len(),
got: bytes.len(),
},
)?;

let metadata_len = MetadataHeader::parse(bytes)?;
let metadata_slice: &[u8] = &bytes[MetadataHeader::LEN..][..metadata_len];
let metadata_slice = Self::get_byte_slice(bytes, MetadataHeader::LEN, bytes.len()).ok_or(
DeserializeError::InvalidByteLength {
expected: ArtifactBuild::MAGIC_HEADER.len(),
got: bytes.len(),
},
)?;
let metadata_slice = Self::get_byte_slice(metadata_slice, 0, metadata_len).ok_or(
DeserializeError::InvalidByteLength {
expected: metadata_len,
got: metadata_slice.len(),
},
)?;

let metadata_slice: &[u8] = &metadata_slice[..metadata_len];
let serializable = SerializableModule::deserialize(metadata_slice)?;
let artifact = ArtifactBuild::from_serializable(serializable);
let mut inner_engine = engine.inner_mut();
Expand Down Expand Up @@ -343,7 +363,7 @@ impl Artifact {
// Get pointers to where metadata about local tables should live in VM memory.

let (allocator, memory_definition_locations, table_definition_locations) =
InstanceAllocator::new(&*module);
InstanceAllocator::new(&module);
let finished_memories = tunables
.create_memories(
context,
Expand Down Expand Up @@ -400,7 +420,7 @@ impl Artifact {
.iter()
.map(|init| DataInitializer {
location: init.location.clone(),
data: &*init.data,
data: &init.data,
})
.collect::<Vec<_>>();
handle
Expand Down Expand Up @@ -573,6 +593,16 @@ impl Artifact {
))
}

fn get_byte_slice(input: &[u8], start: usize, end: usize) -> Option<&[u8]> {
fschutt marked this conversation as resolved.
Show resolved Hide resolved
if (start == end && input.len() > start)
|| (start < end && input.len() > start && input.len() >= end)
{
Some(&input[start..end])
} else {
None
}
}

/// Deserialize a ArtifactBuild from an object file
///
/// # Safety
Expand All @@ -583,15 +613,27 @@ impl Artifact {
bytes: &[u8],
) -> Result<Self, DeserializeError> {
let metadata_len = MetadataHeader::parse(bytes)?;
let metadata_slice = Self::get_byte_slice(bytes, MetadataHeader::LEN, bytes.len()).ok_or(
DeserializeError::InvalidByteLength {
expected: MetadataHeader::LEN,
got: bytes.len(),
},
)?;

let metadata_slice = &bytes[MetadataHeader::LEN..][..metadata_len];
let metadata: ModuleMetadata = ModuleMetadata::deserialize(metadata_slice)?;

const WORD_SIZE: usize = mem::size_of::<usize>();
let mut byte_buffer = [0u8; WORD_SIZE];

let mut cur_offset = MetadataHeader::LEN + metadata_len;
byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);

let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)
.ok_or(DeserializeError::InvalidByteLength {
expected: cur_offset + WORD_SIZE,
got: bytes.len(),
})?;

byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
cur_offset += WORD_SIZE;

let num_finished_functions = usize::from_ne_bytes(byte_buffer);
Expand All @@ -603,8 +645,13 @@ impl Artifact {

// read finished functions in order now...
for _i in 0..num_finished_functions {
byte_buffer[0..WORD_SIZE]
.clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)
.ok_or(DeserializeError::InvalidByteLength {
expected: cur_offset + WORD_SIZE,
got: bytes.len(),
})?;

byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
let fp = FunctionBodyPtr(usize::from_ne_bytes(byte_buffer) as _);
cur_offset += WORD_SIZE;

Expand All @@ -625,12 +672,22 @@ impl Artifact {

// read trampolines in order
let mut finished_function_call_trampolines = PrimaryMap::new();
byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);

let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)
.ok_or(DeserializeError::InvalidByteLength {
expected: cur_offset + WORD_SIZE,
got: bytes.len(),
})?;
byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
cur_offset += WORD_SIZE;
let num_function_trampolines = usize::from_ne_bytes(byte_buffer);
for _ in 0..num_function_trampolines {
byte_buffer[0..WORD_SIZE]
.clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)
.ok_or(DeserializeError::InvalidByteLength {
expected: cur_offset + WORD_SIZE,
got: bytes.len(),
})?;
byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
cur_offset += WORD_SIZE;
let trampoline_ptr_bytes = usize::from_ne_bytes(byte_buffer);
let trampoline = mem::transmute::<usize, VMTrampoline>(trampoline_ptr_bytes);
Expand All @@ -640,12 +697,21 @@ impl Artifact {

// read dynamic function trampolines in order now...
let mut finished_dynamic_function_trampolines = PrimaryMap::new();
byte_buffer[0..WORD_SIZE].clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)
.ok_or(DeserializeError::InvalidByteLength {
expected: cur_offset + WORD_SIZE,
got: bytes.len(),
})?;
byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
cur_offset += WORD_SIZE;
let num_dynamic_trampoline_functions = usize::from_ne_bytes(byte_buffer);
for _i in 0..num_dynamic_trampoline_functions {
byte_buffer[0..WORD_SIZE]
.clone_from_slice(&bytes[cur_offset..(cur_offset + WORD_SIZE)]);
let byte_buffer_slice = Self::get_byte_slice(bytes, cur_offset, cur_offset + WORD_SIZE)
.ok_or(DeserializeError::InvalidByteLength {
expected: cur_offset + WORD_SIZE,
got: bytes.len(),
})?;
byte_buffer[0..WORD_SIZE].clone_from_slice(byte_buffer_slice);
let fp = FunctionBodyPtr(usize::from_ne_bytes(byte_buffer) as _);
cur_offset += WORD_SIZE;

Expand Down
2 changes: 1 addition & 1 deletion lib/emscripten/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl EmEnv {

/// Get a reference to the memory
pub fn memory(&self, _mem_idx: u32) -> Memory {
(&*self.memory.read().unwrap()).as_ref().cloned().unwrap()
(*self.memory.read().unwrap()).as_ref().cloned().unwrap()
}

pub fn set_functions(&mut self, funcs: EmscriptenFunctions) {
Expand Down
10 changes: 9 additions & 1 deletion lib/types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,18 @@ pub enum DeserializeError {
/// trying to allocate the required resources.
#[error(transparent)]
Compiler(#[from] CompileError),
/// Input artifact bytes have an invalid length
#[error("invalid input bytes: expected {expected} bytes, got {got}")]
InvalidByteLength {
/// How many bytes were expected
expected: usize,
/// How many bytes the artifact contained
got: usize,
},
}

/// Error type describing things that can go wrong when operating on Wasm Memories.
#[derive(Error, Debug, Clone, PartialEq, Hash)]
#[derive(Error, Debug, Clone, PartialEq, Eq, Hash)]
pub enum MemoryError {
/// Low level error with mmap.
#[error("Error when allocating memory: {0}")]
Expand Down
2 changes: 1 addition & 1 deletion lib/types/src/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl SerializableModule {

/// Returns data initializers to pass to `InstanceHandle::initialize`
pub fn data_initializers(&self) -> &[OwnedDataInitializer] {
&*self.data_initializers
&self.data_initializers
}

/// Returns the memory styles associated with this `Artifact`.
Expand Down
4 changes: 2 additions & 2 deletions lib/vm/src/instance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl Instance {
}

pub(crate) fn module_ref(&self) -> &ModuleInfo {
&*self.module
&self.module
}

fn context(&self) -> &StoreObjects {
Expand Down Expand Up @@ -868,7 +868,7 @@ impl InstanceHandle {
let instance = instance_handle.instance_mut();
let vmctx_ptr = instance.vmctx_ptr();
(instance.funcrefs, instance.imported_funcrefs) = build_funcrefs(
&*instance.module,
&instance.module,
context,
&imports,
&instance.functions,
Expand Down
26 changes: 13 additions & 13 deletions lib/vm/src/libcalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_grow(
/// `vmctx` must be dereferenceable.
#[no_mangle]
pub unsafe extern "C" fn wasmer_vm_memory32_size(vmctx: *mut VMContext, memory_index: u32) -> u32 {
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
let memory_index = LocalMemoryIndex::from_u32(memory_index);

instance.memory_size(memory_index).0
Expand All @@ -205,7 +205,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_size(
vmctx: *mut VMContext,
memory_index: u32,
) -> u32 {
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
let memory_index = MemoryIndex::from_u32(memory_index);

instance.imported_memory_size(memory_index).0
Expand Down Expand Up @@ -303,7 +303,7 @@ pub unsafe extern "C" fn wasmer_vm_table_fill(
/// `vmctx` must be dereferenceable.
#[no_mangle]
pub unsafe extern "C" fn wasmer_vm_table_size(vmctx: *mut VMContext, table_index: u32) -> u32 {
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
let table_index = LocalTableIndex::from_u32(table_index);

instance.table_size(table_index)
Expand All @@ -319,7 +319,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_table_size(
vmctx: *mut VMContext,
table_index: u32,
) -> u32 {
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
let table_index = TableIndex::from_u32(table_index);

instance.imported_table_size(table_index)
Expand All @@ -336,7 +336,7 @@ pub unsafe extern "C" fn wasmer_vm_table_get(
table_index: u32,
elem_index: u32,
) -> RawTableElement {
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
let table_index = LocalTableIndex::from_u32(table_index);

// TODO: type checking, maybe have specialized accessors
Expand Down Expand Up @@ -495,7 +495,7 @@ pub unsafe extern "C" fn wasmer_vm_func_ref(
vmctx: *mut VMContext,
function_index: u32,
) -> VMFuncRef {
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
let function_index = FunctionIndex::from_u32(function_index);

instance.func_ref(function_index).unwrap()
Expand All @@ -510,7 +510,7 @@ pub unsafe extern "C" fn wasmer_vm_func_ref(
pub unsafe extern "C" fn wasmer_vm_elem_drop(vmctx: *mut VMContext, elem_index: u32) {
on_host_stack(|| {
let elem_index = ElemIndex::from_u32(elem_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.elem_drop(elem_index);
})
}
Expand All @@ -530,7 +530,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_copy(
) {
let result = {
let memory_index = LocalMemoryIndex::from_u32(memory_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.local_memory_copy(memory_index, dst, src, len)
};
if let Err(trap) = result {
Expand All @@ -553,7 +553,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_copy(
) {
let result = {
let memory_index = MemoryIndex::from_u32(memory_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.imported_memory_copy(memory_index, dst, src, len)
};
if let Err(trap) = result {
Expand All @@ -576,7 +576,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_fill(
) {
let result = {
let memory_index = LocalMemoryIndex::from_u32(memory_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.local_memory_fill(memory_index, dst, val, len)
};
if let Err(trap) = result {
Expand All @@ -599,7 +599,7 @@ pub unsafe extern "C" fn wasmer_vm_imported_memory32_fill(
) {
let result = {
let memory_index = MemoryIndex::from_u32(memory_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.imported_memory_fill(memory_index, dst, val, len)
};
if let Err(trap) = result {
Expand All @@ -624,7 +624,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_init(
let result = {
let memory_index = MemoryIndex::from_u32(memory_index);
let data_index = DataIndex::from_u32(data_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.memory_init(memory_index, data_index, dst, src, len)
};
if let Err(trap) = result {
Expand All @@ -641,7 +641,7 @@ pub unsafe extern "C" fn wasmer_vm_memory32_init(
pub unsafe extern "C" fn wasmer_vm_data_drop(vmctx: *mut VMContext, data_index: u32) {
on_host_stack(|| {
let data_index = DataIndex::from_u32(data_index);
let instance = (&*vmctx).instance();
let instance = (*vmctx).instance();
instance.data_drop(data_index)
})
}
Expand Down
6 changes: 3 additions & 3 deletions lib/wasi/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ pub fn args_get<M: MemorySize>(
let env = ctx.data();
let (memory, mut state) = env.get_memory_and_wasi_state(&ctx, 0);

let result = write_buffer_array(&memory, &*state.args, argv, argv_buf);
let result = write_buffer_array(&memory, &state.args, argv, argv_buf);

debug!(
"=> args:\n{}",
Expand Down Expand Up @@ -433,7 +433,7 @@ pub fn environ_get<M: MemorySize>(
let (memory, mut state) = env.get_memory_and_wasi_state(&ctx, 0);
trace!(" -> State envs: {:?}", state.envs);

write_buffer_array(&memory, &*state.envs, environ, environ_buf)
write_buffer_array(&memory, &state.envs, environ, environ_buf)
}

/// ### `environ_sizes_get()`
Expand Down Expand Up @@ -5555,7 +5555,7 @@ pub unsafe fn sock_send_file<M: MemorySize>(
sock,
__WASI_RIGHT_SOCK_SEND,
|socket| {
let buf = (&buf[..]).to_vec();
let buf = (buf[..]).to_vec();
socket.send_bytes::<M>(Bytes::from(buf))
}
));
Expand Down