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

[Eli Zaretskii]

> From: Arsen Arsenović <arsen@aarsen.me>
> Cc: Bruno Haible <bruno@clisp.org>, GavinSmith0123@gmail.com,
>  sam@gentoo.org, bug-texinfo@gnu.org, bug-gnulib@gnu.org
> Date: Thu, 08 Dec 2022 09:25:01 +0100
> 
> > Shouldn't we report this to the GCC folks?  It could be a bug in lto,
> > no?  I mean, 'error' is not a symbol that applications cannot use, and
> > if an application defines a function by that name, it should not be
> > imported from the standard library.  Right?
> 
> I believe this would make them part of the same program.

It shouldn't: the linker should only pull functions that it cannot
resolve until it gets to processing the library.  And 'error' should
have been resolved already to the version that is in install-info.c.

> 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).

> 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.

> 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.

> 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.

> 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.

> So, GCC appears to be doing the right thing.

I'm not convinced it does.

> 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.

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.