Bug 23309 - ld.bfd -u option to force symbol discards symbol when used with LTO plugin and the symbol has hidden or internal visibility
Summary: ld.bfd -u option to force symbol discards symbol when used with LTO plugin an...
Status: VERIFIED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.31
: P2 normal
Target Milestone: 2.31
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-18 16:35 UTC by zenith432
Modified: 2018-07-05 11:36 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Proposed patch (678 bytes, patch)
2018-06-20 06:44 UTC, zenith432
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zenith432 2018-06-18 16:35:57 UTC
Test Case:

test.c
==========
#include <stdio.h>

int main(int argc, char** argv)
{
	printf("Hello World\n");
	return 0;
}

void KeepMe(void)
{
	__asm__ volatile ( ".asciz \"This string should appear in the executable.\"" );
}
==========

compile with

gcc -o test -flto -fvisibility=hidden -ffunction-sections -save-temps -fuse-ld=bfd -Wl,--gc-sections,--print-gc-sections,-u,KeepMe test.c
strings -a test | grep This

Output:
/usr/bin/ld.bfd: Removing unused section '.rodata.cst4' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o'
/usr/bin/ld.bfd: Removing unused section '.data' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o'
/usr/bin/ld.bfd: Removing unused section '.rodata' in file '/usr/lib/gcc/x86_64-redhat-linux/8/crtbegin.o'
/usr/bin/ld.bfd: Removing unused section '.text.KeepMe' in file 'test.ltrans0.ltrans.o'

Notice that KeepMe is discarded and does not appear in the output file.

Save the file test.ltrans0.s as test.bfd.s

Now compile with

gcc -o test -flto -fvisibility=hidden -ffunction-sections -save-temps -fuse-ld=gold -Wl,--gc-sections,--print-gc-sections,-u,KeepMe test.c
strings -a test | grep This

(Using gold instead of bfd)

Output:
/usr/bin/ld.gold: removing unused section from '.rodata.cst4' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o'
/usr/bin/ld.gold: removing unused section from '.data' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o'
/usr/bin/ld.gold: removing unused section from '.bss' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o'
/usr/bin/ld.gold: removing unused section from '.text' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crti.o'
/usr/bin/ld.gold: removing unused section from '.data' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crti.o'
/usr/bin/ld.gold: removing unused section from '.bss' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crti.o'
/usr/bin/ld.gold: removing unused section from '.data' in file '/usr/lib/gcc/x86_64-redhat-linux/8/crtbegin.o'
/usr/bin/ld.gold: removing unused section from '.rodata' in file '/usr/lib/gcc/x86_64-redhat-linux/8/crtbegin.o'
/usr/bin/ld.gold: removing unused section from '.data' in file '/usr/lib64/libc_nonshared.a(elf-init.oS)'
/usr/bin/ld.gold: removing unused section from '.bss' in file '/usr/lib64/libc_nonshared.a(elf-init.oS)'
/usr/bin/ld.gold: removing unused section from '.text' in file '/usr/lib/gcc/x86_64-redhat-linux/8/crtend.o'
/usr/bin/ld.gold: removing unused section from '.data' in file '/usr/lib/gcc/x86_64-redhat-linux/8/crtend.o'
/usr/bin/ld.gold: removing unused section from '.bss' in file '/usr/lib/gcc/x86_64-redhat-linux/8/crtend.o'
/usr/bin/ld.gold: removing unused section from '.text' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crtn.o'
/usr/bin/ld.gold: removing unused section from '.data' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crtn.o'
/usr/bin/ld.gold: removing unused section from '.bss' in file '/usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crtn.o'
/usr/bin/ld.gold: removing unused section from '.text' in file 'test.ltrans0.ltrans.o'
/usr/bin/ld.gold: removing unused section from '.data' in file 'test.ltrans0.ltrans.o'
/usr/bin/ld.gold: removing unused section from '.bss' in file 'test.ltrans0.ltrans.o'
This string should appear in the executable.

So KeepMe is in the executable as it should be.

Rename test.ltrans0.s to test.gold.s

and then... diff test.bfd.s test.gold.s
30a31,32
> 	.globl	KeepMe
> 	.hidden	KeepMe
Comment 1 zenith432 2018-06-19 10:06:56 UTC
This looks strongly connected to this

