Re: -Wlto-type-mismatch warning in error()

[Arsen Arsenović]

Hi,

Eli Zaretskii <eliz@gnu.org> writes:

>> On top of that, Gnulib is pulling in error anyway:
>> 
>> $ nm ./gnulib/lib/libgnu.a | grep error
>>                  U error
>> $ nm install-info.o ../gnulib/lib/libgnu.a |& grep '\<error\>'
>> 00000000 T error
>>          U error
>
> Not sure what you wanted to demonstrate with this.  The above says
> that install-info.o includes the implementation of 'error', whereas
> libgnu.a only references 'error' as an unresolved symbol (which should
> then be resolved by the linker).

I was trying to say that the files being linked (in this case,
install-info.o and libgnu.a) already contain calls to error, the call
above is in lib/xalloc-die.c, which is compiled against the declaration
in lib/error.h, both in Gnulib.  Due to LTO being enabled, that call
would record error as the type error.h declares it:

  void error (int __status, int __errnum, const char *__format, ...)

Later, LTO mushes all that together, sees an undefined reference to
``error'', and sees a definition for it, compares the type, and sees
that they are mismatched, hence the warning.

>> My guess is that libgnu_a-xalloc-die.o (the file emitting the U error
>> symbol) includes gnulib/lib/error.h, GCC records that declaration
>> (through it's use in xalloc_die), and then detects a mismatch with the
>> one emitted by install-info.o (the T error symbol) and hence warns.
>
> The "U" has nothing to do with declaration, it is there because
> xalloc-die.c calls 'error', and the compiler sees the call.  AFAIU,
> Gnulib's intention in the above case is that this unresolved call will
> be resolved by linking against libc.

Indeed.

>> I imagine this would result is some very strange runtime failures if
>> anyone ever observed install-info hit an xalloc_die condition.
>
> Not if the reference in xalloc_die was resolved to the libc function
> by the same name.

It isn't:

  (gdb) info symbol 0x55555555a1b0
  error in section .text of .../install-info/ginstall-info
  (gdb) info symbol 0x7ffff7eb56c0
  error_at_line in section .text of /usr/lib64/libc.so.6
  (gdb) # symbol values obtained via p &...

And it wouldn't be, since the executable contains:

   76: 00000000000061b0    249 FUNC    GLOBAL DEFAULT       14 error
   68: 00000000000061b0    249 FUNC    GLOBAL DEFAULT       14 error

>> As a test, building on musl (which lacks the error API, for some reason)
>> causes the executable to be compiled with the install-info error, rather
>> than the Gnulib one, demonstrating why this warning happens.
>> 
>> Attempting to --whole-archive link on that platform grants us:
>> 
>> $ x86_64-linux-musl-gcc -o ginstall-info install-info.o \
>>   -Wl,--whole-archive ../gnulib/lib/libgnu.a -Wl,--no-whole-archive
>> /usr/libexec/gcc/x86_64-linux-musl/ld: 
>> ../gnulib/lib/libgnu.a(libgnu_a-error.o): in function `error':
>> error.c:(.text+0xe0): multiple definition of `error'; 
>> install-info.o:install-info.c:(.text+0x4a0): first defined here
>> collect2: error: ld returned 1 exit status
>
> Now _that_ IMO is a problem with Gnulib's use in this case: Gnulib
> evidently assumes that no application will define its own 'error'
> function, something that applications are free to do.

That is fair, yes.  It'd be a bit difficult to fix this issue, I think,
in Gnulib, since there's two cases:

- The system exposes error (), Gnulib doesn't touch it
- The system lacks error (), Gnulib provides it.

In the latter instance, Gnulib could just use a private alias
(e.g. gl_error) to call error(3), but that wouldn't work in the former
case because Glibc (et al.) lacks such an alias.  Aliases and static
libraries also might not mix as we'd want them to here.  I cannot
remember.

[[ Note that in the above musl case, (I lost the backlog) the function]]
[[ gets in the final executable is install-info.c:error rather than   ]]
[[ Gnulib error, so xalloc_die would behave the same wrong way there. ]]

>> I imagine a similar thing would happen with a static glibc link.
>> Renaming the functions or adding ``static'' elides this issue.
>
> IMO, doing that sweeps the problem under the carpet, without solving
> it.  We should try finding a better solution.

In that case, renaming the error () function in install-info is likely a
better path forward.

>> So, GCC appears to be doing the right thing.
>
> I'm not convinced it does.

It might be best to ask about this on gcc@gcc.gnu.org then, though I do
believe it's acting as it ought to be, seeing as the program is
referring to two different ``error''s in the same namespace.

>> Since I went through the process of making all the symbols in that file
>> (besides main) local, here's the patch that does that, though it's based
>> on a not-particularly-clean head (so, ChangeLog might conflict):
>
> It's up to Gavin, but a change like this means install-info will never
> be able to have more than one source file, with calls between these
> multiple files.  E.g., should install-info acquire a new source file
> called, say, utils.c, the functions in utils.c will be unable to call
> the function 'error' defined in install-info.c.
>
> So I don't think this kind of change is TRT, certainly not in general.

Indeed, if Gavin prefers, simply renaming the function would have the
benefit of being less work later, potentially.

> In general, I believe certain names used by a Standard C Library are
> "reserved", and applications must not redefine them.  But 'error' is
> not one of those reserved names, AFAIK.  So an application is in its
> full rights when it defines its own 'error' that is not compatible
> with that from libc.

Indeed, and if Gnulib is considered part of the said application, and it
usually acts mostly as if it were, this application (ginstall-info)
would have two conflicting ideas of what ``error'' means, and hence
getting this warning.

Have a lovely day.

[[ PS: I wrote this draft hours ago, and I only got to send it now. ]]
[[ Something came up in the meanwhile.  Sentences written based on  ]]
[[ Memory are marked with these brackets.  They could be wrong.     ]]
-- 
Arsen Arsenović

signature.asc
Description: PGP signature