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

2022-12-10 Thread Jeffrey Walton
On Thu, Dec 8, 2022 at 3:58 AM Arsen Arsenović via Gnulib discussion
list  wrote:
>
> Eli Zaretskii  writes:
>
> >> Whereas with the Gnulib 'error' module, there is a conflict between the
> >> two global function definitions (with 'T' linkage) in install-info.c and
> >> in error.c *always*.
> >>
> >> > The simplest way to fix this problem would probably be to rename the 
> >> > "error"
> >> > function in install-info.c.
> >>
> >> Yes, or make it 'static' (like Arsen suggested).
> >
> > 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.  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 '\'
>  T error
>  U error
>
> 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.
>
> I imagine this would result is some very strange runtime failures if
> anyone ever observed install-info hit an xalloc_die condition.
>
> 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
>
> I imagine a similar thing would happen with a static glibc link.
> Renaming the functions or adding ``static'' elides this issue.
>
> So, GCC appears to be doing the right thing.
>
> 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):
>

I believe multiple definitions with the definitions in disagreement is
undefined behavior. https://stackoverflow.com/a/34986350 .

Jeff



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

2022-12-10 Thread Paul Eggert

On 2022-12-09 17:48, Gavin Smith wrote:

Since I went through the process of making all the symbols in that file
(besides main) local, here's the patch that does that

Thanks but no thanks.  install-info.c is a single-file program so there's
no point in adding the static keyword everywhere.


On the contrary, not only can adding 'static' prevent future issues like 
this with other symbols, it can help compilers generate better code in 
cases where adding 'static' lets them easily infer that a function 
cannot be called from outside the current compilation unit.


As a general rule in C, it's good to make all symbols static unless they 
really need to be extern.




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

