[Bug c++/106057] New: Missed stmt_can_throw_external check in stmt_kills_ref_p

[Bug c++/106057] New: Missed stmt_can_throw_external check in stmt_kills_ref_p

[hubicka at gcc dot gnu.org]

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

            Bug ID: 106057
           Summary: Missed stmt_can_throw_external check in
                    stmt_kills_ref_p
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hubicka at gcc dot gnu.org
  Target Milestone: ---

int a;
int b;

__attribute__((const))
int maybethrow()
{
        if (!b)
                throw(0);
        return 0;
}

void
test()
{
        a=1;
        a=maybethrow();
        a=0;
}
int
main()
{
        try {
                test();
        }
        catch(int) {
                if (!a)
                        __builtin_abort ();
        }
        return 0;
}

aborts when built with -O2 while I think it should not.  The bug is that
 a=maybethrow()
is considered a kill which it is not since exception happens earlier.

I think we need:
diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index b1e7a2d5afc..7307e386f96 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -3337,7 +3338,9 @@ stmt_kills_ref_p (gimple *stmt, ao_ref *ref)
         ???  We only need to care about the RHS throwing.  For aggregate
         assignments or similar calls and non-call exceptions the LHS
         might throw as well.  */
-      && !stmt_can_throw_internal (cfun, stmt))
+      && !stmt_can_throw_internal (cfun, stmt)
+      && (!stmt_can_throw_external (cfun, stmt)
+         || !ref_may_alias_global_p (ref, false)))
     {
       tree lhs = gimple_get_lhs (stmt);
       /* If LHS is literally a base of the access we are done.  */

[Bug c++/106057] Missed stmt_can_throw_external check in stmt_kills_ref_p

[hubicka at gcc dot gnu.org]

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

--- Comment #1 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
C only testcase (also misoptimized in clang)

#include <setjmp.h>
int b;
jmp_buf buf;

__attribute__((noinline))
int maybethrow()
{
        if (!b)
                longjmp (buf,1);
        return 2;
}

void
test(int *a)
{
        *a=1;
        *a=maybethrow();
}
int
main()
{
        int a=-1;
        if (setjmp (buf) == 1)
        {
                if (a!=1)
                        __builtin_abort ();
                        return 0;
        }
        test (&a);
        return 0;
}

[Bug c++/106057] Missed stmt_can_throw_external check in stmt_kills_ref_p

[hubicka at gcc dot gnu.org]

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

--- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
The second testcase (with longjmp) invalid since longjmp can clobber automatic
variable and making the variable static breaks the testcase since we believe
htat longjmp reads global memory state (it doesn't).

The first testcase is also kind-of bordeline since it is question whether
const/pure can throw.

Performance wise we probably do not need to care about longjmp but with
cxa_throw we may want to get side effects modeled right...

[Bug c++/106057] Missed stmt_can_throw_external check in stmt_kills_ref_p

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jan Hubicka <hubicka@gcc.gnu.org>:

https://gcc.gnu.org/g:7fd34782b95bbe1b4dc9936b8923f86d4aaee379

commit r13-1241-g7fd34782b95bbe1b4dc9936b8923f86d4aaee379
Author: Jan Hubicka <jh@suse.cz>
Date:   Fri Jun 24 13:52:44 2022 +0200

    Fix stmt_kills_ref_p WRT external throws

    Add missing check to stmt_kills_ref_p for case that function
    is terminated by EH before call return value kills the ref. In the PR
    I tried to construct testcase but I don't know how to do that until I
    annotate EH code with fnspec attributes which I will do in separate patch
    and add a testcase.

            PR ipa/106057
            * tree-ssa-alias.cc (stmt_kills_ref_p): Check for external throw.

[Bug c++/106057] Missed stmt_can_throw_external check in stmt_kills_ref_p

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:0f2c7ccd14a29a8af8318f50b8296098fb0ab218

commit r13-2034-g0f2c7ccd14a29a8af8318f50b8296098fb0ab218
Author: Jan Hubicka <jh@suse.cz>
Date:   Fri Aug 12 16:25:28 2022 +0200

    Fix invalid devirtualization when combining final keyword and anonymous
types

    this patch fixes a wrong code issue where we incorrectly devirtualize to
    __builtin_unreachable.  The problem occurs in combination of anonymous
    namespaces and final keyword used on methods.  We do two optimizations here
     1) when reacing final method we cut the search for possible new targets
     2) if the type is anonymous we detect whether it is ever instatiated by
        looking if its vtable is referred to.
    Now this goes wrong when thre is an anonymous type with final method that
    is not instantiated while its derived type is.  So if 1 triggers we need
    to make 2 to look for vtables of all derived types as done by this patch.

    Bootstrpaped/regtested x86_64-linux

    Honza

    gcc/ChangeLog:

    2022-08-10  Jan Hubicka  <hubicka@ucw.cz>

            PR middle-end/106057
            * ipa-devirt.cc (type_or_derived_type_possibly_instantiated_p): New
            function.
            (possible_polymorphic_call_targets): Use it.

    gcc/testsuite/ChangeLog:

    2022-08-10  Jan Hubicka  <hubicka@ucw.cz>

            PR middle-end/106057
            * g++.dg/tree-ssa/pr101839.C: New test.

[Bug c++/106057] Missed stmt_can_throw_external check in stmt_kills_ref_p

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

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

commit r12-9478-gea162107bb376f5ffa18bbda70e14b47bc338070
Author: Jan Hubicka <jh@suse.cz>
Date:   Fri Aug 12 16:25:28 2022 +0200

    Fix invalid devirtualization when combining final keyword and anonymous
types

    this patch fixes a wrong code issue where we incorrectly devirtualize to
    __builtin_unreachable.  The problem occurs in combination of anonymous
    namespaces and final keyword used on methods.  We do two optimizations here
     1) when reacing final method we cut the search for possible new targets
     2) if the type is anonymous we detect whether it is ever instatiated by
        looking if its vtable is referred to.
    Now this goes wrong when thre is an anonymous type with final method that
    is not instantiated while its derived type is.  So if 1 triggers we need
    to make 2 to look for vtables of all derived types as done by this patch.

    Bootstrpaped/regtested x86_64-linux

    Honza

    gcc/ChangeLog:

    2022-08-10  Jan Hubicka  <hubicka@ucw.cz>

            PR middle-end/106057
            * ipa-devirt.cc (type_or_derived_type_possibly_instantiated_p): New
            function.
            (possible_polymorphic_call_targets): Use it.

    gcc/testsuite/ChangeLog:

    2022-08-10  Jan Hubicka  <hubicka@ucw.cz>

            PR middle-end/106057
            * g++.dg/tree-ssa/pr101839.C: New test.

    (cherry picked from commit 0f2c7ccd14a29a8af8318f50b8296098fb0ab218)