[ABI] [C++] [Coroutines] Promise may not be at the fixed offset from the coroutine frame when Promise has large alignment

[ChuanqiXu9]

Reproducer: https://godbolt.org/z/WhG6z5czo

The ABI of coroutines require the layout of the coroutine frame to be:

struct coroutine_frame {
    void (*resume_fn)();
    void (*destroy_fn)();
    promise_type promise;
    ... // Other needed variables
};

In another words, the layout implies that the promise needs to be at the fixed offset from the coroutine frame. The offset is generally 16 (2 pointer size). However, currently the compiler will try to reorder the position of promise when the alignment of promise exceeds the max default align value (which is generally 16 too, but this 16 is irreverent with the above 16). So here is the problem.

Note that this promise won't be covered by the well-known coro handle alignment problem, which is covered by -fcoro-aligned-allocation option.

[llvmbot]

@llvm/issue-subscribers-coroutines

[llvmbot]

@llvm/issue-subscribers-c-20

[ChuanqiXu9]

Note that we will get the correct address for promise now by handle.promise() because we calculate the offset in the same way now: (OriginOffset + Align - 1) & ~(Align - 1U) instead of using 16 directly. So it won't be problematic for C++ users who will get the promise type by the coroutine_handle<Promise>::promise() member function. It'll only be problematic for people who'll extract the promise type by adding the fixed offset to the frame.

[iains]

The agreement amongst the "coroutines ABI" group was that, when we allow greater alignment than two pointers, the padding would be placed before the resume and destroy:

However the frame pointer would continue to point to the resume() function.

struct coroutine_frame {
   // maybe excess alignment padding here when coro frame alignment > 2 * sizeof (ptr).
    void (*resume_fn)();
    void (*destroy_fn)();
    promise_type promise;
    ... // Other needed variables
};

This was felt to be important (a) to allow interoperability and (b) some 'vendors' are concerned to retain the efficiency of being able to do an indirect call via an ABI register.

[iains]

it is noted that the coroutine C++ [itanium] ABI amendments are still in draft (but actually the same ABI is adopted for MSVC, I believe).