Bug 25355 - nm reports data variable as "T" with -flto
Summary: nm reports data variable as "T" with -flto
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.35 (HEAD)
: P2 normal
Target Milestone: 2.35
Assignee: Not yet assigned to anyone
URL: https://sourceware.org/ml/binutils/20...
Keywords:
: 25356 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-01-08 09:14 UTC by Martin Liška
Modified: 2020-03-25 14:10 UTC (History)
10 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2020-02-09 00:00:00


Attachments
A patch (3.88 KB, patch)
2020-02-09 23:13 UTC, H.J. Lu
Details | Diff
valgrind log file (1.53 KB, text/plain)
2020-02-11 11:47 UTC, Martin Liška
Details
libiberty archive (650.02 KB, application/x-bzip)
2020-02-11 13:50 UTC, Martin Liška
Details
Try this (1.44 KB, patch)
2020-02-11 17:00 UTC, H.J. Lu
Details | Diff
A patch (1.35 KB, patch)
2020-02-18 12:27 UTC, H.J. Lu
Details | Diff
A new patch (1.34 KB, patch)
2020-02-18 12:32 UTC, H.J. Lu
Details | Diff
An updated patch (1.33 KB, patch)
2020-02-18 16:10 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2020-01-08 09:14:54 UTC
As being discussed here:
https://lists.gnu.org/archive/html/libtool/2020-01/msg00022.html

nm reports misleadingly a global variable:

$ cat nm.c
char nm_test_var;

$ gcc-9 nm.c -c -fno-common
$ nm nm.o
0000000000000000 B nm_test_var

$ gcc-9 nm.c -c -fno-common -flto
$ nm nm.o
00000000 T nm_test_var

So the issue is that I would expect "B" from the last nm invocation.
Comment 1 Nick Clifton 2020-01-08 12:48:05 UTC
Hi Martin,

  I think that this problem is due to a weakness in the plugin-api.  The specification of the ld_plugin_symbol_kind enum only has five values:

  enum ld_plugin_symbol_kind
  {
    LDPK_DEF,
    LDPK_WEAKDEF,
    LDPK_UNDEF,
    LDPK_WEAKUNDEF,
    LDPK_COMMON
  };

So it does not have a way to distinguish between code symbols and data symbols.
Hence when a defined symbol is seen the BFD library has to make a choice, and it chooses to assume that it is a code symbol.  Given that section names have also been discarded by this point, it is impossible for the library to distinguish between code and data symbols.

So unless we create a new version of the plugin API, I think that this will have to remain as an unsolved problem.  At least that is my take on the subject.  Lets see if anyone else has any better ideas.

Cheers
  Nick
Comment 2 Nick Bowler 2020-01-08 15:21:26 UTC
Summary of the issue in libtool:

libtool needs to produce C declarations for arbitrary symbols based on nm output, in order to implement various features such as dlpreopen which depend on putting symbol values into C data structures.

Some linkers (apparently this the case on HP-UX) are strict and won't let function declarations refer to variables and vice versa.  So the nm symbol type is used to produce either a function or variable declaration, as appropriate.

Unfortunately, with LTO enabled the GNU linker (or does GCC do everything in this case?) seems to also have this restriction: it is an error for a function declaration to refer to a variable or vice versa.  So libtool uses the symbol type reported by nm to produce function declarations for functions, and variable declarations for variables.

Unfortunately, with LTO enabled then GNU nm does not distinguish between functions and variables (however, clang and llvm-nm seems to be fine in this regard).

There is no obvious way to work around the LTO function/data declaration issue in C code.  It seems libtool has to know, in advance, whether any given symbol is code or data in order to produce a declaration that will link successfully.

I see several options that would work to resolve the libtool problem:

  - Fix GNU nm to report functions and variables differently (best).
  - Fix GNU ld (or GCC?) so that it is not a hard error to refer to variables with function declarations or vice versa, so the nm issue does not matter.
  - Figure out another way to get the value of a symbol in C code without knowing anything other than its name.