https://sourceware.org/bugzilla/show_bug.cgi?id=22983

and that same function
is_visible_from_outside
in ld/plugin.c
Comment 2 zenith432 2018-06-19 16:01:40 UTC
So here's what's going...

In ld/plugin.c (ld.bfd)... the following code in get_symbols_v2
=====
      if (res == LDPR_PREVAILING_DEF_IRONLY)
        {
          /* We need to know if the sym is referenced from non-IR files.  Or
             even potentially-referenced, perhaps in a future final link if
             this is a partial one, perhaps dynamically at load-time if the
             symbol is externally visible.  */
          if (blhe->non_ir_ref_regular)
            res = LDPR_PREVAILING_DEF;
          else if (is_visible_from_outside (&syms[n], blhe))
            res = def_ironly_exp;
        }
=====
For symbol 'main', no_ir_ref_regular is true, so it sets resolution of LDPR_PREVAILING_DEF and the symbol is handled correctly.
For symbol 'KeepMe', no_ir_ref_regular is false, and it calls is_visible_from_outside(), which return true because KeepMe is in the list of entry points.  The makes it set a resolution of LDPR_PREVAILING_DEF_IRONLY_EXP (the value of def_ironly_exp) - which is what cases KeepMe to be incorrectly converted from "global hidden" to "local".

And in gold?
=====
            {
              if (is_referenced_from_outside(lsym))
                res = LDPR_PREVAILING_DEF;
              else if (is_visible_from_outside(lsym))
                res = ldpr_prevailing_def_ironly_exp;
              else
                res = LDPR_PREVAILING_DEF_IRONLY;
            }
=====
is in function Pluginobj::get_symbol_resolution_info.  The function is_referenced_from_outside returns true for both "main", and "KeepMe", which sets a resolution of LDPR_PREVAILING_DEF for both of them getting them to be handled correctly.
In is_referenced_from_outside, lsym->in_real_elf returns true for "main"
  and parameters->options().is_undefined(lsym->name()) returns true for "KeepMe".

To resolve this, the logic in ld.bfd needs to be changed so that both symbols end up with a resolution of LDPR_PREVAILING_DEF.
Comment 3 zenith432 2018-06-20 06:44:22 UTC
Created attachment 11088 [details]
Proposed patch
Comment 4 Sourceware Commits 2018-07-04 13:30:04 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=94d401b8b88a76b1372ce44e805516756a4f737b

commit 94d401b8b88a76b1372ce44e805516756a4f737b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Jul 4 06:27:58 2018 -0700

    ld: Set non_ir_ref_regular for --undefined SYMBOL
    
    Set non_ir_ref_regular to TRUE for symbols forced into the output file
    so that they won't be removed by garbage collection with LTO.
    
    	PR ld/23309
    	* ldlang.c (insert_undefined): Set non_ir_ref_regular to TRUE.
    	* plugin.c (is_visible_from_outside): Don't scan entry_symbol.
    	* testsuite/ld-plugin/pr23309.c: New file.
    	* testsuite/ld-plugin/pr23309.d: Likewise.
Comment 5 H.J. Lu 2018-07-04 13:35:07 UTC
Fixed for 2.32.
Comment 6 zenith432 2018-07-04 14:06:53 UTC
verified fix from commit 94d401b8.
Comment 7 Sourceware Commits 2018-07-05 11:35:39 UTC
The binutils-2_31-branch branch has been updated by H.J. Lu <hjl@sourceware.org>:

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

commit 2c4995ba2b4b1182ce3b228152f697c55a0ff9fb
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Jul 5 04:33:11 2018 -0700

    ld: Set non_ir_ref_regular for --undefined SYMBOL
    
    Set non_ir_ref_regular to TRUE for symbols forced into the output file
    so that they won't be removed by garbage collection with LTO.
    
    	PR ld/23309
    	* ldlang.c (insert_undefined): Set non_ir_ref_regular to TRUE.
    	* plugin.c (is_visible_from_outside): Don't scan entry_symbol.
    	* testsuite/ld-plugin/pr23309.c: New file.
    	* testsuite/ld-plugin/pr23309.d: Likewise.
    
    (cherry picked from commit 94d401b8b88a76b1372ce44e805516756a4f737b)
Comment 8 H.J. Lu 2018-07-05 11:36:07 UTC
Also fixed for 2.31.