Improve conventions for returning arrays from functions

[briansmith]

In some cases, e.g. in the implementations of ring::aead::seal_in_place and ring_aead::open_in_place, we pass a function an uninitialized (actually, zero-initialized) buffer and then expect that function to have filled the buffer with data if it returns Ok(()). Instead, we should consider changing the result type of these functions to Result<[u8; N], ()> to better use the type system. Although it looks like this would result in more copying, I was told that the compiler actually optimizes the proposed new form into the current form. This needs to be verified.

After that is done, we should also improve the case where a FFI function fills in the output array. In this case, I guess constructing an array using [0; N] and then calling the FFI function is definitely going to cause a memset to occur before the call to the FFI function. We should probably just use mem::uninitialized instead, to avoid that memset, in code where the performance of this memset might matter (e.g. ring::aead and ring::digest).

[briansmith]

See also #128.

[frewsxcv]

I was curious about this, so I decided to look at this briefly. This doesn't necessarily help much (I'm mostly just speaking out loud), though might as well share what I've done related to this. Given this Rust code:

fn foo(mut a: [u8; 1024]) -> [u8; 1024] {
    let mut i = 0;
    for b in a.iter_mut() {
        *b = i;
        i += 1;
    }
    a
}

fn main() {}

I generated the MIR and here's a relevant section:

MIR for fn foo::foo (id=4)
fn(arg0: [u8; 1024]) -> [u8; 1024] {
    let mut var0: [u8; 1024]; // a
    ...
    let mut tmp13: [u8; 1024];
    ...
    bb13: {
        tmp13 = var0;
        return = tmp13;
        goto -> bb1;
    }

I'm going to assume that the tmp13 = var0; is treated as copying since we're not dealing with references.

LLVM IR:

; Function Attrs: uwtable
define internal void @_ZN3foo20h35dc108034dcf5e0eaaE([1024 x i8]* noalias nocapture sret dereferenceable(1024), [1024 x i8]* noalias nocapture dereferenceable(1024)) unnamed_addr #0 !dbg !40 {
entry-block:
  %a = alloca [1024 x i8]
  %i = alloca i8
  %_result = alloca {}
  ...

This is first few lines for the foo function. As you can see, it looks like an allocation happens at the LLVM IR level.

I didn't have time tonight to look to closely at the ASM to see if LLVM is doing the optimization here.

EDIT: Playpen link

[briansmith]

Thanks for looking at this. Your function foo is doing something that seems weird to me. I was thinking something more along the lines of:

fn foo() -> [u8; 1024] {
    let mut b = [0u8; 1024];
    for i in 0..b.len() {
        b[i] = i as u8;
    }
    b
}

Indeed, rustc doesn't optimize away the copy of b. However, in our case we're only needing to return arrays that are small (512 bits == 64 bytes for digests, 128 bits == 16 bytes for AEAD authentication tags). Thus, the compiler might optimize them better. It would be worth modifying ring::aead to work this way and then look at the generated ASM to see if it is worse.

[briansmith]

It seems rust-lang/rfcs#788 is already filed about implementing the expected optimization.

[briansmith]

Closing this. All of the API improvements of this sort will be handled on a case-by-case basis.