[llvm-bugs] [Bug 43098] New: [coroutines] coroutine frame constructs overaligned promise objects at wrong offset

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=43098

            Bug ID: 43098
           Summary: [coroutines] coroutine frame constructs overaligned
                    promise objects at wrong offset
           Product: new-bugs
           Version: trunk
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: lewissbaker at gmail.com
                CC: htmldeveloper at gmail.com, llvm-bugs at lists.llvm.org

If a coroutine has a promise_type that has an alignment that is greater than 2
* sizeof(void*) (eg. >= 32 byte alignment on 64-bit platforms) then there is an
inconsistency in the calculation of the promise object offset within the
coroutine frame struct generated in CoroFrame.cpp and the offset claculated by
the lowering of __llvm.coro.promise() in CoroEarly.cpp

This results in a coroutine_handle constructed using
coroutine_handle::from_promise() within the promise_type::get_return_object()
method calculating and incorrect coroutine frame address which then promptly
segfaults if you try to call resume() or destroy() on that coroutine_handle.

See https://godbolt.org/z/B_SsFR

It appears that when the promise type has alignment greater than 2 *
sizeof(void*) that the code generated by the ramp function continues to
construct the promise object at offset 2 * sizeof(void*) into the coroutine
frame, whereas the __builtin_coro_promise() / llvm.coro.promise() intrinsics
(used by coroutine_handle::promise/from_promise) are using an alignment-padded
offset for the promise relative to the coroutine frame pointer.

This inconsistency causes two problems:
1. Inside the `promise_type::get_return_object()` if you call
coroutine_handle<promise_type>::from_promise() then you will get back a
coroutine_handle that has the wrong address. Attempting to use this handle in
any way to resume/destroy the coroutine will almost certainly lead to an
access-violation as it will be trying to call a function pointer loaded from
potentially uninitialised memory. This seems to be what is happening in the
issue reported by the linked post.

2. If your coroutine type does not call coroutine_handle::from_promise() and
and manages to execute until you hit a suspend-point, then if you try to use
the coroutine_handle passed to await_suspend() to access the promise by calling
the .promise() method then you will receive a reference to a different address
than the one that the promise was constructed at by the coroutine ramp
function.


The problem is possibly in the buildFrameType() function in CoroFrame.cpp.

This calculates the layout of the ABI-stable part of the coroutine frame struct
as follows:

    Type *IndexType = Type::getIntNTy(C, IndexBits);
    Types.push_back(FnPtrTy);
    Types.push_back(FnPtrTy);
    Types.push_back(PromiseType);
    Types.push_back(IndexType);

However, when adding later members to the coroutine frame type it has some
additional logic to deal with adding padding members for alignment which is not
present when adding the members to the ABI-stable section.

      if (unsigned AllocaAlignment = AI->getAlignment()) {
        // If alignment is specified in alloca, see if we need to insert extra
        // padding.
        if (auto PaddingTy = Padder.getPaddingType(Ty, AllocaAlignment)) {
          Types.push_back(PaddingTy);
          Padder.addType(PaddingTy);
        }
      }

Do we need to add this same padding logic when adding the PromiseType and
IndexType members so that the offset calculated for the promise is consistent
with llvm.coro.promise() intrinsic?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190823/e4d5722e/attachment.html>