[Bug c/61958] New: function arbitrarily placed in .text.unlikely section

[Bug c/61958] New: function arbitrarily placed in .text.unlikely section

[jpoimboe at redhat dot com]

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

            Bug ID: 61958
           Summary: function arbitrarily placed in .text.unlikely section
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jpoimboe at redhat dot com

Created attachment 33206
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33206&action=edit
gzipped .i file for the bad case

When making changes to a certain source file in the Linux kernel, gcc is
unexpectedly moving two functions from the .text section to the .text.unlikely
section.

When compiling the original version of the source file, the two functions are
placed in the .text section:

$ cd linux
$ git describe
v3.16-rc7-7-g31dab71
$ make net/ipv4/fib_trie.o
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CC      net/ipv4/fib_trie.o
$ eu-readelf -s net/ipv4/fib_trie.o |grep 'insert_leaf_info\|leaf_info_new'
    6: 0000000000000000    136 FUNC    LOCAL  DEFAULT        1 insert_leaf_info
   37: 0000000000000dc0     88 FUNC    LOCAL  DEFAULT        1 leaf_info_new
$ eu-readelf -S net/ipv4/fib_trie.o |grep '\[ 1\]'
[ 1] .text                PROGBITS     0000000000000000 00000040 000038c0  0 AX
    0   0 16


However, when I apply the following patch, the two functions get unexpectedly
moved to .text.unlikely:

$ cat /tmp/fib_trie.patch
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 5afeb5a..cc5e3d3 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1187,6 +1187,7 @@ int fib_table_insert(struct fib_table *tb, struct
fib_config *cfg)

     key = key & mask;

