[Bug c/107769] New: -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata

[Bug c/107769] New: -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata

[resence at parsoma dot net]

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

            Bug ID: 107769
           Summary: -flto with -Os/-O2/-O3 emitted code with gcc 12.x
                    segfaults via mutated global in .rodata
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: resence at parsoma dot net
  Target Milestone: ---

Created attachment 53931
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53931&action=edit
Small, preprocessed testcase to reproduce this issue via -save-temps

On a Debian unstable amd64 system running "gcc (Debian 12.2.0-9) 12.2.0"
running any of:

gcc -flto -Wall -Wextra -pedantic -fopt-info -finline-limit=150 -Os
-fno-strict-aliasing -o r.OsNoG repro.c
gcc -flto -Wall -g -Wextra -pedantic -finline-limit=150 -Os
-fno-strict-aliasing -o r.Os repro.c
gcc -flto -Wall -Wextra -pedantic -fopt-info -finline-limit=150 -O2
-fno-strict-aliasing -o r.O2NoG repro.c
gcc -flto -Wall -g -Wextra -pedantic -finline-limit=150 -O2
-fno-strict-aliasing -o r.O2 repro.c
gcc -flto -Wall -Wextra -pedantic -finline-limit=150 -O3 -fno-strict-aliasing
-o r.O3noG repro.c
gcc -flto -Wall -Wextra -g -pedantic -finline-limit=150 -O3
-fno-strict-aliasing  -o r.O3 repro.c

on the attached repro.c test case results in

