FFI calls may create out-of-bounds loads

[dotdash]

Given this Rust code:

#[repr(C)]
struct S {
    f1: i32,
    f2: i32,
    f3: i32,
}

extern {
    fn foo(s: S);
}

A call to foo() generates IR like this on x86_64 Linux:

  %S = type { i32, i32, i32 }
; ...
  %arg = alloca %S
; ...
  %2 = bitcast %S* %s to i8*
  %3 = bitcast %S* %arg to i8*
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %3, i8* %2, i64 12, i32 4, i1 false)
  %4 = bitcast %S* %arg to { i64, i64 }*
  %5 = load { i64, i64 }, { i64, i64 }* %4
  call void @x({ i64, i64 } %5)

So we have a 12 byte alloca from which we then read 16 bytes.

[abonander]

Isn't this subject to alignment by LLVM?

[dotdash]

@cybergeek94 what do you mean? The alloca is only 12 bytes large. That's also the amount of bytes we write in the memcpy, but then we ask to read data for a type that is 16 bytes large. I don't see how alignment is relevant here.

[dotdash]

@cybergeek94 here's a demo: https://gist.github.com/dotdash/2da61d4d24fa79e0a9bb

Build without optimizations on x86_64 Linux, and you should be able to observe z in the C code, although it hasn't been passed as an argument. The mismatch in the struct type definitions is deliberate to show the problem.

[eefriedman]

Reading past the end of an alloca has well-defined behavior in LLVM IR. I guess we might end up with false positives if we ever support msan.

[dotdash]

@eefriedman are you implying that the generated IR is fine? If so, what is the defined behaviour / where can I read up on that?

http://llvm.org/docs/LangRef.html#pointeraliasing reads as if it is undefined.

Without optimizations, this gives random output:

fn foo() -> (i64, i64) {
    let x = (0i32, 0i32, 0i32);
    let y = &x as *const _ as *const (i64, i64);
    unsafe {
        *y
    }
}

fn main() {
    println!("foo() = {:?}", foo());
}

With optimizations, SROA ignores the out-of-bounds read and emits a warning in debug mode, which doesn't seem like it is well-defined behaviour to me either.

[eefriedman]

Basically, if a pointer points to valid memory, and is sufficiently aligned, you can load bytes up to the alignment... LLVM itself does this at https://github.com/llvm-mirror/llvm/blob/ef4e0adfe75684bf9017c8a9c5309e77b8aa3b7b/lib/Analysis/MemoryDependenceAnalysis.cpp#L322 etc.

That said, looking a bit more closely, the given testcase performs an "align 8" load from an "align 4" pointer, which is more problematic.

[dotdash]

Thanks! That looks like it just assumes that the load won't trap. Part of my concern here is that we might expose additional data this way, like observing z in that C code example. Though I guess optimizations might eventually leave the upper 32bits undefined and expose data anyway, so this might be moot.

The alignment problem is probably due to bad usage of load_ty. Ultimately, we should load and pass the struct elements individually anyway.

[Mark-Simulacrum]

This has been fixed, we now only load 12 bytes: {i64, i32} instead of {i64, i64}. Marking as E-needstest.

#[repr(C)]
struct S {
    f1: i32,
    f2: i32,
    f3: i32,
}

extern {
    fn foo(s: S);
}

fn main() {
    let s = S { f1: 1, f2: 2, f3: 3 };
    unsafe {
        foo(s);
    }
}

  %_3 = alloca %S
  %s = alloca %S
  %0 = getelementptr inbounds %S, %S* %s, i32 0, i32 0
  store i32 1, i32* %0
  %1 = getelementptr inbounds %S, %S* %s, i32 0, i32 2
  store i32 2, i32* %1
  %2 = getelementptr inbounds %S, %S* %s, i32 0, i32 4
  store i32 3, i32* %2
  %3 = bitcast %S* %s to i8*
  %4 = bitcast %S* %_3 to i8*
  ; Copy 12 bytes with alignment of 4
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %4, i8* %3, i64 12, i32 4, i1 false)
  %5 = bitcast %S* %_3 to { i64, i32 }*
  ; Load 12 bytes -- this is now fine, not loading 16 bytes
  %6 = load { i64, i32 }, { i64, i32 }* %5, align 4
  call void @foo({ i64, i32 } %6)
  br label %bb1