[Bug c++/97553] New: [missed optimization] constexprness not noticed when UBsan enabled

[Bug c++/97553] New: [missed optimization] constexprness not noticed when UBsan enabled

[eyalroz at technion dot ac.il]

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

            Bug ID: 97553
           Summary: [missed optimization] constexprness not noticed when
                    UBsan enabled
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: eyalroz at technion dot ac.il
  Target Milestone: ---

(GodBolt example: https://godbolt.org/z/Kvan5c)

Consider the following code:

  #include <string_view>

  constexpr std::string_view f() { return "hello"; }

  static constexpr std::string_view g() {
      auto x { f() };
      return x.substr(1, 3);
  } 

  int foo() { return g().length(); }

if you compile it with flags `--std=c++17 -O3`, it results in a pleasant:

  foo():
          mov     eax, 3
          ret

but if you also enabled undefined-behavior sanitization, i.e. `--std=c++17
-fsanitize=undefined -O3`, then you get a much longer program with UB-related
instrumentation - which is never used.

I'm not sure if it's because some optimizations are disabled with UBsan, in
which case this might be a "misfeature", or whether they're enabled but the
optimization is just missed.

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Whether the function is constexpr or not doesn't really matter when you
evaluate it in non-constant expression contexts.  In those the ubsan
instrumentation is bypassed (the constant expression evaluation does similar
checking), but otherwise it is a normal function like any other, which
including the instrumentation is inlined etc.  And, the runtime sanitization
intentionally isn't heavily optimized away, because the intent is to detect
when the code is invalid, so it can't e.g. optimize away those checks based on
assumption that undefined behavior will not happen.
If you want a constant via C++ means, use int foo() { constexpr int x =
g().length(); return x; }

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[mpolacek at gcc dot gnu.org]

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

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

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

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Looks like INVALID.

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[eyalroz at technion dot ac.il]

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

--- Comment #3 from Eyal Rozenberg <eyalroz at technion dot ac.il> ---
> And, the runtime sanitization intentionally isn't heavily optimized away, because the intent is to detect when the code is invalid, so it can't e.g. optimize away those checks based on assumption that undefined behavior will not happen.

So, doesn't that essentially mean that -O3 cannot properly apply with UBsan?

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Depends on what you mean by properly.  -O3 can be used with sanitization, but
expecting the code to be optimized the same way as without sanitization is
wrong, it is more important to catch as many bugs as possible, and the runtime
instrumentation slows things down anyway.  The sanitization is not meant to be
used for production code, only when debugging it.

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[eyalroz at technion dot ac.il]

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

--- Comment #5 from Eyal Rozenberg <eyalroz at technion dot ac.il> ---
(In reply to Jakub Jelinek from comment #4)
> Depends on what you mean by properly.  -O3 can be used with sanitization,
> but expecting the code to be optimized the same way as without sanitization
> is wrong, it is more important to catch as many bugs as possible, and the
> runtime instrumentation slows things down anyway.  The sanitization is not
> meant to be used for production code, only when debugging it.

I wonder, then, if some kind of notice isn't called for when -O3 and UBsan are
used together.

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:5fea1be820508e1fbc610d1a54b61c1add33c36f

commit r13-6120-g5fea1be820508e1fbc610d1a54b61c1add33c36f
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Feb 17 15:18:10 2023 -0500

    c++: speculative constexpr and is_constant_evaluated [PR108243]

    This PR illustrates that __builtin_is_constant_evaluated currently acts
    as an optimization barrier for our speculative constexpr evaluation,
    since we don't want to prematurely fold the builtin to false before the
    expression in question undergoes manifestly constant evaluation if
    appropriate (in which case the builtin must instead be folded to true).

    This patch fixes this by permitting __builtin_is_constant_evaluated to
    get folded as false at appropiate points, namely during cp_fold_function
    and cp_fully_fold_init where we know we're done with manifestly constant
    evaluation.  The function cp_fold gets a flags parameter that controls
    whether we pass mce_false or mce_unknown to maybe_constant_value when
    folding a CALL_EXPR.

            PR c++/108243
            PR c++/97553

    gcc/cp/ChangeLog:

            * cp-gimplify.cc (enum fold_flags): Define.
            (fold_flags_t): Declare.
            (cp_fold_data::genericize): Replace this data member with ...
            (cp_fold_data::fold_flags): ... this.
            (cp_fold_r): Adjust use of cp_fold_data and calls to cp_fold.
            (cp_fold_function): Likewise.
            (cp_fold_maybe_rvalue): Add an internal overload that
            additionally takes and propagates a fold_flags_t parameter, and
            define the existing public overload in terms of it.
            (cp_fold_rvalue): Likewise.
            (cp_fully_fold_init): Adjust use of cp_fold_data.
            (fold_cache): Replace with ...
            (fold_caches): ... this 2-element array of caches.
            (get_fold_cache): Define.
            (clear_fold_cache): Adjust.
            (cp_fold): Add fold_flags_t parameter.  Use get_fold_cache.
            Pass flags to calls to cp_fold, cp_fold_rvalue and
            cp_fold_maybe_rvalue.
            <case CALL_EXPR>: If ff_mce_false is set, fold
            __builtin_is_constant_evaluated to false and pass mce_false to
            maybe_constant_value.

    gcc/testsuite/ChangeLog:

            * g++.dg/opt/is_constant_evaluated1.C: New test.
            * g++.dg/opt/is_constant_evaluated2.C: New test.

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:5425159d176a7a92afc932cbb22d8822667099c4

commit r13-6422-g5425159d176a7a92afc932cbb22d8822667099c4
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Mar 2 14:04:50 2023 -0500

    c++: more mce_false folding from cp_fully_fold_init [PR108243]

    We should also fold the overall initializer passed to cp_fully_fold_init
    with mce_false, which allows folding of the copy-initialization of
    'a1' in the below testcase (the initializer here is an AGGR_INIT_EXPR).

    Unfortunately this doesn't help with direct- or default-initialization
    because we don't call cp_fully_fold_init in that case, and even if we
    did the initializer in that case is expressed as a bare CALL_EXPR
    instead of an AGGR_INIT_EXPR, which cp_fully_fold_init can't really
    fold.

            PR c++/108243
            PR c++/97553

    gcc/cp/ChangeLog:

            * cp-gimplify.cc (cp_fully_fold): Add an internal overload that
            additionally takes and propagate an mce_value parameter, and
            define the existing public overload in terms of it.
            (cp_fully_fold_init): Pass mce_false to cp_fully_fold.

    gcc/testsuite/ChangeLog:

            * g++.dg/opt/is_constant_evaluated3.C: New test.

[Bug c++/97553] [missed optimization] constexprness not noticed when UBsan enabled

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
                 CC|                            |ppalka at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED
   Target Milestone|---                         |13.0
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I think we can call this pretty much fixed for GCC 13.