+    printk("foo\n");
     fi = fib_create_info(cfg);
     if (IS_ERR(fi)) {
         err = PTR_ERR(fi);
$ patch -p1 < /tmp/fib_trie.patch
patching file net/ipv4/fib_trie.c
$ make net/ipv4/fib_trie.o
  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CC      net/ipv4/fib_trie.o
$ eu-readelf -s net/ipv4/fib_trie.o |grep 'insert_leaf_info\|leaf_info_new'
    6: 0000000000000000    111 FUNC    LOCAL  DEFAULT        5 insert_leaf_info
   28: 000000000000006f     82 FUNC    LOCAL  DEFAULT        5 leaf_info_new
$ eu-readelf -S net/ipv4/fib_trie.o |grep '\[ 5\]'
[ 5] .text.unlikely       PROGBITS     0000000000000000 00003770 000000c1  0 AX
    0   0  1


Both of the moved functions are called by fib_insert_node(), which is inlined
by fib_table_insert(), which was modified by the patch.  As far as I can tell,
it seems likely that the moved functions would be called, especially
insert_leaf_info() which is called in the main control path of the function. 
Also it seems odd that adding a call to printk would change the likelihood of a
function being called, since it doesn't change control flow.


Using gcc from Fedora rawhide:
$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.9.1/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla
--enable-bootstrap --enable-shared --enable-threads=posix
--enable-checking=release --enable-multilib --with-system-zlib
--enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object
--enable-linker-build-id --with-linker-hash-style=gnu
--enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --enable-plugin
--enable-initfini-array --disable-libgcj
--with-isl=/builddir/build/BUILD/gcc-4.9.1-20140717/obj-x86_64-redhat-linux/isl-install
--with-cloog=/builddir/build/BUILD/gcc-4.9.1-20140717/obj-x86_64-redhat-linux/cloog-install
--enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686
--build=x86_64-redhat-linux
Thread model: posix
gcc version 4.9.1 20140717 (Red Hat 4.9.1-2) (GCC) 

Full gcc cmdline:
  gcc -Wp,-MD,net/ipv4/.fib_trie.o.d  -nostdinc -isystem
/usr/lib/gcc/x86_64-redhat-linux/4.9.1/include -I./arch/x86/include
-Iarch/x86/include/generated  -Iinclude -I./arch/x86/include/uapi
-Iarch/x86/include/generated/uapi -I./include/uapi -Iinclude/generated/uapi
-include ./include/linux/kconfig.h -D__KERNEL__ -Wall -Wundef
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common
-Werror-implicit-function-declaration -Wno-format-security -m64 -mno-mmx
-mno-sse -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3
-mtune=generic -mno-red-zone -mcmodel=kernel -funit-at-a-time
-maccumulate-outgoing-args -DCONFIG_AS_CFI=1 -DCONFIG_AS_CFI_SIGNAL_FRAME=1
-DCONFIG_AS_CFI_SECTIONS=1 -DCONFIG_AS_FXSAVEQ=1 -DCONFIG_AS_CRC32=1
-DCONFIG_AS_AVX=1 -DCONFIG_AS_AVX2=1 -pipe -Wno-sign-compare
-fno-asynchronous-unwind-tables -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx
-fno-delete-null-pointer-checks -O2 -Wframe-larger-than=2048
-fstack-protector-strong -Wno-unused-but-set-variable -fno-omit-frame-pointer
-fno-optimize-sibling-calls -fno-var-tracking-assignments -g -pg -mfentry
-DCC_USING_FENTRY -Wdeclaration-after-statement -Wno-pointer-sign
-fno-strict-overflow -fconserve-stack -Werror=implicit-int
-Werror=strict-prototypes -Werror=date-time -DCC_HAVE_ASM_GOTO   
-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(fib_trie)" 
-D"KBUILD_MODNAME=KBUILD_STR(fib_trie)" -c -o net/ipv4/fib_trie.o
net/ipv4/fib_trie.c

I have also seen this issue with "gcc version 4.8.3 20140624 (Red Hat 4.8.3-1)
(GCC)" from Fedora 20.

The .i files are attached (and gzipped to be under the bugzilla file size
limit).

[Bug c/61958] function arbitrarily placed in .text.unlikely section

[jpoimboe at redhat dot com]

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

--- Comment #1 from Josh Poimboeuf <jpoimboe at redhat dot com> ---
Created attachment 33207
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33207&action=edit
gzipped .i file for the good case

[Bug middle-end/61958] functions arbitrarily placed in .text.unlikely section

[jpoimboe at redhat dot com]

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

--- Comment #2 from Josh Poimboeuf <jpoimboe at redhat dot com> ---
I see a similar issue with another patch to a different kernel file:

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index b10cd43a..40c275f 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -797,6 +797,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
 #endif

        err = -ENOBUFS;
+       printk("foo\n");
        if (fib_info_cnt >= fib_info_hash_size) {
                unsigned int new_size = fib_info_hash_size << 1;
                struct hlist_head *new_info_hash;


This results in fib_info_hashfn() no longer being inlined (despite its static
inline directive), and being placed in .text.unlikely.  Other functions are
also moved to .text.unlikely: fib_info_hash_free(), kzalloc.constprop.19(), and
fib_info_hash_alloc()  I can also provide the data for this error if needed,
but it looks like the same issue.

[Bug middle-end/61958] functions arbitrarily placed in .text.unlikely section

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Josh Poimboeuf from comment #2)
> This results in fib_info_hashfn() no longer being inlined (despite its
> static inline directive), and being placed in .text.unlikely. 

This sounds like a heuristic is saying this function is cold and has become too
big to inline.

[Bug middle-end/61958] functions arbitrarily placed in .text.unlikely section

[andi-gcc at firstfloor dot org]

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

Andi Kleen <andi-gcc at firstfloor dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andi-gcc at firstfloor dot org

--- Comment #4 from Andi Kleen <andi-gcc at firstfloor dot org> ---
It's because printk is marked cold. That is intentional, as normally only error
paths have printk.

asmlinkage __printf(1, 2) __cold
int printk(const char *fmt, ...);

->INVALID.

[Bug middle-end/61958] functions arbitrarily placed in .text.unlikely section

[mpolacek at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |mpolacek at gcc dot gnu.org
         Resolution|---                         |INVALID

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Agreed.