Add wasm32 implementation for libc

nmattia commented 2 years ago

Fixes #43 .

This adds a wasm32 implementation for free, malloc and realloc. Originally written by @rodrigorc in https://github.com/rust-lang/libc/pull/1092 and adapted.

Disclaimer: I don't know rust, and I haven't really looked at the implementation; I blindly copy-pasted. This should probably be reviewed before it goes in.

kornelski commented 2 years ago

Thanks for the pr.

I'm a little surprised that this is needed. It sounds like a common problem for wasm. Shouldn't these be provided by emscripten or some other crate?

I'm worried that if I add these symbols, but someone links a "real" libc separately, they will conflict.

nmattia commented 2 years ago

The target here really is wasm32-unknown-unknown, which doesn't rely on emscripten. The rust team have said they wouldn't want to support wasm in the libc crate so I reckon that shouldn't be an issue, but I'm not very experienced with rust.

There may be another crate that provides some libc functions, though I couldn't find it!

rodrigorc commented 2 years ago

Hi! I'm the creator of the original (rejectec) pull-request for rust-libc.

The weird thing with this situation is that Wasm has no libc at all, instead it has its own memory allocator, and rust-wasm rightfully uses this as the global allocator.

So there are no malloc/free functions anywhere. Not a big deal, because in Rust nobody uses these functions directly. Or do they? As it happens, some libraries that interact directly with native modules need to use malloc/free to pass around dynamic memory. But in Wasm there is no such modules! So what is happening?

Well, what happens is that there was some crates out there that can be used both with C native libraries or with pure Rust reimplementations of the same APIs. And some of those APIs used to pass memory blocks allocated with malloc, even though there was Rust on both sides.

This project, lodepng-rust, IIUIC, is 100% Rust, so there is little need to use malloc in the first place. I think you should replace every call to malloc/free with native Rust equivalents.

For example:

fn vec_into_raw(v: Vec<u8>) -> Result<(*mut u8, usize), crate::Error> {
    unsafe {
        let len = v.len();
        let data = lodepng_malloc(len) as *mut u8;
        if data.is_null() {
            Err(crate::Error::new(83))
        } else {
            slice::from_raw_parts_mut(data, len).clone_from_slice(&v);
            Ok((data, len))
        }
    }
}

could be reimplemented without copying as:

fn vec_into_raw(v: Vec<u8>) -> (*mut u8, usize) {
    let ptr = v.as_ptr();
    let len = v.len();
    std::mem::forget(v);
    (data, len)
}

kornelski commented 2 years ago

lodepng has this quirk of using raw malloc, because it exposes a C API, and callers are expected to be able to free data allocated by lodepng.

nmattia commented 2 years ago

@kornelski thanks for the feedback and @rodrigorc thanks for the detailed explanation!

I'll change the PR for this file only to be used in wasm32-unknown-unknown. But first I need to debug a downstream issue to make sure this isn't caused by this change.

nmattia commented 2 years ago

Done! Here's the updated commit message for reference: