[Bug c++/111284] New: Some passing-by-value parameters are miscompiled since GCC 9

[Bug c++/111284] New: Some passing-by-value parameters are miscompiled since GCC 9

[de34 at live dot cn]

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

            Bug ID: 111284
           Summary: Some passing-by-value parameters are miscompiled since
                    GCC 9
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Keywords: accepts-invalid, rejects-valid
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: de34 at live dot cn
  Target Milestone: ---

GCC incorrectly rejects the following program since GCC 9
(https://godbolt.org/z/cGK1a1dqK):
```
class self_locator {
public:
    self_locator() = default;
    constexpr self_locator(const self_locator&) noexcept : this_{this} {}
    constexpr self_locator& operator=(const self_locator&) noexcept { return
*this; }

    constexpr bool valid() const noexcept { return this_ == this; }

private:
    self_locator *this_ = this;
};

constexpr bool demonstrator(self_locator x) noexcept
{
    return x.valid();
}

static_assert(demonstrator(self_locator{}), "");
static_assert([](self_locator x){ return x.valid(); }(self_locator{}), "");
```

The `valid` member function should always return true. But if `self_locator` is
passed by value, GCC can sometimes render the parameter in an inconsistent
state (perhaps due to incorrect bitwise copy).

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-09-05
            Summary|Some passing-by-value       |[11/12/13/14 Regression]
                   |parameters are mishandled   |Some passing-by-value
                   |since GCC 9                 |parameters are mishandled
                   |                            |since GCC 9
   Target Milestone|---                         |11.5
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Started with r9-6136-g43574e4ff2afd4

commit 43574e4ff2afd4a2e47c179921a9b5661786ebf3
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Feb 21 22:21:25 2019 +0100

    re PR c++/89285 (ICE after casting the this pointer in the constructor in
C++17 mode)

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
private isn't needed.
struct S {
  S () = default;
  constexpr S (const S &) noexcept : s{this} {}
  constexpr S & operator= (const S &) noexcept { return *this; }
  constexpr bool foo () const noexcept { return s == this; }
  S *s = this;
};

constexpr bool
bar (S x) noexcept
{
  return x.foo ();
}

static_assert (bar (S {}), "");
static_assert ([] (S x) { return x.foo (); } (S {}), "");

The most important change in that commit was to make a copy of the inline body
before it is destructively changed during genericization.  On the other side,
one of the important changes genericization does is adjust accesses to
DECL_BY_REFERENCE PARM_DECLs/RESULT_DECLs.
During the evaluation of static assert, I see we are first evaluating
bar (&TARGET_EXPR <D.2583, <<< Unknown tree: aggr_init_expr
  4
  __ct_comp 
  D.2583
  (struct S *) <<< Unknown tree: void_cst >>> >>>>);
which means that expression is adjusted for the passing of invisiref parms, we
are
passing there address of D.2583 variable with S type which is constructed.
But, later on when trying to constexpr evaluate the body of bar, we see
S::foo (&x);
call, so passing address of x to the method, where x is PARM_DECL with S type.
DECL_BY_REFERENCE even isn't set (yet) on it.
I guess we need to somewhere during constexpr evaluation take into account that
S pointer/reference has been passed to the function (to an invisiref parm) and
the function body still uses it directly rather than changing it into a
pointer/reference and dereferencing it.

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The PARM_DECL has DECL_ARG_TYPE of reference to S.
And the failure is when evaluating the x PARM_DECL as vc_prvalue,
else if (tree v = ctx->global->get_value (t))
doesn't find anything.

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Strange, cxx_bind_parameters_in_call is called on DECL_ARGUMENTS of the
function (so the ones modified by the genericization), while the function
bodies actually have the parameters remaped (stored in the fundef->parms).  Not
really sure how those connect together.

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, /* Associate the bindings with the remapped parms.  */ in constexpr.cc
remaps those then.  So, that loop should take into account also the invisiref
parms disagreements.

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Actually, the binding of parms seems to work fine.
But am out of ideas where this should be fixed.  When evaluating the
comparison, we have lhs
(const struct S *) (struct S *) ((const struct S *) this)->s
and rhs
(const struct S *) this
and we constant evaluate both.  In both cases we find this (foo's artificial
argument in #c2) is &x where x is bar's PARM_DECL.  But in the lhs case, -> is
applied to it,
so x is evaluated as prvalue as {.s=&D.2836} where D.2836 is the local temp
built for AGGR_INIT_EXPR - but nothing during the constexpr evaluation actually
knows that &D.2836 is actually the address of the x argument.  So, lhs folds
into &D.2836 with some cast.
While then constexpr evaluating rhs sees just accessing this which is &x and in
that case x is used as an lvalue, so the folds into &x.  And those two are not
equal.
So, I think we need to treat somewhere the arguments (and return values?) with
TREE_ADDRESSABLE types during constexpr evaluations actually as references
rather than what they are in the source (so what we do after genericization).

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Perhaps we should drop the
      if (TREE_ADDRESSABLE (type))
        /* Undo convert_for_arg_passing work here.  */
        x = convert_from_reference (x);
in cxx_bind_parameters_in_call and somewhere at the start of
cxx_eval_constant_expression's PARM_DECL: case
    case PARM_DECL:
      if (lval && !TYPE_REF_P (TREE_TYPE (t)))
        /* glvalue use.  */;
      else if (tree v = ctx->global->get_value (t))
        r = v;
      else if (lval)
        /* Defer in case this is only used for its type.  */;
      else if (ctx->global->is_outside_lifetime (t))
handle the TREE_ADDRESSABLE (TREE_TYPE (t)) cases differently
(ctx->global->get_value if successful followed essentially by artificial
conversion from reference?  Though not really sure if &x will then cancel out
to & of the TARGET_EXPR etc.
On this testcase after removing that cxx_bind_parameters_in_call hunk
cxx_eval_constant_expression in there results in &D.2836.
x in there is &TARGET_EXPR <D.2836, ...>.

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9, affecting libstdc++'s constexpr std::string

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 57648
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57648&action=edit
gcc14-pr111284.patch

So, I've tried to fix this by constexpr evaluating the arguments passed to
PARM_DECLs with TREE_ADDRESSABLE types in the caller as lvalues rather than
rvaluea and later, if we try to evaluate the PARM_DECL in the callee as lval,
lookup the value and use that, if it is rval constexpr evaluate again as
rvalue.
There is a complication for qualified type, say if the argument is const in the
callee and caller is passing reference to non-const, adjust_temp_type can't
handle that when it isn't a rvalue.

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9, affecting libstdc++'s constexpr std::string

[ppalka at gcc dot gnu.org]

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

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #8)
> Created attachment 57648 [details]
> gcc14-pr111284.patch
> 
> So, I've tried to fix this by constexpr evaluating the arguments passed to
> PARM_DECLs with TREE_ADDRESSABLE types in the caller as lvalues rather than
> rvaluea and later, if we try to evaluate the PARM_DECL in the callee as lval,
> lookup the value and use that, if it is rval constexpr evaluate again as
> rvalue.
> There is a complication for qualified type, say if the argument is const in
> the callee and caller is passing reference to non-const, adjust_temp_type
> can't handle that when it isn't a rvalue.
Interesting, hopefully this fixes the std::string testcases in PR111258 and
related PRs?

And perhaps the following augmented testcase from this PR with a constexpr dtor
that checks valid():

void non_constant();

struct self_locator {
    self_locator() = default;
    constexpr self_locator(const self_locator&) noexcept : this_{this} {}
    constexpr self_locator& operator=(const self_locator&) noexcept { return
*this; }

    constexpr bool valid() const noexcept { return this_ == this; }
    constexpr ~self_locator() { if (!valid()) non_constant(); }

    self_locator *this_ = this;
};

constexpr bool demonstrator(self_locator x) noexcept
{
    return x.valid();
}

static_assert(demonstrator(self_locator{}), "");
static_assert([](self_locator x){ return x.valid(); }(self_locator{}), "");

[Bug c++/111284] [11/12/13/14 Regression] Some passing-by-value parameters are mishandled since GCC 9, affecting libstdc++'s constexpr std::string

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r14-10134-gf541757ba4632e204169dd08a5f10c782199af42
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Apr 25 20:45:04 2024 +0200

    c++: Fix constexpr evaluation of parameters passed by invisible reference
[PR111284]

    My r9-6136 changes to make a copy of constexpr function bodies before
    genericization modifies it broke the constant evaluation of non-POD
    arguments passed by value.
    In the callers such arguments are passed as reference to usually a
    TARGET_EXPR, but on the callee side until genericization they are just
    direct uses of a PARM_DECL with some class type.
    In cxx_bind_parameters_in_call I've used convert_from_reference to
    pretend it is passed by value and then cxx_eval_constant_expression
    is called there and evaluates that as an rvalue, followed by
    adjust_temp_type if the types don't match exactly (e.g. const Foo
    argument and passing to it reference to Foo TARGET_EXPR).

    The reason this doesn't work is that when the TARGET_EXPR in the caller
    is constant initialized, this for it is the address of the
TARGET_EXPR_SLOT,
    but if the code later on pretends the PARM_DECL is just initialized to the
    rvalue of the constant evaluation of the TARGET_EXPR, it is as if there
    is a bitwise copy of the TARGET_EXPR to the callee, so this in the callee
    is then address of the PARM_DECL in the callee.

    The following patch attempts to fix that by constexpr evaluation of such
    arguments in the caller as an lvalue instead of rvalue, and on the callee
    side when seeing such a PARM_DECL, if we want an lvalue, lookup the value
    (lvalue) saved in ctx->globals (if any), and if wanting an rvalue,
    recursing with vc_prvalue on the looked up value (because it is there
    as an lvalue, nor rvalue).

    adjust_temp_type doesn't work for lvalues of non-scalarish types, for
    such types it relies on changing the type of a CONSTRUCTOR, but on the
    other side we know what we pass to the argument is addressable, so
    the patch on type mismatch takes address of the argument value, casts
    to reference to the desired type and dereferences it.

    2024-04-25  Jakub Jelinek  <jakub@redhat.com>

            PR c++/111284
            * constexpr.cc (cxx_bind_parameters_in_call): For PARM_DECLs with
            TREE_ADDRESSABLE types use vc_glvalue rather than vc_prvalue for
            cxx_eval_constant_expression and if it doesn't have the same
            type as it should, cast the reference type to reference to type
            before convert_from_reference and instead of adjust_temp_type
            take address of the arg, cast to reference to type and then
            convert_from_reference.
            (cxx_eval_constant_expression) <case PARM_DECL>: For lval case
            on parameters with TREE_ADDRESSABLE types lookup result in
            ctx->globals if possible.  Otherwise if lookup in ctx->globals
            was successful for parameter with TREE_ADDRESSABLE type,
            recurse with vc_prvalue on the returned value.

            * g++.dg/cpp1z/constexpr-111284.C: New test.
            * g++.dg/cpp1y/constexpr-lifetime7.C: Expect one error on a
different
            line.

[Bug c++/111284] [11/12/13 Regression] Some passing-by-value parameters are mishandled since GCC 9, affecting libstdc++'s constexpr std::string

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
            Summary|[11/12/13/14 Regression]    |[11/12/13 Regression] Some
                   |Some passing-by-value       |passing-by-value parameters
                   |parameters are mishandled   |are mishandled since GCC 9,
                   |since GCC 9, affecting      |affecting libstdc++'s
                   |libstdc++'s constexpr       |constexpr std::string
                   |std::string                 |
             Status|NEW                         |ASSIGNED

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed on the trunk (upcoming 14.1).

[Bug c++/111284] [11/12/13 Regression] Some passing-by-value parameters are mishandled since GCC 9, affecting libstdc++'s constexpr std::string

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:6f1b3f9c97e17aa717ae61bc70afa27adcb7ef44

commit r13-8731-g6f1b3f9c97e17aa717ae61bc70afa27adcb7ef44
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Apr 25 20:45:04 2024 +0200

    c++: Fix constexpr evaluation of parameters passed by invisible reference
[PR111284]

    My r9-6136 changes to make a copy of constexpr function bodies before
    genericization modifies it broke the constant evaluation of non-POD
    arguments passed by value.
    In the callers such arguments are passed as reference to usually a
    TARGET_EXPR, but on the callee side until genericization they are just
    direct uses of a PARM_DECL with some class type.
    In cxx_bind_parameters_in_call I've used convert_from_reference to
    pretend it is passed by value and then cxx_eval_constant_expression
    is called there and evaluates that as an rvalue, followed by
    adjust_temp_type if the types don't match exactly (e.g. const Foo
    argument and passing to it reference to Foo TARGET_EXPR).

    The reason this doesn't work is that when the TARGET_EXPR in the caller
    is constant initialized, this for it is the address of the
TARGET_EXPR_SLOT,
    but if the code later on pretends the PARM_DECL is just initialized to the
    rvalue of the constant evaluation of the TARGET_EXPR, it is as if there
    is a bitwise copy of the TARGET_EXPR to the callee, so this in the callee
    is then address of the PARM_DECL in the callee.

    The following patch attempts to fix that by constexpr evaluation of such
    arguments in the caller as an lvalue instead of rvalue, and on the callee
    side when seeing such a PARM_DECL, if we want an lvalue, lookup the value
    (lvalue) saved in ctx->globals (if any), and if wanting an rvalue,
    recursing with vc_prvalue on the looked up value (because it is there
    as an lvalue, nor rvalue).

    adjust_temp_type doesn't work for lvalues of non-scalarish types, for
    such types it relies on changing the type of a CONSTRUCTOR, but on the
    other side we know what we pass to the argument is addressable, so
    the patch on type mismatch takes address of the argument value, casts
    to reference to the desired type and dereferences it.

    2024-04-25  Jakub Jelinek  <jakub@redhat.com>

            PR c++/111284
            * constexpr.cc (cxx_bind_parameters_in_call): For PARM_DECLs with
            TREE_ADDRESSABLE types use vc_glvalue rather than vc_prvalue for
            cxx_eval_constant_expression and if it doesn't have the same
            type as it should, cast the reference type to reference to type
            before convert_from_reference and instead of adjust_temp_type
            take address of the arg, cast to reference to type and then
            convert_from_reference.
            (cxx_eval_constant_expression) <case PARM_DECL>: For lval case
            on parameters with TREE_ADDRESSABLE types lookup result in
            ctx->globals if possible.  Otherwise if lookup in ctx->globals
            was successful for parameter with TREE_ADDRESSABLE type,
            recurse with vc_prvalue on the returned value.

            * g++.dg/cpp1z/constexpr-111284.C: New test.

    (cherry picked from commit f541757ba4632e204169dd08a5f10c782199af42)

[Bug c++/111284] [11/12 Regression] Some passing-by-value parameters are mishandled since GCC 9, affecting libstdc++'s constexpr std::string

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13 Regression] Some  |[11/12 Regression] Some
                   |passing-by-value parameters |passing-by-value parameters
                   |are mishandled since GCC 9, |are mishandled since GCC 9,
                   |affecting libstdc++'s       |affecting libstdc++'s
                   |constexpr std::string       |constexpr std::string

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed also for 13.3.