Please merge linker bugfix into Ubuntu 20.04 LTS

[Barry M Tannenbaum]

Binutils bug https://sourceware.org/bugzilla/show_bug.cgi?id=26262 fixes a problem that Intel Fortran is running into when using the -ipo option to request link time optimization. Please merge the fix into Ubuntu 20.04 LTS

[D-i-j (d-i-j)]

This can be reproduced with clang 10 (likely also 9; and all master commits before https://reviews.llvm.org/D83967 ). LLVMgold.so is needed.

% cat a.c
int main() {}
% /tmp/Debug/bin/clang -fuse-ld=bfd -fprofile-generate -flto a.c -Wl,-plugin-opt=save-temps -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps
/usr/bin/ld.bfd: /tmp/a-e575a9.o (symbol from plugin): definition of __llvm_profile_raw_version
/usr/bin/ld.bfd: /tmp/Debug/lib/clang/12.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version

/tmp/a-e575a9.o (symbol from plugin) provides a definition in a prevailing comdat group.
libclang_rt.profile-x86_64.a(InstrProfiling.c.o) is an weak definition which should not override the IR definition.

% cat a.out.resolution.txt
/tmp/a-e575a9.o
-r=/tmp/a-e575a9.o,main,plx
-r=/tmp/a-e575a9.o,__llvm_profile_raw_version,l # should be 'plx' instead of 'l'
-r=/tmp/a-e575a9.o,__llvm_profile_filename,plx

Gold and LLD are correct.

% /tmp/Debug/bin/clang -fuse-ld=gold -fprofile-generate -flto a.c -Wl,-plugin-opt=save-temps -Wl,-y,__llvm_profile_raw_version
/tmp/a-668c9c.o: definition of __llvm_profile_raw_version
/tmp/Debug/lib/clang/12.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
a.out.o: definition of __llvm_profile_raw_version
% cat a.out.resolution.txt
/tmp/a-668c9c.o
-r=/tmp/a-668c9c.o,main,plx
-r=/tmp/a-668c9c.o,__llvm_profile_raw_version,plx
-r=/tmp/a-668c9c.o,__llvm_profile_filename,plx

% /tmp/Debug/bin/clang -fuse-ld=lld -fprofile-generate -flto a.c -Wl,-plugin-opt=save-temps -Wl,-y,__llvm_profile_raw_version
/tmp/a-afb841.o: definition of __llvm_profile_raw_version
<internal>: reference to __llvm_profile_raw_version
lto.tmp: definition of __llvm_profile_raw_version
% cat a.out.resolution.txt
/tmp/a-afb841.o
-r=/tmp/a-afb841.o,main,plx
-r=/tmp/a-afb841.o,__llvm_profile_raw_version,plx
-r=/tmp/a-afb841.o,__llvm_profile_filename,plx

The ld bug is in ld/plugin.c:plugin_notice . ld somehow drops bfd_link_hash_defined (provided by "/tmp/a-e575a9.o (symbol from plugin)") in favor of bfd_link_hash_undefweak. Then bfd_link_hash_undefweak is overridden by the weak definition in libclang_rt.profile-x86_64.a(InstrProfiling.c.o)

      /* Otherwise, it must be a new def.
  Ensure any symbol defined in an IR dummy BFD takes on a
  new value from a real BFD. Weak symbols are not normally
  overridden by a new weak definition, and strong symbols
  will normally cause multiple definition errors. Avoid
  this by making the symbol appear to be undefined. */
      else if (((h->type == bfd_link_hash_defweak
   || h->type == bfd_link_hash_defined)
  && is_ir_dummy_bfd (sym_bfd = h->u.def.section->owner))
        || (h->type == bfd_link_hash_common
     && is_ir_dummy_bfd (sym_bfd = h->u.c.p->section->owner)))
 {
   h->type = bfd_link_hash_undefweak;
   h->u.undef.abfd = sym_bfd;
 }

The intended behavior is to let "/tmp/a-e575a9.o (symbol from plugin)" provide the definition.

[Hjl-tools (hjl-tools)]

clang has different behavior depending on -fuse-ld=XXX:

[hjl@gnu-cfl-2 pr26262]$ clang -fuse-ld=gold -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -fprofile-generate -flto -o a a.c -save-temps
a.o: definition of __llvm_profile_raw_version
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
a.o: definition of __llvm_profile_raw_version
[hjl@gnu-cfl-2 pr26262]$ clang -fuse-ld=bfd -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -fprofile-generate -flto -o a a.o
/usr/bin/ld.bfd: a.o: definition of __llvm_profile_raw_version
[hjl@gnu-cfl-2 pr26262]$ clang -fuse-ld=bfd -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -fprofile-generate -flto -o a a.c -save-temps
/usr/bin/ld.bfd: a.o (symbol from plugin): definition of __llvm_profile_raw_version
/usr/bin/ld.bfd: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
[hjl@gnu-cfl-2 pr26262]$ clang -fuse-ld=gold -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -fprofile-generate -flto -o a a.o
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
[hjl@gnu-cfl-2 pr26262]$ rm a.o
[hjl@gnu-cfl-2 pr26262]$ clang -fprofile-generate -flto -c a.c
[hjl@gnu-cfl-2 pr26262]$ clang -fuse-ld=bfd -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -fprofile-generate -flto -o a a.o
/usr/bin/ld.bfd: a.o (symbol from plugin): definition of __llvm_profile_raw_version
/usr/bin/ld.bfd: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
[hjl@gnu-cfl-2 pr26262]$ clang -fuse-ld=gold -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -fprofile-generate -flto -o a a.o
/usr/lib64/clang/10.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
[hjl@gnu-cfl-2 pr26262]$

It looks like a clang bug.

[D-i-j (d-i-j)]

(In reply to H.J. Lu from comment #1)
> clang has different behavior depending on -fuse-ld=XXX:
>
> ...
>
> It looks like a clang bug.

Thanks for the report. It is an LLVMgold.so bug. LLVMgold.so is used by GNU ld and gold for clang LTO (full LTO or ThinLTO). The problem is that

clang -fuse-ld=bfd -flto a.o -o a -Wl,-plugin-opt=save-temps # override a.o

I will fix it in https://reviews.llvm.org/D84132 and cherry pick it into llvm-project's release/11.x branch (newly created for the 11.0.0 release).

Let's move back to the original topic. Use a different output filename (than the basename of the source file) to avoid the -plugin-opt=save-temps issue.

-----

% clang -fuse-ld=bfd -fprofile-generate=out -flto a.c -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -o aa
/usr/bin/ld.bfd: /tmp/a-1cd442.o (symbol from plugin): definition of __llvm_profile_raw_version
/usr/bin/ld.bfd: /tmp/RelA/lib/clang/12.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
% ./aa
% llvm-profdata merge out -o a.profdata
% llvm-profdata show a.profdata
Instrumentation level: Front-end ### incorrect, it should be IR
Total functions: 1
Maximum function count: 1
Maximum internal block count: 0

% rm -r out
% clang -fuse-ld=gold -fprofile-generate=out -flto a.c -Wl,-y,__llvm_profile_raw_version -Wl,-plugin-opt=save-temps -o aa
/tmp/a-3b2969.o: definition of __llvm_profile_raw_version
/tmp/RelA/lib/clang/12.0.0/lib/linux/libclang_rt.profile-x86_64.a(InstrProfiling.c.o): definition of __llvm_profile_raw_version
aa.o: definition of __llvm_profile_raw_version
% ./aa
% llvm-profdata merge out -o a.profdata
% llvm-profdata show a.profdata
Instrumentation level: IR
Total functions: 1
Maximum function count: 1
Maximum internal block count: 0

----

As my first comment says, you can inspect aa.resolution.txt to see that GNU ld incorrectly thinks that __llvm_profile_raw_versio is non-prevailing.

[Hjl-tools (hjl-tools)]

Created attachment 12713
A patch

[Hjl-tools (hjl-tools)]

[hjl@gnu-cfl-2 pr26262]$ cat pr26262a.c
#include <stdio.h>

int counter;
extern void foo (void);
extern void bar (void);
extern void xxx (void);

int
main(void)
{
  bar ();
  foo ();
  xxx ();
  if (counter == 1)
    printf ("PASS\n");
  return 0;
}
[hjl@gnu-cfl-2 pr26262]$ cat pr26262b.c
__attribute__ ((noinline))
void
bar (void)
{
}
[hjl@gnu-cfl-2 pr26262]$ cat pr26262c.c
#include <stdlib.h>

extern int counter;

void
foo (void)
{
  counter++;
}

__attribute__((weak))
void
bar (void)
{
  abort ();
}
[hjl@gnu-cfl-2 pr26262]$ cat pr26262d.c
extern void bar (void);
void
xxx (void)
{
  bar ();
}
[hjl@gnu-cfl-2 pr26262]$ make
gcc -O2 -flto -c -o pr26262a.o pr26262a.c
gcc -O2 -flto -c -o pr26262b.o pr26262b.c
gcc -O2 -c -o pr26262c.o pr26262c.c
gcc -O2 -c -o pr26262d.o pr26262d.c
gcc -fuse-ld=bfd -O2 -o x pr26262a.o pr26262b.o pr26262c.o pr26262d.o
./x
make: *** [Makefile:11: all] Aborted (core dumped)
[hjl@gnu-cfl-2 pr26262]$

[Hjl-tools (hjl-tools)]

A patch is posted at

https://sourceware.org/pipermail/binutils/2020-July/112454.html

[Hjl-tools (hjl-tools)]

(In reply to H.J. Lu from comment #5)
> A patch is posted at
>
> https://sourceware.org/pipermail/binutils/2020-July/112454.html

Fix isn't complete, see PR 26267.

[Cvs-commit (cvs-commit)]

The master branch has been updated by H.J. Lu <email address hidden>:

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

commit 0e6a3f07f50723d1831291492b96fdf74bcbdc11
Author: H.J. Lu <email address hidden>
Date: Wed Jul 22 03:49:07 2020 -0700

    ld: Properly override the IR definition

    We change the previous definition in the IR object to undefweak only
    after all LTO symbols have been read.

    include/

            PR ld/26262
            PR ld/26267
            * bfdlink.h (bfd_link_info): Add lto_all_symbols_read.

    ld/

            PR ld/26262
            PR ld/26267
            * ldlang.c (lang_process): Set lto_all_symbols_read after all
            LTO IR symbols have been read.
            * plugin.c (plugin_notice): Override the IR definition only if
            all LTO IR symbols have been read or the new definition is
            non-weak and the the IR definition is weak
            * testsuite/ld-plugin/lto.exp: Run PR ld/26262 and ld/26267
            tests.
            * testsuite/ld-plugin/pr26262a.c: New file.
            * testsuite/ld-plugin/pr26262b.c: Likewise.
            * testsuite/ld-plugin/pr26262c.c: Likewise.
            * testsuite/ld-plugin/pr26267.err: Likewise.
            * testsuite/ld-plugin/pr26267a.c: Likewise.
            * testsuite/ld-plugin/pr26267b.c: Likewise.
            * testsuite/ld-plugin/pr26267c.c: Likewise.

[Hjl-tools (hjl-tools)]

Fixed for 2.36 so far.

[Cvs-commit (cvs-commit)]

The binutils-2_35-branch branch has been updated by H.J. Lu <email address hidden>:

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

commit 041d1c2d4f03a134cb7574e4d62d2ce4418503ff
Author: H.J. Lu <email address hidden>
Date: Wed Jul 22 03:49:07 2020 -0700

    ld: Properly override the IR definition

    We change the previous definition in the IR object to undefweak only
    after all LTO symbols have been read.

    include/

            PR ld/26262
            PR ld/26267
            * bfdlink.h (bfd_link_info): Add lto_all_symbols_read.

    ld/

            PR ld/26262
            PR ld/26267
            * ldlang.c (lang_process): Set lto_all_symbols_read after all
            LTO IR symbols have been read.
            * plugin.c (plugin_notice): Override the IR definition only if
            all LTO IR symbols have been read or the new definition is
            non-weak and the the IR definition is weak
            * testsuite/ld-plugin/lto.exp: Run PR ld/26262 and ld/26267
            tests.
            * testsuite/ld-plugin/pr26262a.c: New file.
            * testsuite/ld-plugin/pr26262b.c: Likewise.
            * testsuite/ld-plugin/pr26262c.c: Likewise.
            * testsuite/ld-plugin/pr26267.err: Likewise.
            * testsuite/ld-plugin/pr26267a.c: Likewise.
            * testsuite/ld-plugin/pr26267b.c: Likewise.
            * testsuite/ld-plugin/pr26267c.c: Likewise.

    (cherry picked from commit 0e6a3f07f50723d1831291492b96fdf74bcbdc11)

[Hjl-tools (hjl-tools)]

Fixed for 2.35.

[Dimitri John Ledkov (xnox)]

Barry M Tannenbaum, btw you can also raise issues via /intel project. You could also join ~intel-team to work together with others that escalate and work on Intel enablement via typical channels.

Is this bugfix on the 2.34 branch?

After full archive rebuild we are planning to SRU binutils from the 2.34 stable branch.

[Brian Murray (brian-murray)]

From #ubuntu-meeting on 2020-11-12

08:26 < doko> that binutils fix is not yet on the 2.34
              branch

[Steve Langasek (vorlon)]

blocked on this being in the 2.34 branch upstream