Bug 114115 - xz-utils segfaults when built with -fprofile-generate (bad interaction between IFUNC and binding?)
Summary: xz-utils segfaults when built with -fprofile-generate (bad interaction betwee...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: gcov-profile (show other bugs)
Version: 14.0
: P3 normal
Target Milestone: 14.0
Assignee: H.J. Lu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-26 14:10 UTC by Sam James
Modified: 2024-04-15 11:30 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 14.0
Known to fail:
Last reconfirmed: 2024-02-26 00:00:00


Attachments
A patch (625 bytes, patch)
2024-02-26 16:23 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James 2024-02-26 14:10:54 UTC
This was first reported downstream in Gentoo at https://bugs.gentoo.org/925415.

xz-utils-5.6.0 (it started to use IFUNC recently for crc32) started to segfault, but only when built with -march=x86-64-v3 & -fprofile-generate.

For convenience, a broken builddir is available at http://dev.gentoo.org/~sam/bugs/xz/pgo/xz-5.6.0-abi_x86_64.amd64.tar.xz.

```
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000000000041b6 in ?? ()
(gdb) bt
#0  0x00000000000041b6 in ?? ()
#1  0x00007f861b2fcc75 in crc32_resolve () at /var/tmp/portage/app-arch/xz-utils-5.6.0/work/xz-5.6.0/src/liblzma/check/crc32_fast.c:140
#2  0x00007f861b3541e4 in elf_machine_rela (map=<optimized out>, scope=<optimized out>, reloc=0x7f861b2e05c8, sym=0x7f861b2ddfd8, version=<optimized out>,
    reloc_addr_arg=0x7f861b32ab10 <lzma_crc32@got[plt]>, skip_ifunc=<optimized out>) at ../sysdeps/x86_64/dl-machine.h:314
#3  elf_dynamic_do_Rela (map=0x7f861b343160, scope=<optimized out>, reladdr=<optimized out>, relsize=<optimized out>, nrelative=<optimized out>, lazy=<optimized out>,
    skip_ifunc=<optimized out>) at /var/tmp/portage/sys-libs/glibc-2.39-r1/work/glibc-2.39/elf/do-rel.h:147
#4  _dl_relocate_object (l=l@entry=0x7f861b343160, scope=<optimized out>, reloc_mode=<optimized out>, consider_profiling=<optimized out>, consider_profiling@entry=0) at dl-reloc.c:301
#5  0x00007f861b363d61 in dl_main (phdr=<optimized out>, phnum=<optimized out>, user_entry=<optimized out>, auxv=<optimized out>) at rtld.c:2311
#6  0x00007f861b36059f in _dl_sysdep_start (start_argptr=start_argptr@entry=0x7ffdeae5bd20, dl_main=dl_main@entry=0x7f861b362060 <dl_main>)
    at ../sysdeps/unix/sysv/linux/dl-sysdep.c:140
#7  0x00007f861b361da2 in _dl_start_final (arg=0x7ffdeae5bd20) at rtld.c:494
#8  _dl_start (arg=0x7ffdeae5bd20) at rtld.c:581
#9  0x00007f861b360b88 in _start () from /lib64/ld-linux-x86-64.so.2
#10 0x0000000000000006 in ?? ()
#11 0x00007ffdeae5cfc9 in ?? ()
#12 0x00007ffdeae5d021 in ?? ()
#13 0x00007ffdeae5d026 in ?? ()
#14 0x00007ffdeae5d034 in ?? ()
#15 0x00007ffdeae5d03a in ?? ()
#16 0x00007ffdeae5d04b in ?? ()
#17 0x0000000000000000 in ?? ()
(gdb)
```

```
(gdb) frame 1
#1  0x00007f861b2fcc75 in crc32_resolve () at /var/tmp/portage/app-arch/xz-utils-5.6.0/work/xz-5.6.0/src/liblzma/check/crc32_fast.c:140
140     {
(gdb) list
135     // This resolver is shared between all three dispatch methods. It serves as
136     // the ifunc resolver if ifunc is supported, otherwise it is called as a
137     // regular function by the constructor or first call resolution methods.
138     static crc32_func_type
139     crc32_resolve(void)
140     {
141             return is_arch_extension_supported()
142                             ? &crc32_arch_optimized : &crc32_generic;
143     }
144
(gdb)
```
Comment 1 Sam James 2024-02-26 14:11:54 UTC
One of the xz developers, Jia Tan, has kindly minimised it to not need BIND_NOW. I've adapted it a bit to cleanup flags and warnings.

I can reproduce it with the following, at least:
```
#!/bin/sh
gcc-14 -O2 -march=znver2 -fvisibility=hidden -fPIC -fprofile-update=atomic -fprofile-dir=$(pwd) -fprofile-generate=$(pwd) -c test.c -o test.o -Wall -Wextra
gcc-14 -o libapp.so test.o -shared -Wl,-z,now -fPIC -lgcov
gcc-14 -o app main.c -lgcov -L. -lapp
LD_LIBRARY_PATH=. ./app
```

main.c:
```
#include <stdio.h>

extern int func();

int main(void)
{
    printf( "Hello world %p\n", func);

    return 0;
}
```

test.c:
```
__attribute__((visibility("default")))
void *foo_ifunc2() __attribute__((ifunc("foo_resolver")));


__attribute__((visibility("default")))
void bar(void)
{
}

static int f3()
{
    return 5;
}


__attribute__((visibility("default")))
void (*foo_resolver(void))(void)
{
    f3();
    return bar;
}


__attribute__((optimize("O0")))
__attribute__((visibility("default")))
int func()
{
    foo_ifunc2();
    return 0;
}
```
Comment 2 Sam James 2024-02-26 14:13:09 UTC
The reproducer succeeds for me with Clang 17.0.6, but fails for me with GCC 10..14.
Comment 3 Sam James 2024-02-26 14:13:49 UTC
(In reply to Sam James from comment #1)
> One of the xz developers, Jia Tan, has kindly minimised it to not need
> BIND_NOW. I've adapted it a bit to cleanup flags and warnings.

(oops, sorry, this one does need it - we were discussing whether we could elide it but didn't get there yet.)
Comment 4 Andrew Pinski 2024-02-26 14:37:03 UTC
It is the use of TLS inside an ifunc resolver which seems like causing issues ...
Comment 5 Andrew Pinski 2024-02-26 14:40:33 UTC
The obvious workaround is to mark the ifunc_resolver with no_profile_instrument_function attribute since is only ever called once and really does not need to be PGO'ed anyways.
Comment 6 Richard Biener 2024-02-26 14:52:54 UTC
Maybe we can automatically consider that when handling the ifunc attribute?
Comment 7 H.J. Lu 2024-02-26 16:23:17 UTC
Created attachment 57544 [details]
A patch
Comment 8 H.J. Lu 2024-02-26 22:54:20 UTC
A patch is posted at

https://patchwork.sourceware.org/project/gcc/list/?series=31343
Comment 9 Chung-Ju Wu 2024-04-02 10:27:44 UTC
(In reply to Sam James from comment #1)
> One of the xz developers, Jia Tan, has kindly minimised it to not need
> BIND_NOW. I've adapted it a bit to cleanup flags and warnings.
> 

CVE-2024-3094

Jia Tan is the one who injected backdoor in xz-5.6.0 and xz-5.6.1, which may be the cause of the segfaults. I'm wondering if we still need a workaround for this issue...
Comment 10 Sam James 2024-04-02 10:30:38 UTC
I'm aware, but there's a minimised test case attached here which shows this is still somewhat of a problem by itself.

Either a better diagnostic is needed or to not instrument the resolver.
Comment 11 Sam James 2024-04-02 10:35:10 UTC
(In reply to Sam James from comment #10)
> I'm aware, but there's a minimised test case attached here which shows this
> is still somewhat of a problem by itself.
> 
> Either a better diagnostic is needed or to not instrument the resolver.

s/better// (we don't emit any rn)
Comment 12 Andrew Pinski 2024-04-02 13:50:01 UTC
For anyone reading this, -fprofile-generate with ifunc attributes should be fixed and is not related to the xz backdoor. The issue will show up in valid usage of ifuncs even ones which don't call  external/non-inlined functions like the example code. There is another bug already about the diagnosising the calling of external functions so please don't file a new one. Also we don't need any extra comments about the backdoor in the gcc bugzilla.
Comment 13 GCC Commits 2024-04-03 14:07:10 UTC
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

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

commit r14-9775-gcab32bacaea268ec062b1fb4fc662d90c9d1cfce
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Feb 26 08:38:58 2024 -0800

    tree-profile: Disable indirect call profiling for IFUNC resolvers
    
    We can't profile indirect calls to IFUNC resolvers nor their callees as
    it requires TLS which hasn't been set up yet when the dynamic linker is
    resolving IFUNC symbols.
    
    Add an IFUNC resolver caller marker to cgraph_node and set it if the
    function is called by an IFUNC resolver.  Disable indirect call profiling
    for IFUNC resolvers and their callees.
    
    Tested with profiledbootstrap on Fedora 39/x86-64.
    
    gcc/ChangeLog:
    
            PR tree-optimization/114115
            * cgraph.h (symtab_node): Add check_ifunc_callee_symtab_nodes.
            (cgraph_node): Add called_by_ifunc_resolver.
            * cgraphunit.cc (symbol_table::compile): Call
            symtab_node::check_ifunc_callee_symtab_nodes.
            * symtab.cc (check_ifunc_resolver): New.
            (ifunc_ref_map): Likewise.
            (is_caller_ifunc_resolver): Likewise.
            (symtab_node::check_ifunc_callee_symtab_nodes): Likewise.
            * tree-profile.cc (gimple_gen_ic_func_profiler): Disable indirect
            call profiling for IFUNC resolvers and their callees.
    
    gcc/testsuite/ChangeLog:
    
            PR tree-optimization/114115
            * gcc.dg/pr114115.c: New test.
Comment 14 H.J. Lu 2024-04-03 14:08:20 UTC
Fixed for GCC 14 so far
Comment 15 Jan Hubicka 2024-04-03 14:10:14 UTC
> Fixed for GCC 14 so far
It is simple patch, so backporting is OK after a week in mainline.
Comment 16 GCC Commits 2024-04-05 09:13:40 UTC
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:9ab8fdfeef5b1a47b358e08a98177b2fad65fed9

commit r14-9803-g9ab8fdfeef5b1a47b358e08a98177b2fad65fed9
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Apr 5 10:16:41 2024 +0200

    middle-end/114599 - fix bitmap allocation for check_ifunc_callee_symtab_nodes
    
    There's no default bitmap obstack during global CTORs, so allocate the
    bitmap locally.
    
            PR middle-end/114599
            PR gcov-profile/114115
            * symtab.cc (ifunc_ref_map): Do not use auto_bitmap.
            (is_caller_ifunc_resolver): Optimize bitmap_bit_p/bitmap_set_bit
            pair.
            (symtab_node::check_ifunc_callee_symtab_nodes): Properly
            allocate ifunc_ref_map here.
Comment 17 H.J. Lu 2024-04-14 19:59:10 UTC
(In reply to Jan Hubicka from comment #15)
> > Fixed for GCC 14 so far
> It is simple patch, so backporting is OK after a week in mainline.

These are patches which I am backporting:

https://patchwork.sourceware.org/project/gcc/list/?series=32823
Comment 18 GCC Commits 2024-04-15 11:23:24 UTC
The releases/gcc-13 branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

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

commit r13-8606-gabe3a80aa2d6d53cc9b8c9f7c531e065451d5b6e
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sun Apr 14 12:57:39 2024 -0700

    tree-profile: Disable indirect call profiling for IFUNC resolvers
    
    We can't profile indirect calls to IFUNC resolvers nor their callees as
    it requires TLS which hasn't been set up yet when the dynamic linker is
    resolving IFUNC symbols.
    
    Add an IFUNC resolver caller marker to cgraph_node and set it if the
    function is called by an IFUNC resolver.  Disable indirect call profiling
    for IFUNC resolvers and their callees.
    
    Tested with profiledbootstrap on Fedora 39/x86-64.
    
    gcc/ChangeLog:
    
            PR tree-optimization/114115
            * cgraph.h (symtab_node): Add check_ifunc_callee_symtab_nodes.
            (cgraph_node): Add called_by_ifunc_resolver.
            * cgraphunit.cc (symbol_table::compile): Call
            symtab_node::check_ifunc_callee_symtab_nodes.
            * symtab.cc (check_ifunc_resolver): New.
            (ifunc_ref_map): Likewise.
            (is_caller_ifunc_resolver): Likewise.
            (symtab_node::check_ifunc_callee_symtab_nodes): Likewise.
            * tree-profile.cc (gimple_gen_ic_func_profiler): Disable indirect
            call profiling for IFUNC resolvers and their callees.
    
    gcc/testsuite/ChangeLog:
    
            PR tree-optimization/114115
            * gcc.dg/pr114115.c: New test.
    
    (cherry picked from commit cab32bacaea268ec062b1fb4fc662d90c9d1cfce)
Comment 19 GCC Commits 2024-04-15 11:26:25 UTC
The releases/gcc-12 branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:23049e851ebf840dffdd3f062dba0e795be347f8

commit r12-10331-g23049e851ebf840dffdd3f062dba0e795be347f8
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Feb 26 08:38:58 2024 -0800

    tree-profile: Disable indirect call profiling for IFUNC resolvers
    
    We can't profile indirect calls to IFUNC resolvers nor their callees as
    it requires TLS which hasn't been set up yet when the dynamic linker is
    resolving IFUNC symbols.
    
    Add an IFUNC resolver caller marker to cgraph_node and set it if the
    function is called by an IFUNC resolver.  Disable indirect call profiling
    for IFUNC resolvers and their callees.
    
    Tested with profiledbootstrap on Fedora 39/x86-64.
    
    gcc/ChangeLog:
    
            PR tree-optimization/114115
            * cgraph.h (symtab_node): Add check_ifunc_callee_symtab_nodes.
            (cgraph_node): Add called_by_ifunc_resolver.
            * cgraphunit.cc (symbol_table::compile): Call
            symtab_node::check_ifunc_callee_symtab_nodes.
            * symtab.cc (check_ifunc_resolver): New.
            (ifunc_ref_map): Likewise.
            (is_caller_ifunc_resolver): Likewise.
            (symtab_node::check_ifunc_callee_symtab_nodes): Likewise.
            * tree-profile.cc (gimple_gen_ic_func_profiler): Disable indirect
            call profiling for IFUNC resolvers and their callees.
    
    gcc/testsuite/ChangeLog:
    
            PR tree-optimization/114115
            * gcc.dg/pr114115.c: New test.
    
    (cherry picked from commit cab32bacaea268ec062b1fb4fc662d90c9d1cfce)
Comment 20 GCC Commits 2024-04-15 11:29:29 UTC
The releases/gcc-11 branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:574d52a9b6e40a466b90f4810e72d3dd072d5160

commit r11-11321-g574d52a9b6e40a466b90f4810e72d3dd072d5160
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Feb 26 08:38:58 2024 -0800

    tree-profile: Disable indirect call profiling for IFUNC resolvers
    
    We can't profile indirect calls to IFUNC resolvers nor their callees as
    it requires TLS which hasn't been set up yet when the dynamic linker is
    resolving IFUNC symbols.
    
    Add an IFUNC resolver caller marker to cgraph_node and set it if the
    function is called by an IFUNC resolver.  Disable indirect call profiling
    for IFUNC resolvers and their callees.
    
    Tested with profiledbootstrap on Fedora 39/x86-64.
    
    gcc/ChangeLog:
    
            PR tree-optimization/114115
            * cgraph.h (symtab_node): Add check_ifunc_callee_symtab_nodes.
            (cgraph_node): Add called_by_ifunc_resolver.
            * cgraphunit.c (symbol_table::compile): Call
            symtab_node::check_ifunc_callee_symtab_nodes.
            * symtab.c (check_ifunc_resolver): New.
            (ifunc_ref_map): Likewise.
            (is_caller_ifunc_resolver): Likewise.
            (symtab_node::check_ifunc_callee_symtab_nodes): Likewise.
            * tree-profile.c (gimple_gen_ic_func_profiler): Disable indirect
            call profiling for IFUNC resolvers and their callees.
    
    gcc/testsuite/ChangeLog:
    
            PR tree-optimization/114115
            * gcc.dg/pr114115.c: New test.
    
    (cherry picked from commit cab32bacaea268ec062b1fb4fc662d90c9d1cfce)
Comment 21 H.J. Lu 2024-04-15 11:30:57 UTC
Fixed for GCC 14 and GCC 11/12/13 release branches.