[Bug c++/104177] New: [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[Bug c++/104177] New: [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[ldalessandro at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

            Bug ID: 104177
           Summary: [diagnostic] basic.align#9 should emit diagnostic for
                    unsupported alignas
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ldalessandro at gmail dot com
  Target Milestone: ---

Created attachment 52264
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52264&action=edit
Downloaded CE example.

It appears that gcc doesn't currently support extended alignment for locals in
coroutine frames.

I believe this is **not** a bug, as I think that such extended alignment
support is implementation defined, however I believe that
https://timsong-cpp.github.io/cppwp/n4861/basic.align#9 says that a program
that requests unsupported extended alignment is ill-formed, and thus should get
a diagnostic.

Given the infrastructure required to demonstrate this bug, it's difficult for
me to provide a self contained example, however given the std::generator
support proposed in
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2168r3.pdf, and
implemented in https://godbolt.org/z/T58h1W, the failure is:

  #include <cassert>
  #include <cstddef>
  #include <cstdint>
  #include <cstdio>

  struct alignas(32) Foo {
  };

  static std::generator<const uint64_t> foo() {
      Foo m{};
      fprintf(stdout, "%lu\n", uintptr_t(&m) % alignof(Foo));
      fflush(stdout);
      assert(uintptr_t(&m) % alignof(Foo) == 0);
      co_yield 1;
  }

  int main() {
      for (auto && x : foo()) {
          return x;
      }
  }

CE link: https://godbolt.org/z/GTKPa81W8

Originally came to my attention in a discussion with
https://github.com/bniehoff.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[ldalessandro at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #1 from Luke Dalessandro <ldalessandro at gmail dot com> ---
Also described in
https://stackoverflow.com/questions/66546906/is-it-defined-behavior-to-place-exotically-aligned-objects-in-the-coroutine-stat.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It should be supported. There might be a dup copying the alignment though for
the coroutines.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> It should be supported. There might be a dup copying the alignment though
> for the coroutines.

Bug not dup. Autocorrect got to me.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[ldalessandro at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #4 from Luke Dalessandro <ldalessandro at gmail dot com> ---
Oh, that would be great. I tried relatively hard to find a bug like that, but I
have previously shown a surprising level of incompetence with bugzilla search.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
What I am trying to say is there is a bug in gcc when dealing with alignas with
coroutines. I don't think there is a dup of this bug in the database.
I am also saying is this should work. When I get in front of my computer next,
I will to see if I can see something obviously going wrong.

Also gcc does implement the full alignas up to a very big alignment. Though I
remember seeing some other alignas bugs and I will find them and link them
here.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So this works for me on the trunk.

[Bug c++/104177] [diagnostic] basic.align#9 should emit diagnostic for unsupported alignas

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|12.0                        |11.1.1

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It didn't work in GCC 11.2.0 though.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[diagnostic] basic.align#9  |coroutine frame is not
                   |should emit diagnostic for  |being allocated with the
                   |unsupported alignas         |correct alignment

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The problem is GCC produces:

  _1 = .CO_FRAME (6144, _Coro_frameptr);
  _Coro_frameptr = std::promise_base_alloc<void>::operator new (_1);

But also does not use

std::promise_base_alloc<void>::operator new (std::size_t size, std::align_val_t
align);

There could also be a Defect in the C++ standard when it comes to coroutines
where it misses out on overaligned types on the stack.
Someone who how more knowledge on that should be able to answer the question.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
so I think you might have found a defect in the C++ standard dealing with
coroutines. I looked and there is no mention of alignment when it comes to the
state of the coroutine at all even.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note LLVM/clang has the same issue and there was a patch for the issue here:
https://reviews.llvm.org/D106248

I don't see the followup patch though.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Oh:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2014r0.pdf

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
See the discussion at
https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg222834.html

I wonder what this means for GCC here.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-01-28

--- Comment #13 from Iain Sandoe <iains at gcc dot gnu.org> ---

the design that is agreed (by the coroutine "ABI group", at least) is this:

If the frame requires greater than pointer alignment * 2, we should place the
padding **BEFORE** the (resume*)() and (destroy*)() members - and the frame
pointer will continue to point to the (resume*)() member.  So that, from the
point of view of a continuing caller - there is no difference between such
frames and ones less-aligned.

This requires:
1. keeping some accounting information on the side so that the correct memory
can be freed.

2. having an allocator that honours alignment > 2 * pointer (well. I'd guess
most do that, but we really need the aligned allocator change, however [as
noted in the thread referenced] that change did not make C++20).  We can, of
course, follow clang.

3. ensuring that excess and user alignments are honoured in the placement of
frame entries (I wonder if we actually need to honour anything larger than then
stack would allow).

----

Fro my part, I completely agree that GCC coroutines should support fields will
suitable alignment for larger vector objects.  It's just a question of finding
time to implement the changes, as with all projects ....

The idea is that coroutines form different compilers should be inter-callable
(even though the frame layout might be completely different beyond that
specified for ABI).

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[ldalessandro at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #14 from Luke Dalessandro <ldalessandro at gmail dot com> ---
Thanks for the information Iain. 

Is there something short-term where gcc could provide an "unimplemented"
failure or warning diagnostic for requests for coroutine frames with extended
alignment?

This could save a lot of debugging angst.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[davidledger at live dot com.au]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

David Ledger <davidledger at live dot com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |davidledger at live dot com.au

--- Comment #15 from David Ledger <davidledger at live dot com.au> ---
This is a complete minimum reproduction, just to aid Iain Sandoe:

```CPP
#include <coroutine>
using namespace std;
#include <cstddef>
static char buffer[4];
template <typename a> struct b {
  struct promise_type {
    void *operator new(size_t) { return &buffer[3]; }
    void get_return_object();
    void unhandled_exception();
    suspend_always initial_suspend();
    suspend_always final_suspend() noexcept;
    suspend_always yield_value(a);
  };
};
b<max_align_t *> c() {
  max_align_t buffer[1];
  for (auto d : buffer)
    co_yield &d;
}
```

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[davidledger at live dot com.au]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #16 from David Ledger <davidledger at live dot com.au> ---
The above example produces the xmm instruction on a clearly misaligned value.

I was searching the assembly using:
```SH
#!/bin/bash

g++-11 main.cpp -std=c++2a -O3 -march=native -S
grep -E "vmovdqu\s%xmm0,\s3\+_ZL6buffer" main.s
exit $?
```

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #17 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to David Ledger from comment #15)
> This is a complete minimum reproduction, just to aid Iain Sandoe:

This is well defined code? because I thought operator new has alignment
requirements as defined by the C++ standard ...

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #18 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
basic.stc.dynamic.allocation/3 seems to be the important part here.

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #19 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #17)
> (In reply to David Ledger from comment #15)
> > This is a complete minimum reproduction, just to aid Iain Sandoe:
> 
> This is well defined code? because I thought operator new has alignment
> requirements as defined by the C++ standard ...

That example is undefined even by the standard operator new according
basic.stc.dynamic.allocation/3.3 rule. (and undefined even worse by not enough
for the size too).

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[davidledger at live dot com.au]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

--- Comment #20 from David Ledger <davidledger at live dot com.au> ---
Yeah, your right. I had noticed the mistake and reduced the code without
thinking enough:

```CPP
#include <coroutine>
#include <cstddef>
#include <cstdint>
#include <cassert>
using namespace std;

struct overaligned { alignas(128) char padding[128]; };
alignas(2048) static char buffer[2048];

template <typename a>
struct b
{
    struct promise_type
    {
        void * operator new(size_t) { return
&buffer[alignof(std::max_align_t)]; }
        b<a> get_return_object() { return b<a>{
std::coroutine_handle<promise_type>::from_promise(*this) }; }
        void unhandled_exception() {}
        suspend_always initial_suspend() { return {}; }
        suspend_always final_suspend() noexcept { return {}; }
        suspend_always yield_value(a v) { return {}; }
    };
    std::coroutine_handle<promise_type> handle;
};

b<std::uintptr_t> c()
{
    overaligned temp{};
    auto addr = reinterpret_cast<std::uintptr_t>(&temp);
    assert((addr % alignof(overaligned)) == 0);
    co_yield addr;
}

int main() {
    auto f = c();
    f.handle.resume();
}
```
This will trigger the assert above, you should see:
`Assertion failed: (addr % alignof(overaligned)) == 0, file
F:/Files/Git/hz/test/Scratch/main.cpp, line 29`

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fangzhuhe at 126 dot com

--- Comment #21 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110967 has been marked as a duplicate of this bug. ***

[Bug c++/104177] coroutine frame is not being allocated with the correct alignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104177

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jankodedic2 at gmail dot com

--- Comment #22 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 114900 has been marked as a duplicate of this bug. ***