public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/99859] New: constexpr evaluation with member function is incorrect
@ 2021-03-31 22:54 ldalessandro at gmail dot com
  2021-03-31 23:15 ` [Bug c++/99859] " ldalessandro at gmail dot com
                   ` (23 more replies)
  0 siblings, 24 replies; 25+ messages in thread
From: ldalessandro at gmail dot com @ 2021-03-31 22:54 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 99859
           Summary: constexpr evaluation with member function is incorrect
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ldalessandro at gmail dot com
  Target Milestone: ---

I'm at a loss for how to describe this.

I have an intrusive reference counting smart pointer that manipulates the
pointed-to object's `count_` member either explicitly or via a member function.
The explicit version works fine, the member function does not (both are fine in
clang).

See the difference with -DUSE_DEC in https://godbolt.org/z/rhb7e6rjr.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
@ 2021-03-31 23:15 ` ldalessandro at gmail dot com
  2021-04-01 10:29 ` jakub at gcc dot gnu.org
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: ldalessandro at gmail dot com @ 2021-03-31 23:15 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from Luke Dalessandro <ldalessandro at gmail dot com> ---
It was pointed out that it _also_ works if I change

> static_assert(foo());

to 

> constexpr bool b = foo();
> static_assert(b);

static_assert(foo());

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
  2021-03-31 23:15 ` [Bug c++/99859] " ldalessandro at gmail dot com
@ 2021-04-01 10:29 ` jakub at gcc dot gnu.org
  2021-04-01 14:15 ` jakub at gcc dot gnu.org
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 10:29 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2021-04-01
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We want testcases in our bugzilla rather than external links.
No need for any headers,
template <class T>
struct intrusive_ptr
{
  T *ptr = nullptr;
  constexpr explicit intrusive_ptr(T* p) : ptr(p) {
    ++ptr->count_;
  }
  constexpr ~intrusive_ptr() {
    if (ptr->dec() == 0)
//    if (--ptr->count_ == 0)
      delete ptr;
  }
  constexpr intrusive_ptr(intrusive_ptr const& a) : ptr(a.ptr) {
    ++ptr->count_;
  }
};

struct Foo {
  int count_ = 0;
  constexpr int dec() {
    return --count_;
  }
};
template class intrusive_ptr<Foo>;

constexpr void bar(intrusive_ptr<Foo> a) 
{
//  if (a.ptr->count_ != 2) throw 1;
}

constexpr bool foo() {
  intrusive_ptr a(new Foo());
  bar(a);
  return true;
}

static_assert(foo());

reproduces too.  Wonder if it isn't related to the instantiation of
intrusive_ptr<Foo>::~intrusive_ptr() while constexpr evaluating the static
assert.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
  2021-03-31 23:15 ` [Bug c++/99859] " ldalessandro at gmail dot com
  2021-04-01 10:29 ` jakub at gcc dot gnu.org
@ 2021-04-01 14:15 ` jakub at gcc dot gnu.org
  2021-04-07 11:39 ` jakub at gcc dot gnu.org
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 14:15 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The instantiation isn't the problem,
template <class T>
struct intrusive_ptr
{
  T *ptr = nullptr;
  constexpr explicit intrusive_ptr(T* p) : ptr(p) {
    ++ptr->count_;
  }
  constexpr ~intrusive_ptr() {
    if (ptr->dec() == 0)
      delete ptr;
  }
  constexpr intrusive_ptr(intrusive_ptr const& a) : ptr(a.ptr) {
    ++ptr->count_;
  }
};

struct Foo {
  int count_ = 0;
  constexpr int dec() {
    return --count_;
  }
};

constexpr bool baz() {
  Foo f { 4 };
  intrusive_ptr a(&f);
  return true;
}
constexpr bool x = baz();

constexpr void bar(intrusive_ptr<Foo> a) 
{
}

constexpr bool foo() {
  intrusive_ptr a(new Foo());
  bar(a);
  return true;
}

static_assert(foo());

fails too, and during the finish_static_assert fold_nondependent_expr
evaluation only one cxx_eval_outermost_expr is called.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (2 preceding siblings ...)
  2021-04-01 14:15 ` jakub at gcc dot gnu.org
