[MemCpyOpt] Slot optimization miscompilation

[tmiasko]

Running opt --passes=memcpyopt on

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @f(ptr dead_on_unwind noalias noundef writable dereferenceable(80) %a) {
start:
  %b = alloca [80 x i8]
  call void @g(ptr %b)
  call void @llvm.memcpy.p0.p0.i64(ptr %a, ptr %b, i64 80, i1 false)
  ret void
}

declare void @g(ptr)

results in

define void @f(ptr dead_on_unwind noalias noundef writable dereferenceable(80) %a) {
start:
  %b = alloca [80 x i8], align 1
  call void @g(ptr %a)
  ret void
}

In the resulting program, pointer passed to g can be incorrectly observed to equal %a, even though it was impossible originally.

[gz83]

Hello, I'd like to resolve this issue. I've made some changes locally and obtained the following results. Please review and see if they meet your expectations.

Thank you!

; ModuleID = '/home/aaron/test.ll'
source_filename = "/home/aaron/test.ll"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @f(ptr dead_on_unwind noalias noundef writable dereferenceable(80) %a) {
start:
  %b = alloca [80 x i8], align 1
  call void @g(ptr %b)
  call void @llvm.memcpy.p0.p0.i64(ptr %a, ptr %b, i64 80, i1 false)
  ret void
}

declare void @g(ptr)

; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: readwrite)
declare void @llvm.memcpy.p0.p0.i64(ptr noalias writeonly captures(none), ptr noalias readonly captures(none), i64, i1 immarg) #0

attributes #0 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }

[nikic]

I expect the problem here is that we're treating noalias arguments just like other identified function locals -- but of course noalias is only local in the sense of provenance, not address capture.

However, fixing this naively will essentially break the core purpose of the transform, which is forwarding across sret arguments. These are, as far as the transform is concerned, just like any other writable, dead-on-unwind, dereferenceable, aligned, noalias argument.

[efriedma-quic]

Is captures(none) on the caller and/or the callee enough? Do the ABI rules allow us to add those markings for sret? (Don't have time to think about it right now.)

[nikic]

Having it on the callee should be enough. IIRC we can't make sret nocapture in the general case in C++ due to the interaction with copy elision and capturing constructors. I think it should be fine for C...