Comment 3 Calvin Walton 2020-01-08 19:56:20 UTC
*** Bug 25356 has been marked as a duplicate of this bug. ***
Comment 4 Martin Liška 2020-01-09 08:14:30 UTC
(In reply to Nick Bowler from comment #2)
> Summary of the issue in libtool:
> 
> libtool needs to produce C declarations for arbitrary symbols based on nm
> output, in order to implement various features such as dlpreopen which
> depend on putting symbol values into C data structures.
> 
> Some linkers (apparently this the case on HP-UX) are strict and won't let
> function declarations refer to variables and vice versa.  So the nm symbol
> type is used to produce either a function or variable declaration, as
> appropriate.
> 
> Unfortunately, with LTO enabled the GNU linker (or does GCC do everything in
> this case?) seems to also have this restriction: it is an error for a
> function declaration to refer to a variable or vice versa.

Yes, it's critical for the compiler to not mix different symbol types (fn, var)
with the same name

> So libtool uses
> the symbol type reported by nm to produce function declarations for
> functions, and variable declarations for variables.
> 
> Unfortunately, with LTO enabled then GNU nm does not distinguish between
> functions and variables (however, clang and llvm-nm seems to be fine in this
> regard).

That's true, but it's only related to .o files (LTO bytecode). If you link
a final executable (or a shared library), you'll get proper type information:

$ cat conftest.c
int foo;
int main() { return foo; }
$ gcc conftest.c -flto -fno-common
$ nm a.out | grep ' main'
00000000004010f2 T main
$ nm a.out | grep foo
000000000040402c b foo

Would it be possible to libtool to make the detection on a binary executable
or a shared library?

> 
> There is no obvious way to work around the LTO function/data declaration
> issue in C code.  It seems libtool has to know, in advance, whether any
> given symbol is code or data in order to produce a declaration that will
> link successfully.
> 
> I see several options that would work to resolve the libtool problem:
> 
>   - Fix GNU nm to report functions and variables differently (best).
>   - Fix GNU ld (or GCC?) so that it is not a hard error to refer to
> variables with function declarations or vice versa, so the nm issue does not
> matter.
>   - Figure out another way to get the value of a symbol in C code without
> knowing anything other than its name.
Comment 5 Nick Bowler 2020-01-09 14:15:17 UTC
(In reply to Martin Liška from comment #4)
> That's true, but it's only related to .o files (LTO bytecode). If you link
> a final executable (or a shared library), you'll get proper type information:
> 
> $ cat conftest.c
> int foo;
> int main() { return foo; }
> $ gcc conftest.c -flto -fno-common
> $ nm a.out | grep ' main'
> 00000000004010f2 T main
> $ nm a.out | grep foo
> 000000000040402c b foo
> 
> Would it be possible to libtool to make the detection on a binary executable
> or a shared library?

Probably not.  The features being probed, by their very nature, generally
need to be used prior to linking.

The configure test is actually working essentially correctly.  Libtool
has determined that the feature being probed (the ability to produce
C declarations from nm output) will not work.  This appears to be a
reasonably accurate assessment of the situation when using GNU nm
with LTO.

On the libtool side we can probably make the following improvements:

 - The configure test actually links together the results for two
   features (global_symbol_pipe and global_symbol_to_cdecl).  So if
   one doesn't work libtool assumes neither works.  They could perhaps
   be tested independently as I think global_symbol_pipe is probably 
   mostly OK in spite of this bug.

 - The documentation should mention the possibility that these variables
   could be empty on some targets if the feature is not supported.

 - The export-symbols-regex feature should probably not fail gratuitously
   on targets where global_symbol_pipe is not supported.

 - Investigate alternate ways to implement global_symbol_to_cdecl on
   GNU targets which avoids the LTO linking error.

 - Investigate workarounds for this particular nm bug (e.g., libtool
   could enable -ffat-lto-objects and using nm --plugin '' which restores
   working nm behaviour).
Comment 6 Nick Clifton 2020-01-09 15:28:40 UTC
(In reply to Nick Bowler from comment #5)

>  - The configure test actually links together the results for two
>    features (global_symbol_pipe and global_symbol_to_cdecl).

The following suggestion is an outrageous hack, so please feel free
to ignore it:  What if the BFD library's LTO plugin handling code was
enhanced to include a heuristic that would choose between code and data
based upon the name of the symbol ?  So for "global_symbol_to_cdecl"
it would choose 'data' and for anything else it would stick with its
current default guess of 'code' ...  Obviously this is far from ideal,
but with no other source of information available to it, the symbol
name is all that the library has to go on.

Cheers
  Nick
Comment 7 Nick Bowler 2020-01-09 16:26:51 UTC
(In reply to Nick Clifton from comment #6)
> (In reply to Nick Bowler from comment #5)
> 
> >  - The configure test actually links together the results for two
> >    features (global_symbol_pipe and global_symbol_to_cdecl).
> 
> The following suggestion is an outrageous hack, so please feel free
> to ignore it:  What if the BFD library's LTO plugin handling code was
> enhanced to include a heuristic that would choose between code and data
> based upon the name of the symbol ?  So for "global_symbol_to_cdecl"
> it would choose 'data' and for anything else it would stick with its
> current default guess of 'code' ...  Obviously this is far from ideal,
> but with no other source of information available to it, the symbol
> name is all that the library has to go on.

I don't think such a hack will actually help libtool.

Since this is now a known portability issue we can and should improve the
configure test to detect this scenario reliably.

PS: I was able to work around the LTO function-versus-data link failure
by declaring "sym" like this:

  __asm(".equ sym_____, sym"); int sym() asm("sym_____");

Now my C code can successfully access the value of the "sym" symbol and
GCC doesn't seem to care if "sym" is declared as a variable in another
translation unit.

Libtool could possibly make use of this or a similar trick to work
around this bug as accurate symbol types would not be needed.
Presumably this won't work on every GCC target but it might be
good enough.

Amusingly, "nm" is also busted on objects using this trick with -flto,
showing a_____ as an undefined symbol which is not the case.  But that
shouldn't cause any issue for libtool's uses of nm.
Comment 8 Alan Modra 2020-01-10 10:40:50 UTC
To reinforce Nick's comment #1 about the plugin api, this is what we get in bfd/plugin.c:bfd_plugin_canonicalize_symtab for gcc -fno-common compiling
char nm_test_var;
int nm_test_func (void) { return 0; }

(gdb) p syms[0]
$1 = {name = 0xb33800 "nm_test_func", version = 0x0, def = 0, visibility = 0, size = 0, comdat_key = 0x0, resolution = 0}
(gdb) p syms[1]
$2 = {name = 0xb33820 "nm_test_var", version = 0x0, def = 0, visibility = 0, size = 0, comdat_key = 0x0, resolution = 0}

There isn't anything to distinguish variables from functions, and so nothing can be done in nm.
Comment 9 Martin Liška 2020-01-20 12:56:45 UTC
> 
> Amusingly, "nm" is also busted on objects using this trick with -flto,
> showing a_____ as an undefined symbol which is not the case.  But that
> shouldn't cause any issue for libtool's uses of nm.

Hello Nick.
Have you made a progress about the upstreaming of the workaround?
Comment 10 Nick Bowler 2020-01-20 18:47:21 UTC
(In reply to Martin Liška from comment #9)
> > Amusingly, "nm" is also busted on objects using this trick with -flto,
> > showing a_____ as an undefined symbol which is not the case.  But that
> > shouldn't cause any issue for libtool's uses of nm.
> 
> Hello Nick.
> Have you made a progress about the upstreaming of the workaround?

I did actually implement this trick but unfortunately it turns
out that symbol references in asm cause breakage w/ GCC's LTO when
linking against static libraries.  So this is not a workaround that
can be used in libtool, sadly.

I hope to investigate other options to work around this issue in
libtool.
Comment 11 Arvind Sankar 2020-02-06 20:32:48 UTC
While checking what happens during gcc-10 bootstrap with libtool configuration, I noticed that the current test code doesn't work with C++.

lt__PROGRAM__LTX_preloaded_symbols gets declared as a const struct [], which has static linkage in C++, and at any non-zero optimization level that test file gets completely eliminated.

So the pipe is assumed to always work if configure is running with a C++ compiler.
Comment 12 H.J. Lu 2020-02-09 21:34:54 UTC
It is independent of -fno-common:


[hjl@gnu-cfl-2 pr25355]$ cat x.c
int nm_test_var = 30;

extern int foo (void);

int
main ()
{
  return foo ();
}
[hjl@gnu-cfl-2 pr25355]$ gcc -flto -c x.c
[hjl@gnu-cfl-2 pr25355]$ gcc-nm x.o
         U foo
00000000 T main
00000000 T nm_test_var
[hjl@gnu-cfl-2 pr25355]$
Comment 13 Sergei Trofimovich 2020-02-09 22:47:45 UTC
(In reply to H.J. Lu from comment #12)
> It is independent of -fno-common:
> 
> 
> [hjl@gnu-cfl-2 pr25355]$ cat x.c
> int nm_test_var = 30;
> 
> extern int foo (void);
> 
> int
> main ()
> {
>   return foo ();
> }
> [hjl@gnu-cfl-2 pr25355]$ gcc -flto -c x.c
> [hjl@gnu-cfl-2 pr25355]$ gcc-nm x.o
>          U foo
> 00000000 T main
> 00000000 T nm_test_var
> [hjl@gnu-cfl-2 pr25355]$

Which gcc version you test? gcc-10 switched to -fno-common in https://gcc.gnu.org/PR85678.
Comment 14 H.J. Lu 2020-02-09 23:13:53 UTC
Created attachment 12277 [details]
A patch
Comment 15 Martin Liška 2020-02-10 08:37:27 UTC
Thank you H.J. I can confirm the patch works:

Before:

$ cat x.c
int nm_test_var;
int nm_test_var2 = 1234;

extern int foo (void);

int
main ()
{
  return foo ();
}

$ gcc-9 -fno-common x.c -c -flto && nm x.o
         U foo
00000000 T main
00000000 T nm_test_var
00000000 T nm_test_var2

After:
$ ~/bin/binutils/bin/nm --plugin /usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0 x.o
         U foo
00000000 T main
00000000 B nm_test_var
00000000 D nm_test_var2
Comment 16 H.J. Lu 2020-02-10 14:26:57 UTC
A patch is posted at

https://sourceware.org/ml/binutils/2020-02/msg00144.html
Comment 17 cvs-commit@gcc.gnu.org 2020-02-11 03:04:48 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0aa99dcd70bce68f8efef310350a6294e1143382

commit 0aa99dcd70bce68f8efef310350a6294e1143382
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Feb 10 19:01:42 2020 -0800

    Use GCC LTO wrapper to get real symbols from LTO IR objects
    
    GCC LTO wrapper is needed to extract real symbols from LTO IR objects.
    This patch does the following:
    
    1. Set up GCC LTO wrapper for each LTO IR object.
    2. Run GCC LTO wrapper to get the real object.
    3. Extract symbol info from the real object.
    4. Cleanup afterwards.
    
    bfd/
    
    	PR binutils/25355
    	* configure.ac (HAVE_EXECUTABLE_SUFFIX): New AC_DEFINE.
    	(EXECUTABLE_SUFFIX): Likewise.
    	* config.in: Regenerated.
    	* configure: Likewise.
    	* plugin.c (bfd_plugin_close_and_cleanup): Removed.
    	(plugin_list_entry): Add all_symbols_read, cleanup_handler,
    	gcc, lto_wrapper, resolution_file, resolution_option, gcc_env,
    	real_bfd, real_nsyms, real_syms, lto_nsyms and lto_syms.
    	(get_lto_wrapper): New.
    	(setup_lto_wrapper_env): Likewise.
    	(current_plugin): Likewise.
    	(register_all_symbols_read): Likewise.
    	(register_cleanup): Likewise.
    	(get_symbols): Likewise.
    	(add_input_file): Likewise.
    	(bfd_plugin_close_and_cleanup): Likewise.
    	(claim_file): Removed.
    	(register_claim_file): Set current_plugin->claim_file.
    	(add_symbols): Make a copy of LTO symbols.  Set lto_nsyms and
    	lto_syms in current_plugin.
    	(try_claim): Use current_plugin->claim_file.  Call LTO plugin
    	all_symbols_read handler.  Copy real symbols to plugin_data.
    	Call LTO plugin cleanup handler.  Clean up for LTO wrapper.
    	(try_load_plugin): Don't reuse the previous plugin for LTO
    	wrapper.  Set up GCC LTO wrapper if possible.  Don't set
    	plugin_list_iter->claim_file.
    	(bfd_plugin_canonicalize_symtab): Use real LTO symbols if
    	possible.
    	* plugin.h (plugin_data_struct): Add real_bfd, real_nsyms and
    	real_syms.
    
    ld/
    
    	PR binutils/25355
    	* testsuite/ld-plugin/lto.exp: Run PR binutils/25355 test.
    	* testsuite/ld-plugin/pr25355.c: New file.
    	* testsuite/ld-plugin/pr25355.d: Likewise.
    	* testsuite/lib/ld-lib.exp (run_cc_link_tests): Support compile
    	only dump.
Comment 18 H.J. Lu 2020-02-11 03:06:53 UTC
Fixed for 2.35.
Comment 19 Martin Liška 2020-02-11 11:46:45 UTC
Thank you H.J. but I see the following memory corruption with the patch applied:

$ valgrind ~/bin/binutils/bin/nm --plugin /usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0 x.o
...
==28080== Invalid read of size 8
==28080==    at 0x48A55D: bfd_plugin_canonicalize_symtab (plugin.c:860)
==28080==    by 0x4123C4: _bfd_generic_read_minisymbols (syms.c:826)
==28080==    by 0x4039A9: display_rel_file (nm.c:1112)
==28080==    by 0x4043DA: display_file (nm.c:1379)
==28080==    by 0x402B64: main (nm.c:1860)
==28080==  Address 0x4a81498 is 1,704 bytes inside a block of size 4,064 free'd
==28080==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==28080==    by 0x4ACCF2: objalloc_free (objalloc.c:187)
==28080==    by 0x40FAB2: _bfd_delete_bfd (opncls.c:126)
==28080==    by 0x41067B: bfd_close_all_done (opncls.c:797)
==28080==    by 0x41067B: bfd_close_all_done (opncls.c:785)
==28080==    by 0x48A236: add_input_file (plugin.c:315)
==28080==    by 0x485B1D1: ??? (in /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
==28080==    by 0x48A8FB: try_claim (plugin.c:417)
==28080==    by 0x48A8FB: try_load_plugin (plugin.c:562)
==28080==    by 0x48AF4C: load_plugin (plugin.c:637)
==28080==    by 0x48AF4C: bfd_plugin_object_p (plugin.c:704)
==28080==    by 0x40DE35: bfd_check_format_matches (format.c:261)
==28080==    by 0x4043B1: display_file (nm.c:1375)
==28080==    by 0x402B64: main (nm.c:1860)
==28080==  Block was alloc'd at
==28080==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==28080==    by 0x4ACC2B: _objalloc_alloc (objalloc.c:159)
==28080==    by 0x410990: bfd_alloc (opncls.c:978)
==28080==    by 0x410E29: bfd_zalloc (opncls.c:1027)
==28080==    by 0x42C679: _bfd_elf_new_section_hook (elf.c:2902)
==28080==    by 0x41148E: bfd_section_init (section.c:825)
==28080==    by 0x42B46B: _bfd_elf_make_section_from_shdr (elf.c:1035)
==28080==    by 0x42B46B: _bfd_elf_make_section_from_shdr (elf.c:1023)
==28080==    by 0x42A9A0: bfd_section_from_shdr (elf.c:2586)
==28080==    by 0x4261DB: bfd_elf64_object_p (elfcode.h:815)
==28080==    by 0x40DC89: bfd_check_format_matches (format.c:328)
==28080==    by 0x48A1E3: add_input_file (plugin.c:300)
==28080==    by 0x485B1D1: ??? (in /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
...
Comment 20 Martin Liška 2020-02-11 11:47:22 UTC
Created attachment 12283 [details]
valgrind log file
Comment 21 H.J. Lu 2020-02-11 12:22:04 UTC
(In reply to Martin Liška from comment #19)
> Thank you H.J. but I see the following memory corruption with the patch
> applied:
> 
> $ valgrind ~/bin/binutils/bin/nm --plugin
> /usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0 x.o
> ...
> ==28080== Invalid read of size 8
> ==28080==    at 0x48A55D: bfd_plugin_canonicalize_symtab (plugin.c:860)
> ==28080==    by 0x4123C4: _bfd_generic_read_minisymbols (syms.c:826)
> ==28080==    by 0x4039A9: display_rel_file (nm.c:1112)
> ==28080==    by 0x4043DA: display_file (nm.c:1379)
> ==28080==    by 0x402B64: main (nm.c:1860)
> ==28080==  Address 0x4a81498 is 1,704 bytes inside a block of size 4,064
> free'd
> ==28080==    at 0x48389AB: free (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==28080==    by 0x4ACCF2: objalloc_free (objalloc.c:187)
> ==28080==    by 0x40FAB2: _bfd_delete_bfd (opncls.c:126)
> ==28080==    by 0x41067B: bfd_close_all_done (opncls.c:797)
> ==28080==    by 0x41067B: bfd_close_all_done (opncls.c:785)
> ==28080==    by 0x48A236: add_input_file (plugin.c:315)
> ==28080==    by 0x485B1D1: ??? (in
> /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
> ==28080==    by 0x48A8FB: try_claim (plugin.c:417)
> ==28080==    by 0x48A8FB: try_load_plugin (plugin.c:562)
> ==28080==    by 0x48AF4C: load_plugin (plugin.c:637)
> ==28080==    by 0x48AF4C: bfd_plugin_object_p (plugin.c:704)
> ==28080==    by 0x40DE35: bfd_check_format_matches (format.c:261)
> ==28080==    by 0x4043B1: display_file (nm.c:1375)
> ==28080==    by 0x402B64: main (nm.c:1860)
> ==28080==  Block was alloc'd at
> ==28080==    at 0x483777F: malloc (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==28080==    by 0x4ACC2B: _objalloc_alloc (objalloc.c:159)
> ==28080==    by 0x410990: bfd_alloc (opncls.c:978)
> ==28080==    by 0x410E29: bfd_zalloc (opncls.c:1027)
> ==28080==    by 0x42C679: _bfd_elf_new_section_hook (elf.c:2902)
> ==28080==    by 0x41148E: bfd_section_init (section.c:825)
> ==28080==    by 0x42B46B: _bfd_elf_make_section_from_shdr (elf.c:1035)
> ==28080==    by 0x42B46B: _bfd_elf_make_section_from_shdr (elf.c:1023)
> ==28080==    by 0x42A9A0: bfd_section_from_shdr (elf.c:2586)
> ==28080==    by 0x4261DB: bfd_elf64_object_p (elfcode.h:815)
> ==28080==    by 0x40DC89: bfd_check_format_matches (format.c:328)
> ==28080==    by 0x48A1E3: add_input_file (plugin.c:300)
> ==28080==    by 0x485B1D1: ??? (in
> /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
> ...

I can't reproduce it on master branch:

[hjl@gnu-cfl-2 pr25355]$ valgrind ./nm --plugin /usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0 x.o 
==315887== Memcheck, a memory error detector
==315887== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==315887== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==315887== Command: ./nm --plugin /usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0 x.o
==315887== 
         U foo
00000000 T main
00000000 B nm_test_var
00000000 D nm_test_var2
==315887== 
==315887== HEAP SUMMARY:
==315887==     in use at exit: 2,037 bytes in 17 blocks
==315887==   total heap usage: 115 allocs, 98 frees, 169,604 bytes allocated
==315887== 
==315887== LEAK SUMMARY:
==315887==    definitely lost: 83 bytes in 2 blocks
==315887==    indirectly lost: 0 bytes in 0 blocks
==315887==      possibly lost: 0 bytes in 0 blocks
==315887==    still reachable: 1,954 bytes in 15 blocks
==315887==         suppressed: 0 bytes in 0 blocks
==315887== Rerun with --leak-check=full to see details of leaked memory
==315887== 
==315887== For lists of detected and suppressed errors, rerun with: -s
==315887== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
[hjl@gnu-cfl-2 pr25355]$
Comment 22 H.J. Lu 2020-02-11 12:35:40 UTC
(In reply to Martin Liška from comment #19)
> Thank you H.J. but I see the following memory corruption with the patch
> applied:
> 
> $ valgrind ~/bin/binutils/bin/nm --plugin
> /usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0 x.o
> ...
> ==28080== Invalid read of size 8
> ==28080==    at 0x48A55D: bfd_plugin_canonicalize_symtab (plugin.c:860)

It looks like you were using one of my old patches.  Line 860 in plugin.c
doesn't do anything on master branch.
Comment 23 Martin Liška 2020-02-11 12:42:22 UTC
> 
> It looks like you were using one of my old patches.  Line 860 in plugin.c
> doesn't do anything on master branch.

You are right, sorry for the noise.
Comment 24 Martin Liška 2020-02-11 13:38:29 UTC
Ok, I've got one another problem with the 2 commits (de66c68d899600286b0d054508a2ed7beee64870 and 39f2b43be6ccc3acb29ab84dee48180b6a8fcba5) applied on top of the bintuils 2.34 release. I built a openSUSE package and now I see:

$ nm ./libiberty.a

regex.o:
                 U abort
00000000000001c0 t byte_alt_match_null_string_p
00000000000000d0 t byte_common_op_match_null_string_p
0000000000000000 t byte_compile_range
0000000000000240 t byte_group_match_null_string_p
00000000000003a0 t byte_re_compile_fastmap
0000000000000000 t byte_re_compile_fastmap.cold
0000000000002c10 t byte_regex_compile
0000000000000060 b byte_reg_unset_dummy
0000000000000760 t byte_re_match_2_internal
0000000000000005 t byte_re_match_2_internal.cold
00000000000028d0 t byte_re_search_2
                 U __ctype_b_loc
                 U __ctype_tolower_loc
0000000000000000 b done.3131
                 U free
                 U malloc
                 U memcmp
                 U memcpy
                 U realloc
0000000000000020 b re_comp_buf
0000000000000840 r re_error_msgid
0000000000000080 b re_syntax_table
                 U strcmp
                 U strlen
0000000000005cd0 T xre_comp
0000000000005be0 T xre_compile_fastmap
0000000000005c90 T xre_compile_pattern
0000000000005d80 T xre_exec
0000000000005dc0 T xregcomp
0000000000006100 T xregerror
000000000000000a t xregerror.cold
0000000000005f50 T xregexec
0000000000006180 T xregfree
0000000000005c60 T xre_match
0000000000005c80 T xre_match_2
0000000000000000 D xre_max_failures
0000000000005c30 T xre_search
0000000000005c50 T xre_search_2
0000000000005bf0 T xre_set_registers
0000000000005bd0 T xre_set_syntax
0000000000000008 C xre_syntax_options

cplus-dem.o:
0000000000000090 T ada_demangle
0000000000000600 T cplus_demangle
0000000000000040 T cplus_demangle_name_to_style
0000000000000000 T cplus_demangle_set_style
                 U cplus_demangle_v3
0000000000000000 D current_demangling_style
                 U dlang_demangle
                 U free
                 U java_demangle_v3
00000000000001a0 R libiberty_demanglers
                 U memcpy
0000000000000060 r operators.3980
                 U rust_demangle
                 U _sch_istable
0000000000000000 r special.4008
                 U sprintf
                 U strcmp
                 U strcpy
                 U strlen
                 U strncmp
                 U xmalloc
                 U xstrdup
free(): double free detected in tcache 2
Aborted (core dumped)

$ valgrind nm ./libiberty.a
==26662== Memcheck, a memory error detector
==26662== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==26662== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==26662== Command: nm ./libiberty.a
==26662== 
==26662== Invalid read of size 1
==26662==    at 0x4DF3DC0: __strcmp_avx2 (strcmp-avx2.S:92)
==26662==    by 0x4B1FFF9: try_load_plugin (plugin.c:608)
==26662==    by 0x4B20996: load_plugin (plugin.c:843)
==26662==    by 0x4B20996: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Address 0x4ea7fc0 is 0 bytes inside a block of size 58 free'd
==26662==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B209B6: load_plugin (plugin.c:847)
==26662==    by 0x4B209B6: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Block was alloc'd at
==26662==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B338C4: xmalloc (xmalloc.c:147)
==26662==    by 0x4B2DE79: concat (concat.c:147)
==26662==    by 0x4B2095F: load_plugin (plugin.c:838)
==26662==    by 0x4B2095F: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662== 
==26662== Invalid read of size 1
==26662==    at 0x483BBA8: strcmp (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B1FFF9: try_load_plugin (plugin.c:608)
==26662==    by 0x4B20996: load_plugin (plugin.c:843)
==26662==    by 0x4B20996: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Address 0x4ea7fc1 is 1 bytes inside a block of size 58 free'd
==26662==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B209B6: load_plugin (plugin.c:847)
==26662==    by 0x4B209B6: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Block was alloc'd at
==26662==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B338C4: xmalloc (xmalloc.c:147)
==26662==    by 0x4B2DE79: concat (concat.c:147)
==26662==    by 0x4B2095F: load_plugin (plugin.c:838)
==26662==    by 0x4B2095F: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662== 
==26662== Invalid read of size 1
==26662==    at 0x4DF3DC0: __strcmp_avx2 (strcmp-avx2.S:92)
==26662==    by 0x4B20042: try_load_plugin (plugin.c:637)
==26662==    by 0x4B20996: load_plugin (plugin.c:843)
==26662==    by 0x4B20996: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Address 0x4ea7fc0 is 0 bytes inside a block of size 58 free'd
==26662==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B209B6: load_plugin (plugin.c:847)
==26662==    by 0x4B209B6: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Block was alloc'd at
==26662==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B338C4: xmalloc (xmalloc.c:147)
==26662==    by 0x4B2DE79: concat (concat.c:147)
==26662==    by 0x4B2095F: load_plugin (plugin.c:838)
==26662==    by 0x4B2095F: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662== 
==26662== Invalid read of size 1
==26662==    at 0x483BBA8: strcmp (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B20042: try_load_plugin (plugin.c:637)
==26662==    by 0x4B20996: load_plugin (plugin.c:843)
==26662==    by 0x4B20996: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Address 0x4ea7fc1 is 1 bytes inside a block of size 58 free'd
==26662==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B209B6: load_plugin (plugin.c:847)
==26662==    by 0x4B209B6: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662==  Block was alloc'd at
==26662==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==26662==    by 0x4B338C4: xmalloc (xmalloc.c:147)
==26662==    by 0x4B2DE79: concat (concat.c:147)
==26662==    by 0x4B2095F: load_plugin (plugin.c:838)
==26662==    by 0x4B2095F: bfd_plugin_object_p (plugin.c:869)
==26662==    by 0x49477F5: bfd_check_format_matches (format.c:261)
==26662==    by 0x10D2A1: display_archive (nm.c:1314)
==26662==    by 0x10D2A1: display_file (nm.c:1373)
==26662==    by 0x10B9BF: main (nm.c:1860)
==26662== 

regex.o:
                 U abort
00000000000001c0 t byte_alt_match_null_string_p
00000000000000d0 t byte_common_op_match_null_string_p
0000000000000000 t byte_compile_range
0000000000000240 t byte_group_match_null_string_p
00000000000003a0 t byte_re_compile_fastmap
0000000000000000 t byte_re_compile_fastmap.cold
0000000000002c10 t byte_regex_compile
0000000000000060 b byte_reg_unset_dummy
...

Can you please H.J. take a look what can cause that?
Comment 25 H.J. Lu 2020-02-11 13:46:22 UTC
(In reply to Martin Liška from comment #24)
> Ok, I've got one another problem with the 2 commits
> (de66c68d899600286b0d054508a2ed7beee64870 and
> 39f2b43be6ccc3acb29ab84dee48180b6a8fcba5) applied on top of the bintuils
> 2.34 release. I built a openSUSE package and now I see:
> 
> $ nm ./libiberty.a
> 
...

> Can you please H.J. take a look what can cause that?

Works for me on master branch.  Please try master branch to see if
it works for you.
Comment 26 H.J. Lu 2020-02-11 13:47:54 UTC
(In reply to H.J. Lu from comment #25)
> (In reply to Martin Liška from comment #24)
> > Ok, I've got one another problem with the 2 commits
> > (de66c68d899600286b0d054508a2ed7beee64870 and
> > 39f2b43be6ccc3acb29ab84dee48180b6a8fcba5) applied on top of the bintuils
> > 2.34 release. I built a openSUSE package and now I see:
> > 
> > $ nm ./libiberty.a
> > 
> ...
> 
> > Can you please H.J. take a look what can cause that?
> 
> Works for me on master branch.  Please try master branch to see if
> it works for you.

Please upload your libiberty.a here.
Comment 27 Martin Liška 2020-02-11 13:50:14 UTC
> 
> Works for me on master branch.  Please try master branch to see if
> it works for you.

It looks one needs a system setup to have multiple plug-in which cause that:

$ ls /usr/bin/../bin/../lib/bfd-plugins
liblto_plugin.so.0.0.0  LLVMgold.so

Now I put breakpoint here: try_load_plugin
and I see:

Breakpoint 2, try_load_plugin (pname=pname@entry=0x5555555653f0 "/usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0", abfd=abfd@entry=0x555555564a70, has_plugin_p=has_plugin_p@entry=0x7fffffffd97c) at ../../bfd/plugin.c:593
593	{
(gdb) fin
Run till exit from #0  try_load_plugin (pname=pname@entry=0x5555555653f0 "/usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0", abfd=abfd@entry=0x555555564a70, has_plugin_p=has_plugin_p@entry=0x7fffffffd97c) at ../../bfd/plugin.c:593
load_plugin (abfd=0x555555564a70) at ../../bfd/plugin.c:844
844			      if (has_plugin <= 0)
Value returned is $15 = 0

(gdb) list
839			  if (stat (full_name, &st) == 0 && S_ISREG (st.st_mode))
840			    {
841			      int valid_plugin;
842	
843			      found = try_load_plugin (full_name, abfd, &valid_plugin);
844			      if (has_plugin <= 0)
845				has_plugin = valid_plugin;
846			    }
847			  free (full_name);
848			  if (found)
(gdb) n
845				has_plugin = valid_plugin;
(gdb) 
847			  free (full_name);
(gdb) p full_name
$16 = 0x5555555653f0 "/usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0"
(gdb) p current_plugin->plugin_name
$17 = 0x5555555653f0 "/usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0"

So the free for full_name is called, but the pointer is stored in current_plugin->plugin_name. And later compared with..
Comment 28 Martin Liška 2020-02-11 13:50:44 UTC
Created attachment 12286 [details]
libiberty archive
Comment 29 H.J. Lu 2020-02-11 14:41:02 UTC
(In reply to Martin Liška from comment #28)
> Created attachment 12286 [details]
> libiberty archive

I can reproduce it now.  I will fix it.
Comment 30 H.J. Lu 2020-02-11 17:00:22 UTC
Created attachment 12287 [details]
Try this
Comment 31 Martin Liška 2020-02-11 21:10:41 UTC
(In reply to H.J. Lu from comment #30)
> Created attachment 12287 [details]
> Try this

I can confirm that patch works.
Comment 32 H.J. Lu 2020-02-11 23:39:05 UTC
Fixed.
Comment 33 cvs-commit@gcc.gnu.org 2020-02-11 23:39:24 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=22fe7df8c964c23f5760ecf9653af86ede8b5030

commit 22fe7df8c964c23f5760ecf9653af86ede8b5030
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Feb 11 15:36:13 2020 -0800

    Plugin: Treat each object as independent
    
    Since plugin treats each object as independent, we must do a fresh dlopen
    of plugin for each object.
    
    	PR binutils/25355
    	* plugin.c (try_claim): Always clean up for LTO wrapper.
    	(try_load_plugin): Treat each object as independent.  Create a
    	copy for plugin name.
Comment 34 Martin Liška 2020-02-12 10:06:19 UTC
I have one more question. It's a quite common case for me that that I do testing of the built GCC :

$ export PATH=/home/marxin/bin/gcc/bin/:$PATH && export LD_LIBRARY_PATH=/home/marxin/bin/gcc/lib64/:$LD_LIBRARY_PATH
...
but the search path of gcc/lto-wrapper ends up for a location where I have a system compiler:

$ strace -f -s512 nm quotes.o 2>&1 | grep execv
execve("/usr/bin/nm", ["nm", "quotes.o"], 0x7fffffffdcf8 /* 88 vars */) = 0
[pid  7064] execve("/usr/lib64/gcc/x86_64-suse-linux/9/lto-wrapper", ["/usr/lib64/gcc/x86_64-suse-linux/9/lto-wrapper", "@/tmp/ccTEcaUg"], 0x555555567980 /* 90 vars */ <unfinished ...>

which ends badly :/
Any solution for this scenario?
Comment 35 Martin Liška 2020-02-12 10:09:37 UTC
Reopening as it's quite fragile now..
Comment 36 H.J. Lu 2020-02-12 10:33:51 UTC
(In reply to Martin Liška from comment #34)
> I have one more question. It's a quite common case for me that that I do
> testing of the built GCC :
> 
> $ export PATH=/home/marxin/bin/gcc/bin/:$PATH && export
> LD_LIBRARY_PATH=/home/marxin/bin/gcc/lib64/:$LD_LIBRARY_PATH
> ...
> but the search path of gcc/lto-wrapper ends up for a location where I have a
> system compiler:
> 
> $ strace -f -s512 nm quotes.o 2>&1 | grep execv
> execve("/usr/bin/nm", ["nm", "quotes.o"], 0x7fffffffdcf8 /* 88 vars */) = 0
> [pid  7064] execve("/usr/lib64/gcc/x86_64-suse-linux/9/lto-wrapper",
> ["/usr/lib64/gcc/x86_64-suse-linux/9/lto-wrapper", "@/tmp/ccTEcaUg"],
> 0x555555567980 /* 90 vars */ <unfinished ...>
> 
> which ends badly :/
> Any solution for this scenario?

Which liblto_plugin.so did nm load? Which liblto_plugin.so should nm load?
Comment 37 Martin Liška 2020-02-12 10:40:23 UTC
> 
> Which liblto_plugin.so did nm load? Which liblto_plugin.so should nm load?

It loads the following plugin:

stat("/usr/bin/../lib64/bfd-plugins", 0x7fffffffd980) = -1 ENOENT (No such file or directory)
stat("/usr/bin/../bin/../lib/bfd-plugins", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
openat(AT_FDCWD, "/usr/bin/../bin/../lib/bfd-plugins", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 4
stat("/usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0", {st_mode=S_IFREG|0755, st_size=84904, ...}) = 0

The plugins are very stable and the plugin works fine with current GCC master. Problem is that we need to find a corresponding lto-wrapper. It's more challenging..
Comment 38 H.J. Lu 2020-02-12 10:50:11 UTC
(In reply to Martin Liška from comment #37)
> > 
> > Which liblto_plugin.so did nm load? Which liblto_plugin.so should nm load?
> 
> It loads the following plugin:
> 
> stat("/usr/bin/../lib64/bfd-plugins", 0x7fffffffd980) = -1 ENOENT (No such
> file or directory)
> stat("/usr/bin/../bin/../lib/bfd-plugins", {st_mode=S_IFDIR|0755,
> st_size=4096, ...}) = 0
> openat(AT_FDCWD, "/usr/bin/../bin/../lib/bfd-plugins",
> O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 4
> stat("/usr/bin/../bin/../lib/bfd-plugins/liblto_plugin.so.0.0.0",
> {st_mode=S_IFREG|0755, st_size=84904, ...}) = 0
> 
> The plugins are very stable and the plugin works fine with current GCC
> master. Problem is that we need to find a corresponding lto-wrapper. It's
> more challenging..

What is wrong to use the matching lto-wrapper for the plugin being used?
Comment 39 Martin Liška 2020-02-12 10:54:07 UTC
> 
> What is wrong to use the matching lto-wrapper for the plugin being used?

It's probably fine. I'm just wondering how to use a locally install GCC to cooperate fine with binutils.
Comment 40 Martin Liška 2020-02-12 10:56:45 UTC
I will have to start using AR=gcc-ar, ...
Comment 41 Martin Liška 2020-02-12 10:57:56 UTC
Closing again.
Comment 42 H.J. Lu 2020-02-12 12:15:34 UTC
(In reply to Martin Liška from comment #39)
> > 
> > What is wrong to use the matching lto-wrapper for the plugin being used?
> 
> It's probably fine. I'm just wondering how to use a locally install GCC to
> cooperate fine with binutils.

You need to make nm to use the plugin from the GCC you want to use.
Comment 43 Romain Geissler 2020-02-13 14:34:38 UTC
Hi,

Would it be possible to backport the commits required by this bug in the branch 2.34 that was just released ? That would allow the upcoming gcc 10 (which defaults to -fno-common) to work just fine when combined with -flto and the variety of open source projects using auto tools.

Cheers,
Romain
Comment 44 H.J. Lu 2020-02-13 14:44:02 UTC
(In reply to Romain Geissler from comment #43)
> Hi,
> 
> Would it be possible to backport the commits required by this bug in the
> branch 2.34 that was just released ? That would allow the upcoming gcc 10
> (which defaults to -fno-common) to work just fine when combined with -flto
> and the variety of open source projects using auto tools.
> 

FWIW, I backported all my patches to users/hjl/pr25355/binutils-2_34-branch
Comment 45 Romain Geissler 2020-02-13 15:04:40 UTC
@Nick, are you ok to cherry-pick commits from branch users/hjl/pr25355/binutils-2_34-branch in the official 2.34 branch ?
Comment 46 Nick Clifton 2020-02-13 17:16:19 UTC
(In reply to Romain Geissler from comment #45)
> @Nick, are you ok to cherry-pick commits from branch
> users/hjl/pr25355/binutils-2_34-branch in the official 2.34 branch ?

Yes - I will look into it soon.

Cheers
  Nick
Comment 47 cvs-commit@gcc.gnu.org 2020-02-16 11:35:33 UTC
The binutils-2_34-branch branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=804b7fd4fdc545a6ed18aee3d4186574861634ef

commit 804b7fd4fdc545a6ed18aee3d4186574861634ef
Author: Nick Clifton <nickc@redhat.com>
Date:   Sun Feb 16 11:33:15 2020 +0000

    Import fixes for using the LTO plugin with nm.
    
      PR 25355
    
    bfd:
     2020-02-13  H.J. Lu  <hongjiu.lu@intel.com>
    
     * plugin.c (try_load_plugin): Make plugin_list_iter an argument
     and use it if it isn't NULL.  Remove has_plugin_p argument.  Add
     a build_list_p argument.  Don't search plugin_list.  Short circuit
     when building the plugin list.
     (has_plugin): Renamed to has_plugin_list.
     (bfd_plugin_set_plugin): Don't set has_plugin.
     (bfd_plugin_specified_p): Check plugin_list instead.
     (build_plugin_list): New function.
     (load_plugin): Call build_plugin_list and use plugin_list.
    
     2020-02-11  H.J. Lu  <hongjiu.lu@intel.com>
    
     PR binutils/25355
     * plugin.c (try_claim): Always clean up for LTO wrapper.
     (try_load_plugin): Treat each object as independent.  Create a
     copy for plugin name.
    
     2020-02-11  H.J. Lu  <hongjiu.lu@intel.com>
    
     * plugin.c (add_symbols): Clear plugin_data memory.
    
     2020-02-10  H.J. Lu  <hongjiu.lu@intel.com>
    
     PR binutils/25355
     * configure.ac (HAVE_EXECUTABLE_SUFFIX): New AC_DEFINE.
     (EXECUTABLE_SUFFIX): Likewise.
     * config.in: Regenerated.
     * configure: Likewise.
     * plugin.c (bfd_plugin_close_and_cleanup): Removed.
     (plugin_list_entry): Add all_symbols_read, cleanup_handler,
     gcc, lto_wrapper, resolution_file, resolution_option, gcc_env,
     real_bfd, real_nsyms, real_syms, lto_nsyms and lto_syms.
     (get_lto_wrapper): New.
     (setup_lto_wrapper_env): Likewise.
     (current_plugin): Likewise.
     (register_all_symbols_read): Likewise.
     (register_cleanup): Likewise.
     (get_symbols): Likewise.
     (add_input_file): Likewise.
     (bfd_plugin_close_and_cleanup): Likewise.
     (claim_file): Removed.
     (register_claim_file): Set current_plugin->claim_file.
     (add_symbols): Make a copy of LTO symbols.  Set lto_nsyms and
     lto_syms in current_plugin.
     (try_claim): Use current_plugin->claim_file.  Call LTO plugin
     all_symbols_read handler.  Copy real symbols to plugin_data.
     Call LTO plugin cleanup handler.  Clean up for LTO wrapper.
     (try_load_plugin): Don't reuse the previous plugin for LTO
     wrapper.  Set up GCC LTO wrapper if possible.  Don't set
     plugin_list_iter->claim_file.
     (bfd_plugin_canonicalize_symtab): Use real LTO symbols if
     possible.
     * plugin.h (plugin_data_struct): Add real_bfd, real_nsyms and
     real_syms.
    
    ld:
     2020-02-10  H.J. Lu  <hongjiu.lu@intel.com>
    
     PR binutils/25355
     * testsuite/ld-plugin/lto.exp: Run PR binutils/25355 test.
     * testsuite/ld-plugin/pr25355.c: New file.
     * testsuite/ld-plugin/pr25355.d: Likewise.
     * testsuite/lib/ld-lib.exp (run_cc_link_tests): Support compile
     only dump.
Comment 48 Martin Liška 2020-02-18 08:04:46 UTC
Ok, I've just packaged the 2.34 release branch with the backport of this issue.
However, I see the following Invalid free:

$ touch a.c && gcc -c -flto a.c -o a.o
$ touch a.c && gcc -c -flto a.c -o b.o
$ ar r libconsole.a a.o b.o
ar: creating libconsole.a
bfd plugin: could not unlink output file
free(): invalid pointer
Aborted (core dumped)

$ valgrind ar r libconsole.a a.o b.o
==17300== Memcheck, a memory error detector
==17300== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==17300== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==17300== Command: ar r libconsole.a a.o b.o
==17300== 
==17300== Invalid free() / delete / delete[] / realloc()
==17300==    at 0x4839D7B: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==17300==    by 0x4B71929: xrealloc (xmalloc.c:179)
==17300==    by 0x485B1B2: ??? (in /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
==17300==    by 0x4B5E338: try_claim (plugin.c:565)
==17300==    by 0x4B5E338: try_load_plugin (plugin.c:723)
==17300==    by 0x4B5E91B: load_plugin (plugin.c:855)
==17300==    by 0x4B5E91B: bfd_plugin_object_p (plugin.c:868)
==17300==    by 0x49857F5: bfd_check_format_matches (format.c:261)
==17300==    by 0x497E5FC: _bfd_compute_and_write_armap (archive.c:2283)
==17300==    by 0x497EF0B: _bfd_write_archive_contents (archive.c:2147)
==17300==    by 0x498CE09: bfd_close (opncls.c:755)
==17300==    by 0x10E45F: write_archive (ar.c:1240)
==17300==    by 0x10F0CB: replace_members (ar.c:1482)
==17300==    by 0x10BF4C: main (ar.c:887)
==17300==  Address 0x4f568b0 is 0 bytes inside a block of size 8 free'd
==17300==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==17300==    by 0x485ABC3: ??? (in /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
==17300==    by 0x4B5E362: try_claim (plugin.c:574)
==17300==    by 0x4B5E362: try_load_plugin (plugin.c:723)
==17300==    by 0x4B5E91B: load_plugin (plugin.c:855)
==17300==    by 0x4B5E91B: bfd_plugin_object_p (plugin.c:868)
==17300==    by 0x49857F5: bfd_check_format_matches (format.c:261)
==17300==    by 0x497EA17: _bfd_write_archive_contents (archive.c:2127)
==17300==    by 0x498CE09: bfd_close (opncls.c:755)
==17300==    by 0x10E45F: write_archive (ar.c:1240)
==17300==    by 0x10F0CB: replace_members (ar.c:1482)
==17300==    by 0x10BF4C: main (ar.c:887)
==17300==  Block was alloc'd at
==17300==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==17300==    by 0x4B7193F: xrealloc (xmalloc.c:177)
==17300==    by 0x485B1B2: ??? (in /usr/lib64/gcc/x86_64-suse-linux/9/liblto_plugin.so.0.0.0)
==17300==    by 0x4B5E338: try_claim (plugin.c:565)
==17300==    by 0x4B5E338: try_load_plugin (plugin.c:723)
==17300==    by 0x4B5E91B: load_plugin (plugin.c:855)
==17300==    by 0x4B5E91B: bfd_plugin_object_p (plugin.c:868)
==17300==    by 0x49857F5: bfd_check_format_matches (format.c:261)
==17300==    by 0x497EA17: _bfd_write_archive_contents (archive.c:2127)
==17300==    by 0x498CE09: bfd_close (opncls.c:755)
==17300==    by 0x10E45F: write_archive (ar.c:1240)
==17300==    by 0x10F0CB: replace_members (ar.c:1482)
==17300==    by 0x10BF4C: main (ar.c:887)
==17300== 

ar: out of memory allocating 16 bytes after a total of 0 bytes
Comment 49 Martin Liška 2020-02-18 08:49:18 UTC
If I revert backport of 99845b3b77ed1248b6fb94707f88868bde358ccc, then it's fine.
Comment 50 H.J. Lu 2020-02-18 12:27:40 UTC
Created attachment 12296 [details]
A patch
Comment 51 H.J. Lu 2020-02-18 12:28:11 UTC
(In reply to Martin Liška from comment #49)
> If I revert backport of 99845b3b77ed1248b6fb94707f88868bde358ccc, then it's
> fine.

Please try the new patch.
Comment 52 H.J. Lu 2020-02-18 12:32:24 UTC
Created attachment 12297 [details]
A new patch
Comment 53 Martin Liška 2020-02-18 12:47:41 UTC
(In reply to H.J. Lu from comment #52)
> Created attachment 12297 [details]
> A new patch

The patch works for me! Thank you.
Comment 54 Martin Liška 2020-02-18 15:15:41 UTC
However, I see one another segfault:

$ cat 1.i
int a;
$ cat 2.i
[empty file]
$ gcc -flto=auto -O2 -fPIC 1.i -c
$ gcc -flto=auto -O2 -fPIC 2.i -c
$ ar cru x.a 1.o 2.o
$ ranlib x.a
Segmentation fault (core dumped)

$ valgrind ranlib x.a
==423== Memcheck, a memory error detector
==423== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==423== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==423== Command: ranlib x.a
==423== 
==423== Invalid read of size 1
==423==    at 0x498CDF1: bfd_close (opncls.c:753)
==423==    by 0x4B5D76D: bfd_plugin_close_and_cleanup (plugin.c:1074)
==423==    by 0x498CD2A: bfd_close_all_done (opncls.c:789)
==423==    by 0x497C31F: archive_close_worker (archive.c:2788)
==423==    by 0x4B6F38E: htab_traverse_noresize (hashtab.c:775)
==423==    by 0x497F9A4: _bfd_archive_close_and_cleanup (archive.c:2835)
==423==    by 0x498CD2A: bfd_close_all_done (opncls.c:789)
==423==    by 0x10CA11: write_archive (ar.c:1247)
==423==    by 0x10DBA1: ranlib_only.part.0 (ar.c:1498)
==423==    by 0x10BCF1: ranlib_only (ar.c:1492)
==423==    by 0x10BCF1: ranlib_main (ar.c:695)
==423==    by 0x10BCF1: main (ar.c:759)
==423==  Address 0x4f57644 is 68 bytes inside a block of size 280 free'd
==423==    at 0x48389AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==423==    by 0x498CD4E: bfd_close_all_done (opncls.c:797)
==423==    by 0x498CD4E: bfd_close_all_done (opncls.c:785)
==423==    by 0x4B5D76D: bfd_plugin_close_and_cleanup (plugin.c:1074)
==423==    by 0x498CD2A: bfd_close_all_done (opncls.c:789)
==423==    by 0x497C31F: archive_close_worker (archive.c:2788)
==423==    by 0x4B6F38E: htab_traverse_noresize (hashtab.c:775)
==423==    by 0x497F9A4: _bfd_archive_close_and_cleanup (archive.c:2835)
==423==    by 0x498CD2A: bfd_close_all_done (opncls.c:789)
==423==    by 0x10CA11: write_archive (ar.c:1247)
==423==    by 0x10DBA1: ranlib_only.part.0 (ar.c:1498)
==423==    by 0x10BCF1: ranlib_only (ar.c:1492)
==423==    by 0x10BCF1: ranlib_main (ar.c:695)
==423==    by 0x10BCF1: main (ar.c:759)
==423==  Block was alloc'd at
==423==    at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==423==    by 0x4986A41: bfd_malloc (libbfd.c:275)
==423==    by 0x4986BD9: bfd_zmalloc (libbfd.c:360)
==423==    by 0x498C72B: _bfd_new_bfd (opncls.c:62)
==423==    by 0x498C917: bfd_fopen (opncls.c:200)
==423==    by 0x4B5D7B4: add_input_file (plugin.c:389)
==423==    by 0x52E01D1: ???
==423==    by 0x4B5E3C3: try_claim (plugin.c:564)
==423==    by 0x4B5E3C3: try_load_plugin (plugin.c:710)
==423==    by 0x4B5E8EB: load_plugin (plugin.c:848)
==423==    by 0x4B5E8EB: bfd_plugin_object_p (plugin.c:861)
==423==    by 0x49857F5: bfd_check_format_matches (format.c:261)
==423==    by 0x497EA17: _bfd_write_archive_contents (archive.c:2127)
==423==    by 0x498CE09: bfd_close (opncls.c:755)
Comment 55 H.J. Lu 2020-02-18 16:10:47 UTC
Created attachment 12299 [details]
An updated patch
Comment 56 H.J. Lu 2020-02-18 16:11:30 UTC
(In reply to Martin Liška from comment #48)
> Ok, I've just packaged the 2.34 release branch with the backport of this
> issue.
> However, I see the following Invalid free:
> 
> $ touch a.c && gcc -c -flto a.c -o a.o
> $ touch a.c && gcc -c -flto a.c -o b.o
> $ ar r libconsole.a a.o b.o
> ar: creating libconsole.a
> bfd plugin: could not unlink output file
> free(): invalid pointer
> Aborted (core dumped)
> 

Try the latest one.
Comment 57 Martin Liška 2020-02-18 16:21:28 UTC
> Try the latest one.

I can confirm it works.
Comment 58 cvs-commit@gcc.gnu.org 2020-02-19 11:51:57 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=dcf06b89b9129da6988878a77afdd02d3acc2e30

commit dcf06b89b9129da6988878a77afdd02d3acc2e30
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Feb 19 03:29:51 2020 -0800

    plugin: Call dlclose before return in try_load_plugin
    
    Since plugin can be used only once in try_load_plugin, call dlclose
    before return.
    
    	PR binutils/25355
    	* plugin.c (plugin_list_entry): Remove handle.
    	(try_load_plugin): Call dlclose before return.
Comment 59 H.J. Lu 2020-02-19 12:31:48 UTC
Fixed on master branch.
Comment 60 Martin Liška 2020-02-19 13:06:32 UTC
(In reply to H.J. Lu from comment #59)
> Fixed on master branch.

Good. Please pull the revision to the 2.34 branch.
Comment 61 H.J. Lu 2020-02-19 17:02:27 UTC
Also fixed on 2.34 branch.
Comment 62 cvs-commit@gcc.gnu.org 2020-02-19 17:02:29 UTC
The binutils-2_34-branch branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e2b46ba142d9901897d8189422f0bcc28e5660b8

commit e2b46ba142d9901897d8189422f0bcc28e5660b8
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Feb 19 03:29:51 2020 -0800

    plugin: Call dlclose before return in try_load_plugin
    
    Since plugin can be used only once in try_load_plugin, call dlclose
    before return.
    
    	PR binutils/25355
    	* plugin.c (plugin_list_entry): Remove handle.
    	(try_load_plugin): Call dlclose before return.
    
    (cherry picked from commit dcf06b89b9129da6988878a77afdd02d3acc2e30)
Comment 63 Martin Liška 2020-02-21 11:04:34 UTC
I have one more question about the lto-wrapper usage: is there any reason why 'ar' and 'ranlib' also use it? It makes building of some packages significantly slower.
Comment 64 H.J. Lu 2020-02-21 11:35:12 UTC
(In reply to Martin Liška from comment #63)
> I have one more question about the lto-wrapper usage: is there any reason
> why 'ar' and 'ranlib' also use it? It makes building of some packages
> significantly slower.

Please open a new bug.
Comment 65 Martin Liška 2020-02-21 11:42:13 UTC
> 
> Please open a new bug.

Sure:
https://sourceware.org/bugzilla/show_bug.cgi?id=25584
Comment 66 cvs-commit@gcc.gnu.org 2020-03-20 10:57:19 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3d98c46092341c1373d960d0a66ca502d5b7ee7f

commit 3d98c46092341c1373d960d0a66ca502d5b7ee7f
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Fri Mar 20 03:55:17 2020 -0700

    plugin: Don't invoke LTO-wrapper
    
    Don't invoke LTO-wrapper since the LTO wrapper approach is not only
    slow but also unreliable.  For GCC 10 or newer, LDPT_ADD_SYMBOLS_V2
    will be used.
    
    bfd/
    
            * configure.ac (HAVE_EXECUTABLE_SUFFIX): Removed.
            (EXECUTABLE_SUFFIX): Likewise.
            * config.in: Regenerated.
            * configure: Likewise.
            * plugin.c (bfd_plugin_close_and_cleanup): Defined as
            _bfd_generic_close_and_cleanup.
            (plugin_list_entry): Remove resolution_file, resolution_option,
            real_bfd, real_nsyms, real_syms, lto_nsyms, lto_syms, gcc,
            lto_wrapper, gcc_env and initialized,
            (need_lto_wrapper_p): Removed.
            (get_lto_wrapper): Likewise.
            (setup_lto_wrapper_env): Likewise.
            (register_all_symbols_read): Likewise.
            (egister_cleanup): Likewise.
            (get_symbols): Likewise.
            (add_input_file): Likewise.
            (bfd_plugin_set_program_name): Remove need_lto_wrapper.
            (add_symbols): Updated.
            (try_claim): Likewise.
            (try_load_plugin): Likewise.
            (bfd_plugin_canonicalize_symtab): Likewise.
            * plugin.h (bfd_plugin_set_program_name): Remove int argument.
            (plugin_data_struct): Remove real_bfd, real_nsyms and real_syms.
    
    binutils/
    
            * ar.c (main): Update bfd_plugin_set_program_name call.
            * nm.c (main): Likewise.
    
    ld/
    
            * testsuite/ld-plugin/lto.exp (lto_link_tests): Run PR ld/25355
            test only for GCC 10 or newer.
Comment 67 cvs-commit@gcc.gnu.org 2020-03-25 14:10:35 UTC
The binutils-2_34-branch branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=23820109ced73d231b69ea6fa9a85b743d7843c8

commit 23820109ced73d231b69ea6fa9a85b743d7843c8
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Mar 25 06:58:19 2020 -0700

    plugin: Don't invoke LTO-wrapper
    
    Don't invoke LTO-wrapper since the LTO wrapper approach is not only
    slow but also unreliable.  For GCC 10 or newer, LDPT_ADD_SYMBOLS_V2
    will be used.
    
    bfd/
    
            * configure.ac (HAVE_EXECUTABLE_SUFFIX): Removed.
            (EXECUTABLE_SUFFIX): Likewise.
            * config.in: Regenerated.
            * configure: Likewise.
            * plugin.c (bfd_plugin_close_and_cleanup): Defined as
            _bfd_generic_close_and_cleanup.
            (plugin_list_entry): Remove resolution_file, resolution_option,
            real_bfd, real_nsyms, real_syms, lto_nsyms, lto_syms, gcc,
            lto_wrapper, gcc_env and initialized,
            (need_lto_wrapper_p): Removed.
            (get_lto_wrapper): Likewise.
            (setup_lto_wrapper_env): Likewise.
            (register_all_symbols_read): Likewise.
            (egister_cleanup): Likewise.
            (get_symbols): Likewise.
            (add_input_file): Likewise.
            (bfd_plugin_set_program_name): Remove need_lto_wrapper.
            (add_symbols): Updated.
            (try_claim): Likewise.
            (try_load_plugin): Likewise.
            (bfd_plugin_canonicalize_symtab): Likewise.
            * plugin.h (bfd_plugin_set_program_name): Remove int argument.
            (plugin_data_struct): Remove real_bfd, real_nsyms and
            real_syms.
    
    binutils/
    
            * ar.c (main): Update bfd_plugin_set_program_name call.
            * nm.c (main): Likewise.
    
    ld/
    
            * testsuite/ld-plugin/lto.exp (lto_link_tests): Run PR ld/25355
            test only for GCC 10 or newer.
    
    (cherry picked from commit 3d98c46092341c1373d960d0a66ca502d5b7ee7f)