@ 2021-04-07 11:39 ` jakub at gcc dot gnu.org
  2021-04-07 12:05 ` ppalka at gcc dot gnu.org
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 11:39 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, on the #c3 testcase, if I put a breakpoint before and after
fold_nondependent_expr in finish_static_assert and temporarily in between those
two breakpoints put a breakpoint on cxx_eval_call_expression and
cxx_eval_increment_expression, it is clear that while
cxx_eval_call_expression is called on
...
intrusive_ptr<Foo>::~intrusive_ptr (&D.2255);
Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>);
intrusive_ptr<Foo>::~intrusive_ptr (&a);
Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>);

where during the first Foo::dec call evaluation
cxx_eval_increment_expression is called on --((struct Foo *) this)->count_
during the second Foo::dec call we use cached result (1) and don't evaluate the
body at all.  But the body of the constexpr method has side-effects on the
class it is called on (and the result is incorrect too), on the testcase when
not evaluating at compile time but runtime the first Foo::dec shall
predecrement count_ from 2 to 1 (thus return 1) and the second one should
predecrement count_ from 1 to 0 (thus return 0).

For the constexpr new I had to add the
          /* If the call allocated some heap object that hasn't been
             deallocated during the call, or if it deallocated some heap
             object it has not allocated, the call isn't really stateless
             for the constexpr evaluation and should not be cached.
             It is fine if the call allocates something and deallocates it
             too.  */
