Introduce wasmtime_environ::collections::{OomArc, OomBox}

[fitzgen]

These types mirror their std/alloc counterparts, but only provide fallible constructors and properly handle OOM by returning Err(OutOfMemory).

Note that stable Rust doesn't actually give us any method to build fallible allocation for Arc<T>, and we do not wish to fork Arc<T> since it is full of very subtle unsafe code, so OomArc::new is only actually fallible in practice when using nightly Rust and setting RUSTFLAGS="--cfg arc_try_new" during the build. We use a custom cfg, rather than a cargo feature, so that cargo test --all-features --workspace (which is morally what CI does) continues to Just Work in the workspace, same as we do for e.g. Pulley's usage of the nightly Rust tail calls feature.

Part of #12069

[github-actions]

Subscribe to Label Action

cc @fitzgen

Details

This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

WDYT abuot only having top-level functions for fallible Box and Arc allocation, maybe also with extension traits if we really want? Unlike the collections once a Box or Arc is created there's no need to have a custom wrapper to ensure that only oom-handling methods are called, so maintaining these wrappers may not be necessary if we accept that fuzzing will find Box::new and we know what to replace it with

[alexcrichton]

I think I'd like to push back a bit harder on this. Can you share some disassemblies or such you've been looking at to motivate this helper? I'm really worried about pessimizing the fast path due to the use of an un-inlinable-closure here

[fitzgen]

IIRC, I was previously testing with .context() chaining.

I also just tested this which reflects the allocation case here, but I actually get identical disassemblies with and without the annotations on fn out_of_line_slow_path. So at minimum it seems like we can remove it here.

Going to revisit the context chaining in a minute.

[fitzgen]

This program:

#![crate_type = "cdylib"]

use std::ptr::NonNull;
use wasmtime_internal_error::{Context, Result, try_new_uninit_box};

#[unsafe(no_mangle)]
pub extern "C" fn foo(value: usize, out_ptr: NonNull<u8>) {
    let res: Result<Box<usize>> = (|| {
        let uninit = try_new_uninit_box::<usize>().context("failed to allocate Box<usize>")?;
        Ok(Box::write(uninit, value))
    })();
    unsafe {
        out_ptr.cast().write(res);
    }
}

With out_of_line_slow_path

Details

0000000000007fe0 <foo>:
    7fe0:	41 56                	push   %r14
    7fe2:	53                   	push   %rbx
    7fe3:	48 83 ec 18          	sub    $0x18,%rsp
    7fe7:	48 89 f3             	mov    %rsi,%rbx
    7fea:	49 89 fe             	mov    %rdi,%r14
    7fed:	bf 08 00 00 00       	mov    $0x8,%edi
    7ff2:	ff 15 90 df 03 00    	call   *0x3df90(%rip)        # 45f88 <malloc@GLIBC_2.2.5>
    7ff8:	48 85 c0             	test   %rax,%rax
    7ffb:	74 1b                	je     8018 <foo+0x38>
    7ffd:	31 c9                	xor    %ecx,%ecx
    7fff:	f6 c1 01             	test   $0x1,%cl
    8002:	75 29                	jne    802d <foo+0x4d>
    8004:	4c 89 30             	mov    %r14,(%rax)
    8007:	31 c9                	xor    %ecx,%ecx
    8009:	48 89 0b             	mov    %rcx,(%rbx)
    800c:	48 89 43 08          	mov    %rax,0x8(%rbx)
    8010:	48 83 c4 18          	add    $0x18,%rsp
    8014:	5b                   	pop    %rbx
    8015:	41 5e                	pop    %r14
    8017:	c3                   	ret
    8018:	bf 08 00 00 00       	mov    $0x8,%edi
    801d:	e8 ee fe ff ff       	call   7f10 <wasmtime_internal_error::boxed::try_alloc::out_of_line_slow_path::<core::result::Result<core::ptr::non_null::NonNull<u8>, wasmtime_internal_error::oom::OutOfMemory>, wasmtime_internal_error::boxed::try_alloc::{closure#0}>>
    8022:	48 89 c1             	mov    %rax,%rcx
    8025:	48 89 d0             	mov    %rdx,%rax
    8028:	f6 c1 01             	test   $0x1,%cl
    802b:	74 d7                	je     8004 <foo+0x24>
    802d:	48 89 04 24          	mov    %rax,(%rsp)
    8031:	48 8d 05 e0 3a 03 00 	lea    0x33ae0(%rip),%rax        # 3bb18 <_fini+0x23fc>
    8038:	48 89 44 24 08       	mov    %rax,0x8(%rsp)
    803d:	48 c7 44 24 10 1d 00 	movq   $0x1d,0x10(%rsp)
    8044:	00 00 
    8046:	48 89 e7             	mov    %rsp,%rdi
    8049:	e8 12 b0 ff ff       	call   3060 <<core::result::Result<_, _> as wasmtime_internal_error::context::Context<_, _>>::context::out_of_line_slow_path::<core::result::Result<alloc::boxed::Box<core::mem::maybe_uninit::MaybeUninit<usize>>, wasmtime_internal_error::error::Error>, <core::result::Result<alloc::boxed::Box<core::mem::maybe_uninit::MaybeUninit<usize>>, wasmtime_internal_error::oom::OutOfMemory> as wasmtime_internal_error::context::Context<alloc::boxed::Box<core::mem::maybe_uninit::MaybeUninit<usize>>, wasmtime_internal_error::oom::OutOfMemory>>::context<&str>::{closure#0}>>
    804e:	b9 01 00 00 00       	mov    $0x1,%ecx
    8053:	eb b4                	jmp    8009 <foo+0x29>
    8055:	e8 09 b4 ff ff       	call   3463 <core::panicking::panic_cannot_unwind>
    805a:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)

Without out_of_line_slow_path:

Details

0000000000001100 <foo>:
    1100:	41 56                	push   %r14
    1102:	53                   	push   %rbx
    1103:	50                   	push   %rax
    1104:	48 89 f3             	mov    %rsi,%rbx
    1107:	49 89 fe             	mov    %rdi,%r14
    110a:	bf 08 00 00 00       	mov    $0x8,%edi
    110f:	ff 15 d3 2e 00 00    	call   *0x2ed3(%rip)        # 3fe8 <malloc@GLIBC_2.2.5>
    1115:	48 85 c0             	test   %rax,%rax
    1118:	74 07                	je     1121 <foo+0x21>
    111a:	4c 89 30             	mov    %r14,(%rax)
    111d:	31 c9                	xor    %ecx,%ecx
    111f:	eb 0a                	jmp    112b <foo+0x2b>
    1121:	b9 01 00 00 00       	mov    $0x1,%ecx
    1126:	b8 11 00 00 00       	mov    $0x11,%eax
    112b:	48 89 0b             	mov    %rcx,(%rbx)
    112e:	48 89 43 08          	mov    %rax,0x8(%rbx)
    1132:	48 83 c4 08          	add    $0x8,%rsp
    1136:	5b                   	pop    %rbx
    1137:	41 5e                	pop    %r14
    1139:	c3                   	ret
    113a:	66 0f 1f 44 00 00    	nopw   0x0(%rax,%rax,1)

So yeah, looks like it isn't helpful here either, and if I can't consistently observes better codegen then we should definitely default to not constraining the compiler and removing this macro. I'll remove it from this PR and do another for the existing usage in main.