[PATCH] D96922: [Coroutine] Check indirect uses of alloca when checking lifetime info

[Xun Li via Phabricator via llvm-commits]

lxfind added a comment.

> We don't need more information to do that; we need to use the information we have *correctly*.  When you're analyzing uses of the alloca, you need to deal with uses that you don't fully understand by just giving up on the analysis and assuming that the object is used for its entire lifetime (as limited by lifetime intrinsics, if possible, but the entire function if not).

The fundamental problem (and what makes this so hard) is that we need to be very accurate to make the program correct. Being conservative is insufficient for correctness. This is due to the fact that we often access data after the frame is destroyed, and anything accessed after the frame is destroyed cannot be on the frame. So for those data we need to be able to accurately tell if they should be on the stack or if the developer has a bug.

Below is some IR that troubled me for a while (removed some lines that are irrelevant to simplify it). The alloca in question is % __coro_gro. Its lifetime started early, used after coro.end. 
Since it's used after coro.end, at which point the coroutine frame is already destroyed, __coro_gro cannot be put on the frame and has to be put on the stack.
However technically it lives across suspension points and should be put on the frame. 
The current implementation happens to work and put it on the stack because suspension checker stops propagating when it sees coro.end. So any use of data after coro.end is considered to not cross suspension point to anything before coro.suspend.
Also, I am not sure if we can prove that _ZN5folly15expected_detail7PromiseIiN12_GLOBAL__N_13ErrEE17get_return_objectEv never captures %__coro_gro. Maybe a function cannot capture an argument that's sret? Not sure.

  ; Function Attrs: sanitize_address uwtable
  define hidden i64 @_Z3foov() #3 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
  entry:
    %__coro_gro = alloca %"struct.folly::expected_detail::PromiseReturn", align 8
    %__promise = alloca %"struct.folly::expected_detail::Promise", align 8
    %0 = bitcast %"struct.folly::expected_detail::Promise"* %__promise to i8*
    %1 = call token @llvm.coro.id(i32 16, i8* nonnull %0, i8* bitcast (i64 ()* @_Z3foov to i8*), i8* null)
    %2 = call i1 @llvm.coro.alloc(token %1)
    br i1 %2, label %coro.alloc, label %invoke.cont21
  
  coro.alloc:                                       ; preds = %entry
    %3 = tail call i64 @llvm.coro.size.i64()
    %call = call i8* @_Znwm(i64 %3)
    br label %invoke.cont21
  
  invoke.cont21:                                    ; preds = %coro.alloc, %entry
    %4 = phi i8* [ null, %entry ], [ %call, %coro.alloc ]
    %5 = call i8* @llvm.coro.begin(token %1, i8* %4) #13
    %6 = bitcast %"struct.folly::expected_detail::PromiseReturn"* %__coro_gro to i8*
    call void @llvm.lifetime.start.p0i8(i64 24, i8* nonnull %6) #6
    call fastcc void @_ZN5folly15expected_detail7PromiseIiN12_GLOBAL__N_13ErrEE17get_return_objectEv(%"struct.folly::expected_detail::PromiseReturn"* nonnull sret %__coro_gro, %"struct.folly::expected_detail::Promise"* nonnull %__promise) #6
    %call26 = call fastcc zeroext i1 @_ZNK5folly15expected_detail9AwaitableIiN12_GLOBAL__N_13ErrEE11await_readyEv(%"struct.folly::expected_detail::Awaitable"* nonnull %tmpcast) #6
    br i1 %call26, label %cont37, label %await.suspend
  
  await.suspend:                                    ; preds = %invoke.cont21
    %10 = call token @llvm.coro.save(i8* null)
    %11 = bitcast %"class.std::experimental::coroutines_v1::coroutine_handle.21"* %retval.i109 to i8*
    call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %11)
    call fastcc void @_ZNSt12experimental13coroutines_v116coroutine_handleIN5folly15expected_detail7PromiseIiN12_GLOBAL__N_13ErrEEEEC2Ev(%"class.std::experimental::coroutines_v1::coroutine_handle.21"* nonnull %retval.i109) #6
    call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %11)
    invoke fastcc void @_ZN5folly15expected_detail9AwaitableIiN12_GLOBAL__N_13ErrEE13await_suspendIiEEvNSt12experimental13coroutines_v116coroutine_handleINS0_7PromiseIT_S3_EEEE(%"struct.folly::expected_detail::Awaitable"* nonnull %tmpcast, i8* %5)
            to label %invoke.cont35 unwind label %lpad
  
  invoke.cont35:                                    ; preds = %await.suspend
    %12 = call i8 @llvm.coro.suspend(token %10, i1 false)
    switch i8 %12, label %coro.ret [
      i8 0, label %cont37
      i8 1, label %cleanup49
    ]
  
  ...
  
  cleanup83:                                        ; preds = %invoke.cont54, %cleanup49, %cleanup49.thread
    call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0) #6
    %24 = call i8* @llvm.coro.free(token %1, i8* %5)
    %25 = icmp eq i8* %24, null
    br i1 %25, label %coro.ret, label %coro.free
  
  coro.free:                                        ; preds = %cleanup83
    %26 = tail call i64 @llvm.coro.size.i64()
    call void @_ZdlPvm(i8* nonnull %24, i64 %26) #6
    br label %coro.ret
  
  coro.ret:                                         ; preds = %coro.free, %cleanup83, %invoke.cont35
    %27 = call i1 @llvm.coro.end(i8* null, i1 false) #13
    %call95 = invoke fastcc i64 @_ZNR5folly15expected_detail13PromiseReturnIiN12_GLOBAL__N_13ErrEEcvNS_8ExpectedIiS3_EEEv(%"struct.folly::expected_detail::PromiseReturn"* nonnull %__coro_gro)
            to label %cleanup.action unwind label %lpad93
  ...
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96922/new/

https://reviews.llvm.org/D96922