repro.c: In function ‘incRef__system_5450’:
repro.c:12:39: warning: unused parameter ‘c’ [-Wunused-parameter]
   12 | static void incRef__system_5450(void* c) {
      |                                 ~~~~~~^
repro.c: In function ‘isOnStack__system_5438’:
repro.c:15:74: warning: unused parameter ‘p’ [-Wunused-parameter]
   15 | unsigned char __attribute__((__noinline__)) isOnStack__system_5438(void
*p) {
      |                                                                   
~~~~~~^

and a binary which segfaults:

$ for i in r.O*; do echo $i; ./$i; done
r.O2
Segmentation fault
r.O2NoG
Segmentation fault
r.O3
Segmentation fault
r.O3noG
Segmentation fault
r.Os
Segmentation fault
r.OsNoG
Segmentation fault

Running valgrind results in:
==1048150== Memcheck, a memory error detector
==1048150== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==1048150== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==1048150== Command: ./r.Os
==1048150== 
==1048150== 
==1048150== Process terminating with default action of signal 11 (SIGSEGV)
==1048150==  Bad permissions for mapped region at address 0x10A008
==1048150==    at 0x109098: UnknownInlinedFun (repro.c:24)
==1048150==    by 0x109098: UnknownInlinedFun (repro.c:45)
==1048150==    by 0x109098: main (repro.c:50)

(with similar output for other optimization settings, less for settings without
debug information)

The proximate cause of this is that the global variable g__r_1 has been placed
in the .rodata section:
(gdb) p &g__r_1 
$2 = (void **) 0x555555556008 <g.r_1>
(gdb) info file
...
        0x0000555555556000 - 0x0000555555556010 is .rodata
...

This seems to only happen with a combination of all of (a) -flto (even with one
source file), (b) some greater-than-O1 level optimization level, (c) absence of
-fno-inline-small-functions option; and (d) gcc 12.x or newer.

Running gcc UBSAN on this reveals nothing obviously additional:
$ rm -v a.out && gcc -Og -flto -finline-limit=150 -fsanitize=undefined -Wall
-Wextra -pedantic -fno-strict-aliasing repro.c && ./a.out && echo gcc UBSAN
complete
removed 'a.out'
repro.c: In function ‘incRef__system_5450’:
repro.c:12:39: warning: unused parameter ‘c’ [-Wunused-parameter]
   12 | static void incRef__system_5450(void* c) {
      |                                 ~~~~~~^
repro.c: In function ‘isOnStack__system_5438’:
repro.c:15:74: warning: unused parameter ‘p’ [-Wunused-parameter]
   15 | unsigned char __attribute__((__noinline__)) isOnStack__system_5438(void
*p) {
      |                                                                   
~~~~~~^
gcc UBSAN complete

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|-flto with -Os/-O2/-O3      |[12/13 Regression] -flto
                   |emitted code with gcc 12.x  |with -Os/-O2/-O3 emitted
                   |segfaults via mutated       |code with gcc 12.x
                   |global in .rodata           |segfaults via mutated
                   |                            |global in .rodata
     Ever confirmed|0                           |1
      Known to fail|                            |12.1.0, 13.0
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |12.3
   Last reconfirmed|                            |2022-11-20
      Known to work|                            |11.1.0

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, reduced further, removing malloc and changing srand into a function
inside the TU (with noipa attribute):
[[gnu::noipa]]
void hjj (unsigned int lk)
{
    (void)lk;
}
void nn(int i, int n);
[[gnu::noinline]]
int ll(void) {
    return 1;
}
void hh(int* dest, int src)
{
    if (!ll() && !src)
        hjj(100);
    (*dest) = 1;
}
void gg(int* result, int x)
{
    if (x >= 0)
        return;

    int xx;
    xx = *result;
    hh(result, ll());
    if (xx >= *result)
        nn(xx, *result);
}
void nn(int i, int n) {
    int T8_;
    if (n < 0)
        __builtin_exit(0);
    T8_ = 0;
    gg(&T8_, i);
    __builtin_exit(0);
}
void kk(int* x, int i) {
    hh(x, ll());
    if (i < 0 || i >= *x)
        nn(i,*x);
}
int g__r_1 = 0;
int main() {
    kk(&g__r_1, 0);
    return 0;
}

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13 Regression] -flto    |[12/13 Regression] -flto
                   |with -Os/-O2/-O3 emitted    |with -Os/-O2/-O3 emitted
                   |code with gcc 12.x          |code with gcc 12.x
                   |segfaults via mutated       |segfaults via mutated
                   |global in .rodata           |global in .rodata since
                   |                            |r12-2887-ga6da2cddcf0e959d
                 CC|                            |hubicka at gcc dot gnu.org

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-2887-ga6da2cddcf0e959d.

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

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

--- Comment #3 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Looks like ipa-prop loses track of uses of the variable.  I will look into it
more.

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|hubicka at gcc dot gnu.org         |jamborm at gcc dot gnu.org

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Indeed, I hope I know what is going on.

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[yinyuefengyi at gmail dot com]

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

Xionghu Luo (luoxhu at gcc dot gnu.org) <yinyuefengyi at gmail dot com> changed:

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

--- Comment #5 from Xionghu Luo (luoxhu at gcc dot gnu.org) <yinyuefengyi at gmail dot com> ---
For case c#1:
g__r_1 is a global variable changed in function hh, but ipa-prop think it is
only loaded by reference without change then removed references in
gcc/ipa-prop.cc:propagate_controlled_uses.


.wpa.081i.cp:

g__r_1/6 (g__r_1)
  Type: variable definition analyzed
  Visibility: semantic_interposition prevailing_def_ironly
  References:
  Referring: main/7 (addr) kk.constprop.0/16 (addr) kk.part.0.constprop.0/17
(read)
  Read from file: /tmp/cc3peQfe.o
  Availability: available
  Varpool flags: initialized


.wpa.085i.inline:
ipa-prop: Address IPA constant will reach a load so adding LOAD reference from
main/7 to g__r_1/6.
ipa-prop: Removed a reference from main/7 to g__r_1/6.
ipa-prop: Removing cloning-created reference from kk.constprop/16 to g__r_1/6.
...
g__r_1/6 (g__r_1)
  Type: variable definition analyzed
  Visibility: semantic_interposition prevailing_def_ironly
  References:
  Referring: main/7 (read) main/7 (read) kk.part.0.constprop.0/17 (read)
  Read from file: /tmp/cc3peQfe.o
  Availability: available
  Varpool flags: initialized


It seems a bug exposed by r12-2887-ga6da2cddcf0e959d, but maybe actually caused
by r12-2523-g13586172d0b70c since it fail to identify globals not read-only...

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[jamborm at gcc dot gnu.org]

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

--- Comment #6 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Yes, you identified the correct commit.  The same jump function is double
counted (once during iPA-CP and then again during inlining) when we drop
references and so an address reference is replaced with a read one.  I will
make this bug a priority next week.

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[jamborm at gcc dot gnu.org]

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

--- Comment #7 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 54817
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54817&action=edit
potential patch

I am testing the attached patch.  I'd like to think about the whole situation a
bit more next week, but this seems like a way to fix this and PR 109318.

[Bug ipa/107769] [12/13 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:8e08c7886eed5824bebd0e011526ec302d622844

commit r13-7206-g8e08c7886eed5824bebd0e011526ec302d622844
Author: Martin Jambor <mjambor@suse.cz>
Date:   Mon Apr 17 12:59:51 2023 +0200

    ipa: Fix double reference-count decrements for the same edge (PR 107769, PR
109318)

    It turns out that since addition of the code that can identify globals
    which are only read from, the code that keeps track of the references
    can decrement their count for the same calls, once during IPA-CP and
    then again during inlining.  Fixed by adding a special flag to the
    pass-through variant and simply wiping out the reference to the
    refdesc structure from the constant ones.

    Moreover, during debugging of the issue I have discovered that the
    code removing references could remove a reference associated with the
    same statement but of a wrong type.  In all cases it wanted to remove
    an IPA_REF_ADDR reference so removing a lesser one instead should do
    no harm in practice, but we should try to be consistent and so this
    patch extends symtab_node::find_reference so that it searches for a
    reference of a given type only.

    gcc/ChangeLog:

    2023-04-14  Martin Jambor  <mjambor@suse.cz>

            PR ipa/107769
            PR ipa/109318
            * cgraph.h (symtab_node::find_reference): Add parameter use_type.
            * ipa-prop.h (ipa_pass_through_data): New flag refdesc_decremented.
            (ipa_zap_jf_refdesc): New function.
            (ipa_get_jf_pass_through_refdesc_decremented): Likewise.
            (ipa_set_jf_pass_through_refdesc_decremented): Likewise.
            * ipa-cp.cc (ipcp_discover_new_direct_edges): Provide a value for
            the new parameter of find_reference.
            (adjust_references_in_caller): Likewise. Make sure the constant
jump
            function is not used to decrement a refdec counter again.  Only
            decrement refdesc counters when the pass_through jump function
allows
            it.  Added a detailed dump when decrementing refdesc counters.
            * ipa-prop.cc (ipa_print_node_jump_functions_for_edge): Dump new
flag.
            (ipa_set_jf_simple_pass_through): Initialize the new flag.
            (ipa_set_jf_unary_pass_through): Likewise.
            (ipa_set_jf_arith_pass_through): Likewise.
            (remove_described_reference): Provide a value for the new parameter
of
            find_reference.
            (update_jump_functions_after_inlining): Zap refdesc of new jfunc if
            the previous pass_through had a flag mandating that we do so.
            (propagate_controlled_uses): Likewise.  Only decrement refdesc
            counters when the pass_through jump function allows it.
            (ipa_edge_args_sum_t::duplicate): Provide a value for the new
            parameter of find_reference.
            (ipa_write_jump_function): Assert the new flag does not have to be
            streamed.
            * symtab.cc (symtab_node::find_reference): Add parameter use_type,
use
            it in searching.

    gcc/testsuite/ChangeLog:

    2023-04-06  Martin Jambor  <mjambor@suse.cz>

            PR ipa/107769
            PR ipa/109318
            * gcc.dg/ipa/pr109318.c: New test.
            * gcc.dg/lto/pr107769_0.c: Likewise.

[Bug ipa/107769] [12 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13/14 Regression] -flto |[12 Regression] -flto with
                   |with -Os/-O2/-O3 emitted    |-Os/-O2/-O3 emitted code
                   |code with gcc 12.x          |with gcc 12.x segfaults via
                   |segfaults via mutated       |mutated global in .rodata
                   |global in .rodata since     |since
                   |r12-2887-ga6da2cddcf0e959d  |r12-2887-ga6da2cddcf0e959d

--- Comment #9 from Martin Jambor <jamborm at gcc dot gnu.org> ---
This has been fixed on master just in time to get into gcc 13 RC.  Backport to
gcc-12 will follow.

[Bug ipa/107769] [12 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Martin Jambor
<jamborm@gcc.gnu.org>:

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

commit r12-9476-gbea3885200c549419567ad3a43ac71642619ad1a
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Apr 26 18:38:39 2023 +0200

    ipa: Fix double reference-count decrements for the same edge (PR 107769, PR
109318)

    It turns out that since addition of the code that can identify globals
    which are only read from, the code that keeps track of the references
    can decrement their count for the same calls, once during IPA-CP and
    then again during inlining.  Fixed by adding a special flag to the
    pass-through variant and simply wiping out the reference to the
    refdesc structure from the constant ones.

    Moreover, during debugging of the issue I have discovered that the
    code removing references could remove a reference associated with the
    same statement but of a wrong type.  In all cases it wanted to remove
    an IPA_REF_ADDR reference so removing a lesser one instead should do
    no harm in practice, but we should try to be consistent and so this
    patch extends symtab_node::find_reference so that it searches for a
    reference of a given type only.

    gcc/ChangeLog:

    2023-04-14  Martin Jambor  <mjambor@suse.cz>

            PR ipa/107769
            PR ipa/109318
            * cgraph.h (symtab_node::find_reference): Add parameter use_type.
            * ipa-prop.h (ipa_pass_through_data): New flag refdesc_decremented.
            (ipa_zap_jf_refdesc): New function.
            (ipa_get_jf_pass_through_refdesc_decremented): Likewise.
            (ipa_set_jf_pass_through_refdesc_decremented): Likewise.
            * ipa-cp.cc (ipcp_discover_new_direct_edges): Provide a value for
            the new parameter of find_reference.
            (adjust_references_in_caller): Likewise. Make sure the constant
jump
            function is not used to decrement a refdec counter again.  Only
            decrement refdesc counters when the pass_through jump function
allows
            it.  Added a detailed dump when decrementing refdesc counters.
            * ipa-prop.cc (ipa_print_node_jump_functions_for_edge): Dump new
flag.
            (ipa_set_jf_simple_pass_through): Initialize the new flag.
            (ipa_set_jf_unary_pass_through): Likewise.
            (ipa_set_jf_arith_pass_through): Likewise.
            (remove_described_reference): Provide a value for the new parameter
of
            find_reference.
            (update_jump_functions_after_inlining): Zap refdesc of new jfunc if
            the previous pass_through had a flag mandating that we do so.
            (propagate_controlled_uses): Likewise.  Only decrement refdesc
            counters when the pass_through jump function allows it.
            (ipa_edge_args_sum_t::duplicate): Provide a value for the new
            parameter of find_reference.
            (ipa_write_jump_function): Assert the new flag does not have to be
            streamed.
            * symtab.cc (symtab_node::find_reference): Add parameter use_type,
use
            it in searching.

    gcc/testsuite/ChangeLog:

    2023-04-06  Martin Jambor  <mjambor@suse.cz>

            PR ipa/107769
            PR ipa/109318
            * gcc.dg/ipa/pr109318.c: New test.
            * gcc.dg/lto/pr107769_0.c: Likewise.

    (cherry picked from commit 8e08c7886eed5824bebd0e011526ec302d622844)

[Bug ipa/107769] [12 Regression] -flto with -Os/-O2/-O3 emitted code with gcc 12.x segfaults via mutated global in .rodata since r12-2887-ga6da2cddcf0e959d

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #11 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Fixed.