On Thu, 3 Jul 2025, Jason Merrill wrote:
> On 7/2/25 7:58 PM, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > for trunk?
> >
> > -- >8 --
> >
> > Here the flag -fno-delete-null-pointer-checks causes the trivial address
> > comparison in
> >
> > inline int a, b;
> > static_assert(&a != &b);
> >
> > to be rejected as non-constant because with the flag we can't assume
> > such weak symbols are non-NULL, which causes symtab/fold-const.cc to
> > punt on such comparisons. Note this also affects -fsanitize=undefined
> > since it implies -fno-delete-null-pointer-checks.
>
> Right, the underlying problem is that we use the one flag to mean two things:
>
> 1) a static storage duration decl can live at address 0
> 2) do more careful checking for null pointers/lvalues (i.e. in
> gimple_call_nonnull_result_p)
>
> Both cases are related to checking for null, but they are different situations
> and really shouldn't depend on the same flag.
>
> Your patch seems wrong for #1 targets; on such a target 'a' might end up
> allocated at address 0, so "&a != nullptr" is not decidable at compile time.
>
> OTOH such targets are a small minority, and I suspect they already have other
> C++ issues with e.g. a conversion to base not adjusting a null pointer.
Yep, and normally I would not be so bold to propose making such a
trade-off, but this seems to be exactly the trade-off we made in
PR96862 for -frounding-math? The flag makes lossy floating-point
operations depend on the run-time rounding mode and so not decidable at
compile time, so we ended up disabling the flag during constexpr
evaluation and using the default rounding mode. I don't immediately
see why -frounding-math maybe be special.
On a related note, I notice we accept
[[gnu::weak]] inline int a, b;
static_assert(&a != &b);
with the default -fdelete-null-pointer-checks, which seems wrong,
we probably should reject address comparisons of weak symbols as
non-constant by default.
>
> Jakub, what do you think? It's been 9 years since you proposed a better fix
> in the PR, but that hasn't happened yet.
>
> > This issue seems conceptually the same as PR96862 which was about
> > -frounding-math breaking some constexpr floating point arithmetic,
> > and we fixed that PR by disabling -frounding-math during manifestly
> > constant evaluation. This patch proposes to do the same for
> > -fno-delete-null-pointer-checks, disabling it during maniestly constant
> > evaluation. I opted to disable it narrowly around the relevant
> > fold_binary call which seems to address all reported constexpr failures,
> > but we could consider it disabling it more broadly as well.
> >
> > PR c++/71962
> >
> > gcc/cp/ChangeLog:
> >
> > * constexpr.cc (cxx_eval_binary_expression): Set
> > flag_delete_null_pointer_checks alongside folding_cxx_constexpr
> > during manifestly constant evaluation.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/ext/constexpr-pr71962.C: New test.
> > * g++.dg/ubsan/pr71962.C: New test.
> > ---
> > gcc/cp/constexpr.cc | 2 ++
> > gcc/testsuite/g++.dg/ext/constexpr-pr71962.C | 18 ++++++++++++++++++
> > gcc/testsuite/g++.dg/ubsan/pr71962.C | 5 +++++
> > 3 files changed, 25 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/ext/constexpr-pr71962.C
> > create mode 100644 gcc/testsuite/g++.dg/ubsan/pr71962.C
> >
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 704d936f2ec3..e8426b40c543 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -4068,6 +4068,8 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx,
> > tree t,
> > || TREE_CODE (type) != REAL_TYPE))
> > {
> > auto ofcc = make_temp_override (folding_cxx_constexpr, true);
> > + auto odnpc = make_temp_override (flag_delete_null_pointer_checks,
> > + true);
> > r = fold_binary_initializer_loc (loc, code, type, lhs, rhs);
> > }
> > else
> > diff --git a/gcc/testsuite/g++.dg/ext/constexpr-pr71962.C
> > b/gcc/testsuite/g++.dg/ext/constexpr-pr71962.C
> > new file mode 100644
> > index 000000000000..57cb14ac804e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/constexpr-pr71962.C
> > @@ -0,0 +1,18 @@
> > +// PR c++/71962
> > +// { dg-do compile { target c++11 } }
> > +// { dg-additional-options "-fno-delete-null-pointer-checks" }
> > +
> > +struct A { void f(); };
> > +static_assert(&A::f != nullptr, "");
> > +
> > +#if __cpp_inline_variables
> > +inline int a, b;
> > +static_assert(&a != &b, "");
> > +static_assert(&a != nullptr, "");
> > +#endif
> > +
> > +int main() {
> > + static int x, y;
> > + static_assert(&x != &y, "");
> > + static_assert(&x != nullptr, "");
> > +}
> > diff --git a/gcc/testsuite/g++.dg/ubsan/pr71962.C
> > b/gcc/testsuite/g++.dg/ubsan/pr71962.C
> > new file mode 100644
> > index 000000000000..f17c825da449
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ubsan/pr71962.C
> > @@ -0,0 +1,5 @@
> > +// PR c++/71962
> > +// { dg-do compile { target c++11 } }
> > +// { dg-additional-options "-fsanitize=undefined" }
> > +
> > +#include "../ext/constexpr-pr71962.C"
>
>
