[Bug c++/97566] New: [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[Bug c++/97566] New: [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[natattak at gmail dot com]

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

            Bug ID: 97566
           Summary: [[no_unique_address]] causes miscompiles when mixed
                    with EBO in constexpr context
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: natattak at gmail dot com
  Target Milestone: ---

Example: (https://godbolt.org/z/Y5q5br)

  #include <memory>
  #include <cassert>

  // error disappears if E doesn't inherit from B
  struct B {};
  struct E : B {};

  struct counter {
    constexpr counter() = default;
    constexpr void inc() {
      size++;
    }

    // error disappears if you remove or reorder this value
    int unused = 0;
    int size = 0;
    [[no_unique_address]] E empty = {};
  };

  constexpr int test() {
    counter x;
    x.inc();
    return x.size;
  }

  int main() {
    assert(test() == 1);        // works, unless optimisations enabled
    static_assert(test() == 1); // fails, always
  }

In particular, the runtime assertion only passes on -O0; any higher
optimisation e.g. -O1 causes it to fail.

Compiled as `g++ -std=c++20`.

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-10-29
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I particularly like that you can do this and it compiles and runs OK:

  assert(test() == 1);        // works, unless optimisations enabled
  static_assert(test() == 0);


Or to make it always run OK:

#ifdef __OPTIMIZE__
  assert(test() == 0);
#else
  assert(test() == 1);
#endif
  static_assert(test() == 0);

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org
                 CC|                            |jason at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:a4dfd0f089af33f2af57bf422f9859405b9b4a16

commit r11-6918-ga4dfd0f089af33f2af57bf422f9859405b9b4a16
Author: Jason Merrill <jason@redhat.com>
Date:   Sun Jan 24 00:55:49 2021 -0500

    c++: constexpr and empty fields [PR97566]

    In the discussion of PR98463, Jakub pointed out that in C++17 and up,
    cxx_fold_indirect_ref_1 could use the field we build for an empty base.  I
    tried implementing that, but it broke one of the tuple tests, so I did some
    more digging.

    To start with, I generalized the PR98463 patch to handle the case where we
    do have a field, for an empty base or [[no_unique_address]] member.  This
is
    enough also for the no-field case because the member of the empty base must
    itself be an empty field; if it weren't, the base would not be empty.

    I looked for related PRs and found 97566, which was also fixed by the
patch.
    After some poking around to figure out why, I noticed that the testcase had
    been breaking because E, though an empty class, has an ABI nvsize of one
    byte, and we were giving the [[no_unique_address]] FIELD_DECL that
    DECL_SIZE, whereas in build_base_field_1 empty base fields always get
    DECL_SIZE zero, and various places were relying on that to recognize empty
    fields.  So I adjusted both the size and the checking.  When I adjusted
    check_bases I wondered if we were correctly handling bases with only empty
    data members, but it appears we do.

    I'm deferring the cxx_fold_indirect_ref_1 change until stage 1, as I don't
    think it actually fixes anything.

    gcc/cp/ChangeLog:

            PR c++/97566
            PR c++/98463
            * class.c (layout_class_type): An empty field gets size 0.
            (is_empty_field): New.
            (check_bases): Check it.
            * cp-tree.h (is_empty_field): Declare it.
            * constexpr.c (cxx_eval_store_expression): Check it.
            (cx_check_missing_mem_inits): Likewise.
            * init.c (perform_member_init): Likewise.
            * typeck2.c (process_init_constructor_record): Likewise.

    gcc/testsuite/ChangeLog:

            PR c++/97566
            * g++.dg/cpp2a/no_unique_address10.C: New test.
            * g++.dg/cpp2a/no_unique_address9.C: New test.

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed for GCC 11.

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:a8dd2b3e96590ceccead63d28fc91c956a5f1a73

commit r11-7141-ga8dd2b3e96590ceccead63d28fc91c956a5f1a73
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Feb 8 15:56:11 2021 -0500

    c++: constexpr, union, and no_unique_address [PR98994]

    My second patch for 97566 omits nested CONSTRUCTORs for empty fields, but
we
    do want them for empty union members.

    gcc/cp/ChangeLog:

            PR c++/98994
            PR c++/97566
            * constexpr.c (cxx_eval_store_expression): Only skip empty fields
in
            RECORD_TYPE.

    gcc/testsuite/ChangeLog:

            PR c++/98994
            * g++.dg/cpp2a/no_unique_address12.C: New test.

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:2168bfb81448ae1bfa4351760a23d4ec051c2a00

commit r12-1827-g2168bfb81448ae1bfa4351760a23d4ec051c2a00
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jun 24 17:32:02 2021 -0400

    c++: constexpr aggr init of empty class [PR101040]

    This is basically the aggregate initializer version of PR97566; as in that
    bug, we are trying to initialize empty field 'obj' in 'single' when there's
    no CONSTRUCTOR entry for the 'single' base class subobject of 'derived'. 
As
    with that bug, the fix is to stop trying to add entries for empty fields,
    this time in cxx_eval_bare_aggregate.

    The change to the other function isn't necessary for this version of
    the patch, but seems worthwhile for robustness anyway.

            PR c++/101040
            PR c++/97566

    gcc/cp/ChangeLog:

            * class.c (is_empty_field): Handle null argument.
            * constexpr.c (cxx_eval_bare_aggregate): Discard initializer
            for empty field.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/no_unique_address13.C: New test.

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jason Merrill
<jason@gcc.gnu.org>:

https://gcc.gnu.org/g:38fed4dfa25ac4894546f0589340ebfc5eef3bee

commit r11-8704-g38fed4dfa25ac4894546f0589340ebfc5eef3bee
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jun 24 17:32:02 2021 -0400

    c++: constexpr aggr init of empty class [PR101040]

    This is basically the aggregate initializer version of PR97566; as in that
    bug, we are trying to initialize empty field 'obj' in 'single' when there's
    no CONSTRUCTOR entry for the 'single' base class subobject of 'derived'. 
As
    with that bug, the fix is to stop trying to add entries for empty fields,
    this time in cxx_eval_bare_aggregate.

    The change to the other function isn't necessary for this version of
    the patch, but seems worthwhile for robustness anyway.

            PR c++/101040
            PR c++/97566

    gcc/cp/ChangeLog:

            * class.c (is_empty_field): Handle null argument.
            * constexpr.c (cxx_eval_bare_aggregate): Discard initializer
            for empty field.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/no_unique_address13.C: New test.

[Bug c++/97566] [[no_unique_address]] causes miscompiles when mixed with EBO in constexpr context

[soap at gentoo dot org]

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

David Seifert <soap at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |soap at gentoo dot org

--- Comment #7 from David Seifert <soap at gentoo dot org> ---
(In reply to Jason Merrill from comment #3)
> Fixed for GCC 11.

Thanks for the fix Jason! Could we possibly get a backport to 10, seeing that
it's needed for building chromium? It appears very related to PR98463, which
was backported to GCC 10.