2022-12-09 Thread Gavin Smith
On Sat, Dec 10, 2022 at 01:48:23AM +, Gavin Smith wrote:
> Making the symbols provided by install-info.c weak might work,
> so one idea is that when a program uses Gnulib, all of the global
> symbols from the program (excluding Gnulib) should be marked as weak
> in produced object files, so that Gnulib code preferentially uses
> code from glibc or other libraries.  I have no idea what would be
> needed to achieve this or what other implications there might be.
> (This won't help if the symbol is weak in those libraries too, though.)

On second thought this would be a bad idea in case the program uses
symbols that clash with library symbols that are marked weak.



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

2022-12-09 Thread Gavin Smith
On Thu, Dec 08, 2022 at 09:25:01AM +0100, Arsen Arsenović wrote:
> > 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.  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 '\'
>  T error
>  U error
> 
> 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.
> 
> I imagine this would result is some very strange runtime failures if
> anyone ever observed install-info hit an xalloc_die condition.

Your analysis makes sense here.  I have committed a change to make it
static.  I have also cherry-picked this commit to the release/7.0
branch in case another release is made from this branch.

Unless there is some way in Gnulib to prefer the glibc symbols when
linking, this seems unavoidable.  Defining the Gnulib symbols as "weak"
wouldn't help; as Arsen has said, it is the definition in install-info.c
itself that shouldn't be used when resolving the reference to "error"
in libgnu.a.

Making the symbols provided by install-info.c weak might work,
so one idea is that when a program uses Gnulib, all of the global
symbols from the program (excluding Gnulib) should be marked as weak
in produced object files, so that Gnulib code preferentially uses
code from glibc or other libraries.  I have no idea what would be
needed to achieve this or what other implications there might be.
(This won't help if the symbol is weak in those libraries too, though.)

I consulted some documentation on the ELF format but there appears
only to be one type of "undefined" symbol - it wouldn't be possible
to make the undefined symbols in libgnu.a preferentially resolve to
glibc symbols rather than other files in the program.   I'm very ignorant
of these matters though so it possible I missed something.

Likewise something might also be possible with how the linker (ld) is run
but somebody would have to research this.

> Since I went through the process of making all the symbols in that file
> (besides main) local, here's the patch that does that

Thanks but no thanks.  install-info.c is a single-file program so there's
no point in adding the static keyword everywhere.



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

2022-12-09 Thread Florian Weimer
* Bruno Haible:

> Gavin Smith wrote:
>> I expect it is not a gnulib problem.  install-info/install-info.c declares
>> a function called "error" which appears to clash with a function from glibc.
>> ... The "error" module is brought in by "xalloc" (which we
>> ask for explicitly).
>
> Yes, I think the problems already exists without use of Gnulib, as it's
> a conflict between install-info.c and glibc. But with the Gnulib 'error'
> module, the problem becomes bigger.
>
> Namely, see:
>
> $ nm --dynamic /lib/x86_64-linux-gnu/libc.so.6 | grep ' error'
> 001214e0 W error@@GLIBC_2.2.5
> 00121700 W error_at_line@@GLIBC_2.2.5
> 00221484 B error_message_count@@GLIBC_2.2.5
> 00221480 B error_one_per_line@@GLIBC_2.2.5
> 00221488 B error_print_progname@@GLIBC_2.2.5
>
> glibc exports 'error' as a weak symbol. This means, without use of Gnulib,
> the symbol from install-info.c overrides the one from glibc, and there is
> a problem if and only if the 'install-info' program links dynamically
> (or loads via 'dlopen') a shared library which happens to use error()
> and expects the semantics from glibc.

Note that weak vs non-weak does not matter for ELF dynamic linking.  The
definition in the main program always interposes those present in
depended-upon libraries.  This is necessary so that we can add functions
to glibc without an implementation-defined namespace prefix and support
multiple, conflicting standards in parallel (e..g, pthread_create is
reserved in POSIX, but not in C).

The weak flag for symbols is merely and artifact of some internal glibc
symbol management macros.

glibc does not participate in LTO, either, so it shouldn't matter here
at all.

> Whereas with the Gnulib 'error' module, there is a conflict between the
> two global function definitions (with 'T' linkage) in install-info.c and
> in error.c *always*.
>
>> The simplest way to fix this problem would probably be to rename the "error"
>> function in install-info.c.
>
> Yes, or make it 'static' (like Arsen suggested).

Right.

Thanks,
Florian




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

2022-12-08 Thread Arsen Arsenović
Hi,

Eli Zaretskii  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 '\'
>>  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 0xa1b0
  error in section .text of .../install-info/ginstall-info
  (gdb) info symbol 0x77eb56c0
  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: 61b0249 FUNCGLOBAL DEFAULT   14 error
   68: 61b0249 FUNCGLOBAL 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 g...@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, 

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

2022-12-08 Thread Eli Zaretskii
> From: Arsen Arsenović 
> Cc: Bruno Haible , gavinsmith0...@gmail.com,
>  s...@gentoo.org, bug-texi...@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 '\'
>  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.



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

2022-12-08 Thread Arsen Arsenović via Gnulib discussion list
Hi,

Eli Zaretskii  writes:

>> Whereas with the Gnulib 'error' module, there is a conflict between the
>> two global function definitions (with 'T' linkage) in install-info.c and
>> in error.c *always*.
>> 
>> > The simplest way to fix this problem would probably be to rename the 
>> > "error"
>> > function in install-info.c.
>> 
>> Yes, or make it 'static' (like Arsen suggested).
>
> 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.  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 '\'
 T error
 U error

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.

I imagine this would result is some very strange runtime failures if
anyone ever observed install-info hit an xalloc_die condition.

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

I imagine a similar thing would happen with a static glibc link.
Renaming the functions or adding ``static'' elides this issue.

So, GCC appears to be doing the right thing.

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

From 65b7657bfc0bc84178c95211690be94c767d725f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= 
Date: Thu, 8 Dec 2022 09:52:13 +0100
Subject: [PATCH] install-info: Make local symbols static

* install-info/install-info.c: Make local symbols static.
---
 ChangeLog   |  5 +++
 install-info/install-info.c | 62 ++---
 2 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9bfba11abb..98c2764b99 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2022-12-08  Arsen Arsenović  
+
+	install-info: Make local symbols static.
+	* install-info/install-info.c: Make local symbols static.
+
 2022-12-01  Arsen Arsenović  
 
 	Re-enable copyable anchors in HTML output
diff --git a/install-info/install-info.c b/install-info/install-info.c
index 8950288f6b..761cbfb4d4 100644
--- a/install-info/install-info.c
+++ b/install-info/install-info.c
@@ -28,11 +28,11 @@ static char *default_section = NULL;
 struct spec_entry;
 struct spec_section;
 
-struct line_data *findlines (char *data, int size, int *nlinesp);
-void insert_entry_here (struct spec_entry *entry, int line_number,
-struct line_data *dir_lines, int n_entries); 
-int compare_section_names (const void *s1, const void *s2);
-int compare_entries_text (const void *e1, const void *e2); 
+static struct line_data *findlines (char *data, int size, int *nlinesp);
+static void insert_entry_here (struct spec_entry *entry, int line_number,
+   struct line_data *dir_lines, int n_entries); 
+static int compare_section_names (const void *s1, const void *s2);
+static int compare_entries_text (const void *e1, const void *e2); 

 /* Data structures.  */
 
@@ -136,7 +136,7 @@ struct menu_section
 /* This table defines all the long-named options, says whether they
use an argument, and maps them into equivalent single-letter options.  */
 
-struct option longopts[] =
+static struct option longopts[] =
 {
   { "add-once",  no_argument, NULL, '1'},
   { "align", required_argument, NULL, 'A'},
@@ -172,39 +172,39 @@ struct option longopts[] =
   { 0 }
 };
 
-regex_t *psecreg = NULL;
+static regex_t *psecreg = NULL;
 
 /* Nonzero means that the name specified for the Info file will be used
(without removing .gz, .info extension or leading path) to match the
entries that must be removed.  */
-int remove_exactly = 0;
+static int remove_exactly = 0;
 
 /* Nonzero means that sections that don't have entries in them will be
deleted.  */
-int remove_empty_sections = 1;
+static int 

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

2022-12-07 Thread Eli Zaretskii
> From: Bruno Haible 
> Date: Thu, 08 Dec 2022 01:21:51 +0100
> 
> Gavin Smith wrote:
> > I expect it is not a gnulib problem.  install-info/install-info.c declares
> > a function called "error" which appears to clash with a function from glibc.
> > ... The "error" module is brought in by "xalloc" (which we
> > ask for explicitly).
> 
> Yes, I think the problems already exists without use of Gnulib, as it's
> a conflict between install-info.c and glibc. But with the Gnulib 'error'
> module, the problem becomes bigger.
> 
> Namely, see:
> 
> $ nm --dynamic /lib/x86_64-linux-gnu/libc.so.6 | grep ' error'
> 001214e0 W error@@GLIBC_2.2.5
> 00121700 W error_at_line@@GLIBC_2.2.5
> 00221484 B error_message_count@@GLIBC_2.2.5
> 00221480 B error_one_per_line@@GLIBC_2.2.5
> 00221488 B error_print_progname@@GLIBC_2.2.5
> 
> glibc exports 'error' as a weak symbol. This means, without use of Gnulib,
> the symbol from install-info.c overrides the one from glibc, and there is
> a problem if and only if the 'install-info' program links dynamically
> (or loads via 'dlopen') a shared library which happens to use error()
> and expects the semantics from glibc.

I don't think I understand how dynamic linking changes the situation
here.  If the application defines a function named 'error', why would
the dynamic linker pull it from a shared libc?

> Whereas with the Gnulib 'error' module, there is a conflict between the
> two global function definitions (with 'T' linkage) in install-info.c and
> in error.c *always*.
> 
> > The simplest way to fix this problem would probably be to rename the "error"
> > function in install-info.c.
> 
> Yes, or make it 'static' (like Arsen suggested).

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?



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

2022-12-07 Thread Bruno Haible
Gavin Smith wrote:
> I expect it is not a gnulib problem.  install-info/install-info.c declares
> a function called "error" which appears to clash with a function from glibc.
> ... The "error" module is brought in by "xalloc" (which we
> ask for explicitly).

Yes, I think the problems already exists without use of Gnulib, as it's
a conflict between install-info.c and glibc. But with the Gnulib 'error'
module, the problem becomes bigger.

Namely, see:

$ nm --dynamic /lib/x86_64-linux-gnu/libc.so.6 | grep ' error'
001214e0 W error@@GLIBC_2.2.5
00121700 W error_at_line@@GLIBC_2.2.5
00221484 B error_message_count@@GLIBC_2.2.5
00221480 B error_one_per_line@@GLIBC_2.2.5
00221488 B error_print_progname@@GLIBC_2.2.5

glibc exports 'error' as a weak symbol. This means, without use of Gnulib,
the symbol from install-info.c overrides the one from glibc, and there is
a problem if and only if the 'install-info' program links dynamically
(or loads via 'dlopen') a shared library which happens to use error()
and expects the semantics from glibc.

Whereas with the Gnulib 'error' module, there is a conflict between the
two global function definitions (with 'T' linkage) in install-info.c and
in error.c *always*.

> The simplest way to fix this problem would probably be to rename the "error"
> function in install-info.c.

Yes, or make it 'static' (like Arsen suggested).

Bruno






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

2022-12-07 Thread Gavin Smith
On Wed, Dec 07, 2022 at 08:57:45PM +, Sam James wrote:

> ../gnulib/lib/error.h:33:13: error: type of ‘error’ does not match original 
> declaration [-Werror=lto-type-mismatch]
>33 | extern void error (int __status, int __errnum, const char *__format, 
> ...)
>   | ^
> install-info.c:218:1: note: type mismatch in parameter 1
>   218 | error (const char *fmt, ...)
>   | ^
> install-info.c:218:1: note: ‘error’ was previously declared here
> install-info.c:218:1: note: code may be misoptimized unless 
> ‘-fno-strict-aliasing’ is used
> lto1: some warnings being treated as errors
> lto-wrapper: fatal error: //usr/bin/x86_64-pc-linux-gnu-gcc returned 1 exit 
> status
> compilation terminated.
> /usr/lib/gcc/x86_64-pc-linux-gnu/12/../../../../x86_64-pc-linux-gnu/bin/ld: 
> error: lto-wrapper failed
> collect2: error: ld returned 1 exit status
> make[3]: *** [Makefile:1514: ginstall-info] Error 1
> ```
> 
> This is with GCC 12.2.1 20221203.
> 
> We've also seen this in GNU make, so not sure if it's a
> gnulib problem or not, as it may be in lib/error.h (hence CCing bug-gnulib):
> - https://bugs.gentoo.org/863713 (texinfo)
> - https://bugs.gentoo.org/863824 (make)

I expect it is not a gnulib problem.  install-info/install-info.c declares
a function called "error" which appears to clash with a function from glibc.

The function called "error" in install-info is correctly used when
e.g. "install-info one two three" is run, printing

install-info: excess command line argument `three'

It may depend on the configuration process and what parts of gnulib are
being used.  The "error" module is brought in by "xalloc" (which we
ask for explicitly).

>From the Gentoo bug report
> -Werror=lto-type-mismatch:
> User to find possible runtime issues in packages. It likely means the package 
> is unsafe to build & use with LTO.
> For projects using the same identifier but with different types across 
> different files, they must be fixed to be consistent across the codebase.

The simplest way to fix this problem would probably be to rename the "error"
function in install-info.c.  Perhaps this issue has never come up before
because people have not used the LTO options for building.



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

2022-12-07 Thread Arsen Arsenović

Sam James  writes:

> ../gnulib/lib/error.h:33:13: error: type of ‘error’ does not match original 
> declaration [-Werror=lto-type-mismatch]
>33 | extern void error (int __status, int __errnum, const char *__format, 
> ...)
>   | ^
> install-info.c:218:1: note: type mismatch in parameter 1
>   218 | error (const char *fmt, ...)
>   | ^
> install-info.c:218:1: note: ‘error’ was previously declared here

This appears to be a result of error in install-info.c being non-static.
It could be static, and avoid a potential symbol collision.

-- 
Arsen Arsenović


signature.asc
Description: PGP signature