dealloc error with dlmalloc >= 2.5

[benluelo]

dlmalloc (the default allocator for wasm) recently added some checks around deallocating memory (issue, relevant code). This is causing issues with cosmwasm, as can be seen in this MRE:

[package]
edition = "2021"
name    = "mre"
version = "0.1.0"

[lib]
crate-type = ["cdylib", "rlib"]

[dependencies]
# if this isn't pinned <= 2.4, runtime error
# dlmalloc = { version = "=0.2.4", features = ["global"] }

use std::mem;

// this is the default allocator, change the version to see the issue
#[global_allocator]
static ALLOC: dlmalloc::GlobalDlmalloc = dlmalloc::GlobalDlmalloc;

#[no_mangle]
extern "C" fn allocate(size: usize) -> u32 {
    alloc(size) as u32
}

pub fn alloc(size: usize) -> *mut Region {
    let data: Vec<u8> = Vec::with_capacity(size);
    let data_ptr = data.as_ptr() as usize;

    let region = build_region_from_components(
        u32::try_from(data_ptr).expect("pointer doesn't fit in u32"),
        u32::try_from(data.capacity()).expect("capacity doesn't fit in u32"),
        0,
    );
    mem::forget(data);
    Box::into_raw(region)
}

/// deallocate expects a pointer to a Region created with allocate.
/// It will free both the Region and the memory referenced by the Region.
#[no_mangle]
extern "C" fn deallocate(pointer: u32) {
    // auto-drop Region on function end
    let _ = unsafe { consume_region(pointer as *mut Region) };
}

#[derive(Default)]
pub struct Response {
    pub messages: Vec<()>,
    pub attributes: Vec<()>,
    pub events: Vec<()>,
    pub data: Option<()>,
}

#[no_mangle]
extern "C" fn instantiate(_ptr0: u32, _ptr1: u32, _ptr2: u32) -> u32 {
    let region = build_region(br#"{"ok":{"messages":[],"attributes":[],"events":[],"data":null}}"#);
    Box::into_raw(region) as u32
}

#[no_mangle]
extern "C" fn interface_version_8() {}

pub fn release_buffer(buffer: Vec<u8>) -> *mut Region {
    let region = build_region(&buffer);
    mem::forget(buffer);
    Box::into_raw(region)
}

pub unsafe fn consume_region(ptr: *mut Region) -> Vec<u8> {
    assert!(!ptr.is_null(), "Region pointer is null");
    let region = Box::from_raw(ptr);

    let region_start = region.offset as *mut u8;
    // This case is explicitely disallowed by Vec
    // "The pointer will never be null, so this type is null-pointer-optimized."
    assert!(!region_start.is_null(), "Region starts at null pointer");

    Vec::from_raw_parts(
        region_start,
        region.length as usize,
        region.capacity as usize,
    )
}

pub fn build_region(data: &[u8]) -> Box<Region> {
    let data_ptr = data.as_ptr() as usize;
    build_region_from_components(
        u32::try_from(data_ptr).expect("pointer doesn't fit in u32"),
        u32::try_from(data.len()).expect("length doesn't fit in u32"),
        u32::try_from(data.len()).expect("length doesn't fit in u32"),
    )
}

fn build_region_from_components(offset: u32, capacity: u32, length: u32) -> Box<Region> {
    Box::new(Region {
        offset,
        capacity,
        length,
    })
}

#[repr(C)]
#[derive(Default, Clone, Copy, Debug)]
pub struct Region {
    /// The beginning of the region expressed as bytes from the beginning of the linear memory
    pub offset: u32,
    /// The number of bytes available in this region
    pub capacity: u32,
    /// The number of bytes used in this region
    pub length: u32,
}

It looks like the issue is in how memory is freed in consume_region, but I'm not entirely sure what exactly.

This is also reproducible with a bare contract with just an instantiate entry point, but I wanted to reduce it to the minimal code possible. this is all based off of cosmwasm 1.5.2. I have not tested other versions.

[aumetra]

Thanks for the report!

After a few tests, I think the issue here is in the release_buffer method. When allocated, the alloc function got the following layout passed to it:

Layout {
    align: 1,
    size: vec_capacity
}

something like that, right?

Well, the function simply discards the capacity of the allocation in favour of the length, which is the actually used and guaranteed initialized part of the capacity.
So when we deallocate we pass in the following layout:

Layout {
    align: 1,
    size: vec_len
}

which is not the same layout (unless the (rather rare) condition of capacity == len is given), therefore technically violating the safety contract documented by the underlying dealloc method, which states:

This function is unsafe because undefined behavior can result if the caller does not ensure all of the following:

• ptr must denote a block of memory currently allocated via this allocator,
• layout must be the same layout that was used to allocate that block of memory.

Will write a patch to fix this up.