setting of cacheable to false in some cases, but this PR let's me wonder if it
is ever ok to cache constexpr results and what the standard actually says (if
constexpr/consteval functions can have side-effects besides computing a return
value (I believe they can't just change some unrelated constexpr variable,
right?) and ditto for constexpr/consteval methods.

So, e.g. shall we remember during the constexpr evaluation if the currently
evaluated constexpr function has side-effects to something other than its
automatic variables or return value and if so, make it effectively
non-cacheable?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (3 preceding siblings ...)
  2021-04-07 11:39 ` jakub at gcc dot gnu.org
@ 2021-04-07 12:05 ` ppalka at gcc dot gnu.org
  2021-04-07 12:14 ` jakub at gcc dot gnu.org
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-07 12:05 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> So, on the #c3 testcase, if I put a breakpoint before and after
> fold_nondependent_expr in finish_static_assert and temporarily in between
> those two breakpoints put a breakpoint on cxx_eval_call_expression and
> cxx_eval_increment_expression, it is clear that while
> cxx_eval_call_expression is called on
> ...
> intrusive_ptr<Foo>::~intrusive_ptr (&D.2255);
> Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>);
> intrusive_ptr<Foo>::~intrusive_ptr (&a);
> Foo::dec (NON_LVALUE_EXPR <((struct intrusive_ptr *) this)->ptr>);

Shouldn't cxx_bind_parameters_in_call have noticed that the argument to these
Foo::dec calls is non-constant, which would inhibit their caching?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (4 preceding siblings ...)
  2021-04-07 12:05 ` ppalka at gcc dot gnu.org
@ 2021-04-07 12:14 ` jakub at gcc dot gnu.org
  2021-04-07 12:15 ` ppalka at gcc dot gnu.org
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 12:14 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The argument is a pointer.
Now, I bet a pointer to an automatic variable will be seen as non-constant and
so in that case we might be ok.
If the argument is a pointer to some global constexpr variable, dunno.
And, on this testcase the argument is a heap pointer which during constexpr
evaluation we pretend it is constant.
So maybe cxx_bind_parameters_in_call should set *non_constant_args also when
arg is ADDR_EXPR of something with heap_*identifier* VAR_DECL as base?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (5 preceding siblings ...)
  2021-04-07 12:14 ` jakub at gcc dot gnu.org
@ 2021-04-07 12:15 ` ppalka at gcc dot gnu.org
  2021-04-07 12:24 ` jakub at gcc dot gnu.org
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-07 12:15 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Patrick Palka <ppalka at gcc dot gnu.org> ---
constexpr void foo(int* x) { ++*x; }
constexpr int bar() {
  int* x = new int(0);
  foo(x);
  foo(x);
  int y = *x;
  delete x;
  return y;
}
static_assert(bar() == 2);

We reject the above testcase for seemingly the same reason -- we cache the call
to foo(x) when we shouldn't have, because cxx_bind_parameters_in_call considers
the call's argument to be TREE_CONSTANT.


We accept the analogous testcase that doesn't use constexpr new/delete:

constexpr void foo(int* x) { ++*x; }
constexpr int bar() {
  int x = 0;
  foo(&x);
  foo(&x);
  return x;
}
static_assert(bar() == 2);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (6 preceding siblings ...)
  2021-04-07 12:15 ` ppalka at gcc dot gnu.org
@ 2021-04-07 12:24 ` jakub at gcc dot gnu.org
  2021-04-07 12:45 ` ppalka at gcc dot gnu.org
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 12:24 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Why does it work for:
constexpr int foo(int* x) { return ++*x; }
struct S { constexpr S() : a(0) { foo(&a); foo(&a); } int a; };
constexpr S s;
static_assert (s.a == 2);
though?  The argument to foo after constexpr processing it is &s.a which is
TREE_CONSTANT too, yet we don't cache the calls.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (7 preceding siblings ...)
  2021-04-07 12:24 ` jakub at gcc dot gnu.org
@ 2021-04-07 12:45 ` ppalka at gcc dot gnu.org
  2021-04-07 13:38 ` jakub at gcc dot gnu.org
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-07 12:45 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> The argument is a pointer.
> Now, I bet a pointer to an automatic variable will be seen as non-constant
> and so in that case we might be ok.
> If the argument is a pointer to some global constexpr variable, dunno.

FWIW I think this is PR80039.  I think we might have to worry about this case
only when evaluating the constructor of the global constexpr variable.

> Why does it work for:
> constexpr int foo(int* x) { return ++*x; }
> struct S { constexpr S() : a(0) { foo(&a); foo(&a); } int a; };
> constexpr S s;
> static_assert (s.a == 2);
> though?  The argument to foo after constexpr processing it is &s.a which is
> TREE_CONSTANT too, yet we don't cache the calls.

Hmm, seems because ctx->strict is not set when the initialization of s gets
evaluated in this case.   But it breaks if we instead do "constexpr S s = S();"

> And, on this testcase the argument is a heap pointer which during constexpr
> evaluation we pretend it is constant.
> So maybe cxx_bind_parameters_in_call should set *non_constant_args also when
> arg is ADDR_EXPR of something with heap_*identifier* VAR_DECL as base?

I think we'd also have to look inside TREE_CONSTANT CONSTRUCTORs to see if
they're storing a pointer to such a variable, so that we also suppress caching
for cases like:

constexpr int foo(auto x) { return ++*x.p; }
struct S {
  int a = 0;

  constexpr S() {
    struct R { int* p; };
    foo(R{&a});
    foo(R{&a});
  }
};
constexpr S s = S();
static_assert (s.a == 2);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (8 preceding siblings ...)
  2021-04-07 12:45 ` ppalka at gcc dot gnu.org
@ 2021-04-07 13:38 ` jakub at gcc dot gnu.org
  2021-04-07 13:44 ` jakub at gcc dot gnu.org
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 13:38 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So perhaps
--- gcc/cp/constexpr.c.jj       2021-03-19 18:36:49.165304923 +0100
+++ gcc/cp/constexpr.c  2021-04-07 15:33:31.993242067 +0200
@@ -1616,6 +1616,22 @@ cxx_bind_parameters_in_call (const const
            /* The destructor needs to see any modifications the callee makes
               to the argument.  */
            *non_constant_args = true;
+         else
+           {
+             tree addr = arg;
+             STRIP_NOPS (addr);
+             /* When a method or function is called with address of a heap
+                allocated variable, don't cache it so that any modifications
+                in it are performed.  */
+             if (TREE_CODE (addr) == ADDR_EXPR)
+               if (tree var = get_base_address (TREE_OPERAND (addr, 0)))
+                 if (VAR_P (var)
+                     && (DECL_NAME (var) == heap_uninit_identifier
+                         || DECL_NAME (var) == heap_identifier
+                         || DECL_NAME (var) == heap_vec_uninit_identifier
+                         || DECL_NAME (var) == heap_vec_identifier))
+                   *non_constant_args = true;
+           }

          /* For virtual calls, adjust the this argument, so that it is
             the object on which the method is called, rather than
for the heap vars and something similar for the global var ctors?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (9 preceding siblings ...)
  2021-04-07 13:38 ` jakub at gcc dot gnu.org
@ 2021-04-07 13:44 ` jakub at gcc dot gnu.org
  2021-04-07 13:48 ` jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 13:44 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For the global vars (so PR80039 too), can the problem be anything but when
cxx_eval_outermost_constant_expr is called on such an object (or part thereof)?
Unfortunately, ctx->object might be NULL, perhaps we could stick a copy of that
into ctx->global->object (get_base_address of the outermost object it is called
on)?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (10 preceding siblings ...)
  2021-04-07 13:44 ` jakub at gcc dot gnu.org
@ 2021-04-07 13:48 ` jakub at gcc dot gnu.org
  2021-04-07 14:04 ` jakub at gcc dot gnu.org
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 13:48 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And in #c9 you're right that it could be embedded in CONSTRUCTORs too.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (11 preceding siblings ...)
  2021-04-07 13:48 ` jakub at gcc dot gnu.org
@ 2021-04-07 14:04 ` jakub at gcc dot gnu.org
  2021-04-07 16:32 ` ppalka at gcc dot gnu.org
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 14:04 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #12)
> And in #c9 you're right that it could be embedded in CONSTRUCTORs too.

Wonder if cp_walk_tree &arg to find the ADDR_EXPR of heap var addresses and
ctx->global->object address would be enough.
Counter example I was thinking about would be:
constexpr int foo(auto x) { return ++*x->p; }
struct S {
  int a = 0;

  constexpr S() {
    struct R { int* p; };
    R b{&a};
    R c{&a};
    foo(&b);
    foo(&c);
  }
};
constexpr S s = S();
static_assert (s.a == 2);
but in that case we handle it correctly already as the address of the automatic
vars is already non-constant.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (12 preceding siblings ...)
  2021-04-07 14:04 ` jakub at gcc dot gnu.org
@ 2021-04-07 16:32 ` ppalka at gcc dot gnu.org
  2021-04-07 17:33 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-07 16:32 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #11)
> For the global vars (so PR80039 too), can the problem be anything but when
> cxx_eval_outermost_constant_expr is called on such an object (or part
> thereof)?
> Unfortunately, ctx->object might be NULL, perhaps we could stick a copy of
> that into ctx->global->object (get_base_address of the outermost object it
> is called on)?

Sounds good to me FWIW

(In reply to Jakub Jelinek from comment #13)
> (In reply to Jakub Jelinek from comment #12)
> > And in #c9 you're right that it could be embedded in CONSTRUCTORs too.
> 
> Wonder if cp_walk_tree &arg to find the ADDR_EXPR of heap var addresses and
> ctx->global->object address would be enough.

Sounds good to me too.  Using cp_walk_tree means we'd also detect the case
where the argument is 'heap_ptr + CST'.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (13 preceding siblings ...)
  2021-04-07 16:32 ` ppalka at gcc dot gnu.org
@ 2021-04-07 17:33 ` jakub at gcc dot gnu.org
  2021-04-08 15:19 ` cvs-commit at gcc dot gnu.org
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-07 17:33 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50525
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50525&action=edit
gcc11-pr99859.patch

Full untested patch.  As for PR80039, I for some reason can't reproduce the
#c0 in there and #c1 looks too close to the testcases from this PR that I think
it isn't useful to add it to the testsuite.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (14 preceding siblings ...)
  2021-04-07 17:33 ` jakub at gcc dot gnu.org
@ 2021-04-08 15:19 ` cvs-commit at gcc dot gnu.org
  2021-04-08 15:21 ` jakub at gcc dot gnu.org
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-08 15:19 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from CVS 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:559d2f1e0eafd96c19dc5324db1a466286c0e7fc

commit r11-8056-g559d2f1e0eafd96c19dc5324db1a466286c0e7fc
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Apr 8 17:15:39 2021 +0200

    c++: Don't cache constexpr functions which are passed pointers to heap or
static vars being constructed [PR99859]

    When cxx_bind_parameters_in_call is called e.g. on a method on an automatic
    variable, we evaluate the argument and because ADDR_EXPR of an automatic
    decl is not TREE_CONSTANT, we set *non_constant_args and don't cache it.
    But when it is called on an object located on the heap (allocated using
    C++20 constexpr new) where we represent it as TREE_STATIC artificial
    var, or when it is called on a static var that is currently being
    constructed, such ADDR_EXPRs are TREE_CONSTANT and we happily cache
    such calls, but they can in those cases have side-effects in the heap
    or static var objects and so caching them means such side-effects will
    happen only once and not as many times as that method or function is
called.
    Furthermore, as Patrick mentioned in the PR, the argument doesn't need to
be
    just ADDR_EXPR of the heap or static var or its components, but it could be
    a CONSTRUCTOR that has the ADDR_EXPR embedded anywhere.
    And the incorrectly cached function doesn't need to modify the pointed vars
    or their components, but some caller could be changing them in between the
    call that was cached and the call that used the cached result.

    The following patch fixes it by setting *non_constant_args also when
    the argument contains somewhere such an ADDR_EXPR, either of a heap
    artificial var or component thereof, or of a static var currently being
    constructed (where for that it uses the same check as
    cxx_eval_store_expression, ctx->global->values.get (...); addresses of
    other static variables would be rejected by cxx_eval_store_expression
    and therefore it is ok to cache such calls).

    2021-04-08  Jakub Jelinek  <jakub@redhat.com>

            PR c++/99859
            * constexpr.c (addr_of_non_const_var): New function.
            (cxx_bind_parameters_in_call): Set *non_constant_args to true
            even if cp_walk_tree on arg with addr_of_non_const_var callback
            returns true.

            * g++.dg/cpp1y/constexpr-99859-1.C: New test.
            * g++.dg/cpp1y/constexpr-99859-2.C: New test.
            * g++.dg/cpp2a/constexpr-new18.C: New test.
            * g++.dg/cpp2a/constexpr-new19.C: New test.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (15 preceding siblings ...)
  2021-04-08 15:19 ` cvs-commit at gcc dot gnu.org
@ 2021-04-08 15:21 ` jakub at gcc dot gnu.org
  2021-04-08 15:22 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-08 15:21 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk, I think we want to backport this though eventually.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (16 preceding siblings ...)
  2021-04-08 15:21 ` jakub at gcc dot gnu.org
@ 2021-04-08 15:22 ` jakub at gcc dot gnu.org
  2021-04-19  1:53 ` ppalka at gcc dot gnu.org
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-08 15:22 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vittorio.romeo at outlook dot com

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
*** Bug 80039 has been marked as a duplicate of this bug. ***

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (17 preceding siblings ...)
  2021-04-08 15:22 ` jakub at gcc dot gnu.org
@ 2021-04-19  1:53 ` ppalka at gcc dot gnu.org
  2021-04-20  9:45 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-19  1:53 UTC (permalink / raw)
  To: gcc-bugs

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pkeir at outlook dot com

--- Comment #19 from Patrick Palka <ppalka at gcc dot gnu.org> ---
*** Bug 96414 has been marked as a duplicate of this bug. ***

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (18 preceding siblings ...)
  2021-04-19  1:53 ` ppalka at gcc dot gnu.org
@ 2021-04-20  9:45 ` cvs-commit at gcc dot gnu.org
  2021-07-28 22:16 ` pinskia at gcc dot gnu.org
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-20  9:45 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

commit r10-9722-ge961da38630c892dfc0723e0726b6a0b0833e951
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Apr 8 17:15:39 2021 +0200

    c++: Don't cache constexpr functions which are passed pointers to heap or
static vars being constructed [PR99859]

    When cxx_bind_parameters_in_call is called e.g. on a method on an automatic
    variable, we evaluate the argument and because ADDR_EXPR of an automatic
    decl is not TREE_CONSTANT, we set *non_constant_args and don't cache it.
    But when it is called on an object located on the heap (allocated using
    C++20 constexpr new) where we represent it as TREE_STATIC artificial
    var, or when it is called on a static var that is currently being
    constructed, such ADDR_EXPRs are TREE_CONSTANT and we happily cache
    such calls, but they can in those cases have side-effects in the heap
    or static var objects and so caching them means such side-effects will
    happen only once and not as many times as that method or function is
called.
    Furthermore, as Patrick mentioned in the PR, the argument doesn't need to
be
    just ADDR_EXPR of the heap or static var or its components, but it could be
    a CONSTRUCTOR that has the ADDR_EXPR embedded anywhere.
    And the incorrectly cached function doesn't need to modify the pointed vars
    or their components, but some caller could be changing them in between the
    call that was cached and the call that used the cached result.

    The following patch fixes it by setting *non_constant_args also when
    the argument contains somewhere such an ADDR_EXPR, either of a heap
    artificial var or component thereof, or of a static var currently being
    constructed (where for that it uses the same check as
    cxx_eval_store_expression, ctx->global->values.get (...); addresses of
    other static variables would be rejected by cxx_eval_store_expression
    and therefore it is ok to cache such calls).

    2021-04-08  Jakub Jelinek  <jakub@redhat.com>

            PR c++/99859
            * constexpr.c (addr_of_non_const_var): New function.
            (cxx_bind_parameters_in_call): Set *non_constant_args to true
            even if cp_walk_tree on arg with addr_of_non_const_var callback
            returns true.

            * g++.dg/cpp1y/constexpr-99859-1.C: New test.
            * g++.dg/cpp1y/constexpr-99859-2.C: New test.
            * g++.dg/cpp2a/constexpr-new18.C: New test.
            * g++.dg/cpp2a/constexpr-new19.C: New test.

    (cherry picked from commit 559d2f1e0eafd96c19dc5324db1a466286c0e7fc)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (19 preceding siblings ...)
  2021-04-20  9:45 ` cvs-commit at gcc dot gnu.org
@ 2021-07-28 22:16 ` pinskia at gcc dot gnu.org
  2021-08-28 21:17 ` pinskia at gcc dot gnu.org
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 25+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-28 22:16 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #21 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks to be fixed in GCC 11.  It still fails for me with GCC 10.3 though.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (20 preceding siblings ...)
  2021-07-28 22:16 ` pinskia at gcc dot gnu.org
@ 2021-08-28 21:17 ` pinskia at gcc dot gnu.org
  2021-08-28 21:21 ` pinskia at gcc dot gnu.org
  2022-02-02 17:29 ` redi at gcc dot gnu.org
  23 siblings, 0 replies; 25+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-08-28 21:17 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (21 preceding siblings ...)
  2021-08-28 21:17 ` pinskia at gcc dot gnu.org
@ 2021-08-28 21:21 ` pinskia at gcc dot gnu.org
  2022-02-02 17:29 ` redi at gcc dot gnu.org
  23 siblings, 0 replies; 25+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-08-28 21:21 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #23 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Bug c++/99859] constexpr evaluation with member function is incorrect
  2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
                   ` (22 preceding siblings ...)
  2021-08-28 21:21 ` pinskia at gcc dot gnu.org
@ 2022-02-02 17:29 ` redi at gcc dot gnu.org
  23 siblings, 0 replies; 25+ messages in thread
From: redi at gcc dot gnu.org @ 2022-02-02 17:29 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #24 from Jonathan Wakely <redi at gcc dot gnu.org> ---
*** Bug 104348 has been marked as a duplicate of this bug. ***

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-02-02 17:29 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 22:54 [Bug c++/99859] New: constexpr evaluation with member function is incorrect ldalessandro at gmail dot com
2021-03-31 23:15 ` [Bug c++/99859] " ldalessandro at gmail dot com
2021-04-01 10:29 ` jakub at gcc dot gnu.org
2021-04-01 14:15 ` jakub at gcc dot gnu.org
2021-04-07 11:39 ` jakub at gcc dot gnu.org
2021-04-07 12:05 ` ppalka at gcc dot gnu.org
2021-04-07 12:14 ` jakub at gcc dot gnu.org
2021-04-07 12:15 ` ppalka at gcc dot gnu.org
2021-04-07 12:24 ` jakub at gcc dot gnu.org
2021-04-07 12:45 ` ppalka at gcc dot gnu.org
2021-04-07 13:38 ` jakub at gcc dot gnu.org
2021-04-07 13:44 ` jakub at gcc dot gnu.org
2021-04-07 13:48 ` jakub at gcc dot gnu.org
2021-04-07 14:04 ` jakub at gcc dot gnu.org
2021-04-07 16:32 ` ppalka at gcc dot gnu.org
2021-04-07 17:33 ` jakub at gcc dot gnu.org
2021-04-08 15:19 ` cvs-commit at gcc dot gnu.org
2021-04-08 15:21 ` jakub at gcc dot gnu.org
2021-04-08 15:22 ` jakub at gcc dot gnu.org
2021-04-19  1:53 ` ppalka at gcc dot gnu.org
2021-04-20  9:45 ` cvs-commit at gcc dot gnu.org
2021-07-28 22:16 ` pinskia at gcc dot gnu.org
2021-08-28 21:17 ` pinskia at gcc dot gnu.org
2021-08-28 21:21 ` pinskia at gcc dot gnu.org
2022-02-02 17:29 ` redi at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).