[Bug lto/59626] [4.8/4.9 Regression] /usr/include/bits/unistd.h:173:1: error: inlining failed in call to always_inline 'readlinkat': recursive inlining

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59626

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|ASSIGNED                    |WAITING

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
We are merging the cgraph nodes for the function and the alias:

Merging nodes for readlinkat. Candidates:
readlinkat/0 (readlinkat) @0x7ffff66ba6f0
  Type: function
  Visibility: prevailing_def_ironly public
  next sharing asm name: 2
  References:
  Referring:
  Read from file: t.o
  Function flags: analyzed finalized
  Called by:
  Calls: *readlinkat/2 (1.00 per call)
*readlinkat/2 (__readlinkat_alias) @0x7ffff66ba940
  Type: function
  Visibility: external public
  previous sharing asm name: 0
  References:
  Referring:
  Read from file: t.o
  Function flags:
  Called by: main/1 (1.00 per call) readlinkat/0 (1.00 per call)
  Calls:
After resolution:
readlinkat/0 (readlinkat) @0x7ffff66ba6f0
  Type: function
  Visibility: prevailing_def_ironly public
  next sharing asm name: 2
  References:
  Referring:
  Read from file: t.o
  Function flags: analyzed finalized
  Called by:
  Calls: *readlinkat/2 (1.00 per call)
*readlinkat/2 (__readlinkat_alias) @0x7ffff66ba940
  Type: function
  Visibility: external public
  previous sharing asm name: 0
  References:
  Referring:
  Read from file: t.o
  Function flags:
  Called by: main/1 (1.00 per call) readlinkat/0 (1.00 per call)
  Calls:
...

Replacing cgraph node __readlinkat_alias/2 by readlinkat/0 for symbol
readlinkat
callgraph:

main/1 (main) @0x7ffff66ba818
  Type: function
  Visibility: force_output prevailing_def public
  References:
  Referring:
  Read from file: t.o
  Availability: available
  Function flags: analyzed finalized
  Called by:
  Calls: readlinkat/0 (1.00 per call)
readlinkat/0 (readlinkat) @0x7ffff66ba6f0
  Type: function
  Visibility: prevailing_def_ironly public
  References:
  Referring:
  Read from file: t.o
  Availability: available
  Function flags: analyzed finalized
  Called by: readlinkat/0 (1.00 per call) main/1 (1.00 per call)
  Calls: readlinkat/0 (1.00 per call)


And there is the optimization issue that we already get rid of the need
for the inline body during early inlining and thus we'd need to keep the
extern decl only.  And always-inline body never "prevails" (even though
we emit them eventually if inlining failed or somebody took its address -
something we maybe should change ...).  It works when using
extern inline __attribute__((always_inline,gnu_inline)) or extern inline
and not -std=c99.  So maybe that inline int readlinkat() {} _can_ prevail?

The glibc fortify wrappers use

extern __inline __attribute__ ((__always_inline__)) __attribute__
((__gnu_inline__)) __attribute__ ((__artificial__)) void *
__attribute__ ((__nothrow__ , __leaf__)) memcpy (void *__restrict __dest, const
void *__restrict __src, size_t __len)

{
  return __builtin___memcpy_chk (__dest, __src, __len, __builtin_object_size
(__dest, 0));
}

and thus extern gnu inline always-inline which works fine.  So IMHO this
bug doesn't warrant P1 status.

Btw, it fails with 4.8 even with -O0 (with 4.9 -O0 works!?  Ah, because
we massaged the condition guarding the error ...).

Still the callgraph is wrong and we miscompile the testcase in 4.9 with -O0
(to an endless recursion).

In the testcase from comment #2 'readlinkat' is

 <function_decl 0x7ffff6d4a800 readlinkat
...
    addressable used public static autoinline decl_5 QI file t.c line 4 col 10
align 8 context <translation_unit_decl 0x7ffff6d6a000 D.1748> attributes
<tree_list 0x7ffff6d45938> initial <block 0x7ffff6d55910>

which makes it go to the LTO symbol table (where the linker plugin then
decides it prevails - which is already wrong).  When adding 'extern' to
the declaration we do _not_ output it in the LTO symbol table because
it isn't even in the symtab encoder (it's reclaimed).

So I'm not sure if the testcase is even valid (without -flto we behave
the same as gcc 4.9 with -O0 and -flto - we compile this to endless
recursion - and a self-recursive always-inline function is ok to being
diagnosed IMHO).

So - I'm inclined to close this as INVALID?  My glibc version always has
'extern' on the inlines.

So - please provide preprocessed source of the file that uses readlinkat
or provide complete compile options.