[ELF] Default to --no-fortran-common

[MaskRay]

Summary

D86142 introduced --fortran-common and defaulted it to true (matching GNU ld
but deviates from gold/macOS ld64). The default state was motivated by transparently
supporting some FORTRAN 77 programs (Fortran 90 deprecated common blocks).
Now I think it again. I believe we made a mistake to change the default:

• this is a weird and legacy rule, though the breakage is very small
• --fortran-common introduced complexity to parallel symbol resolution and will slow down it
• --fortran-common more likely causes issues when users mix COMMON and STB_GLOBAL definitions (see https://github.com/llvm/llvm-project/issues/48570 and https://maskray.me/blog/2022-02-06-all-about-common-symbols). I have seen several issues in our internal projects and Android. On the other hand, --no-fortran-common is safer since COMMON/STB_GLOBAL have the same semantics related to archive member extraction.

Therefore I think we should switch back, not punishing the common uage.
A platform wanting --fortran-common can implement ld.lld as a shell script
wrapper around lld -flavor gnu --fortran-common "$@".

[ikudrin]

If I understand it right, --fortran-common is useful in rare scenarios but can slow down linking when handling COMMON symbols, and it also hinders implementing the parallel symbol resolution. If that is true, I agree with the change.

[MaskRay]

In D122450#3412334, @ikudrin wrote:

If I understand it right, --fortran-common is useful in rare scenarios but can slow down linking when handling COMMON symbols, and it also hinders implementing the parallel symbol resolution. If that is true, I agree with the change.

The understanding is right.

If we implement parallel symbol resolution, we probably have two code paths for a while. Not wasting efforts on implementing --fortran-common for the parallel code path helps.
(If we ever are able to drop the serial code path, we may need to implement --fortran-common. But the code path can be disabled for most use cases to not cause slowdown.)

[ikudrin]

LGTM then

[peter.smith]

Apologies for delay in responding, been OoO for a bit. I'm happy with the change. Do we have enough people that commented in favour of D86142 in the reviewer/subscriber list? If only to make sure they are aware of the change.

[MaskRay]

In D122450#3414088, @peter.smith wrote:

Apologies for delay in responding, been OoO for a bit. I'm happy with the change. Do we have enough people that commented in favour of D86142 in the reviewer/subscriber list? If only to make sure they are aware of the change.

Sent a comment https://reviews.llvm.org/D86142#3414187

[sfertile]

LGTM.

lld/ELF/Driver.cpp

config->exportDynamic =
       args.hasArg(OPT_shared);
   config->filterList = args::getStrings(args, OPT_filter);
   config->fini = args.getLastArgValue(OPT_fini, "_fini");
   config->fixCortexA53Errata843419 = args.hasArg(OPT_fix_cortex_a53_843419) &&
                                      !args.hasArg(OPT_relocatable);
   config->fixCortexA8 =
       args.hasArg(OPT_fix_cortex_a8) && !args.hasArg(OPT_relocatable);
   config->fortranCommon =
-      args.hasFlag(OPT_fortran_common, OPT_no_fortran_common, true);
+      args.hasFlag(OPT_fortran_common, OPT_no_fortran_common, false);
   config->gcSections = args.hasFlag(OPT_gc_sections, OPT_no_gc_sections, false);
   config->gnuUnique = args.hasFlag(OPT_gnu_unique, OPT_no_gnu_unique, true);
   config->gdbIndex = args.hasFlag(OPT_gdb_index, OPT_no_gdb_index, false);
   config->icf = getICF(args);
   config->ignoreDataAddressEquality =
       args.hasArg(OPT_ignore_data_address_equality);
   config->ignoreFunctionAddressEquality =
       args.hasArg(OPT_ignore_function_address_equality);

lld/docs/ReleaseNotes.rst

 Non-comprehensive list of changes in this release
 =================================================
 
 ELF Improvements
 ----------------
 
 * ``-z pack-relative-relocs`` is now available to support ``DT_RELR`` for glibc 2.36+.
   (`D120701 <https://reviews.llvm.org/D120701>`_)
+* ``--no-fortran-common`` (pre 12.0.0 behavior) is now the default.
 
 Breaking changes
 ----------------
 
 * The GNU ld incompatible ``--no-define-common`` has been removed.
 * The obscure ``-dc``/``-dp`` options have been removed.
 * ``-d`` is now ignored.
 * If a prevailing COMDAT group defines STB_WEAK symbol, having a STB_GLOBAL symbol in a non-prevailing group is now rejected with a diagnostic.

lld/test/ELF/common-archive-lookup.s

 ## Bitcode files.
 # RUN: llvm-as -o 1.bc commonblock.ll
 # RUN: llvm-as -o 2.bc blockdata.ll
 # RUN: llvm-as -o 3.bc weak.ll
 
 ## Bitcode archive.
 # RUN: llvm-ar crs 4.a 1.bc 2.bc
 
-# RUN: ld.lld -o 1 main.o 1.a
+# RUN: ld.lld -o 1 main.o 1.a --fortran-common
 # RUN: llvm-objdump -D -j .data 1 | FileCheck --check-prefix=TEST1 %s
 
-# RUN: ld.lld -o 2 main.o --start-lib 1.o strong_data_only.o --end-lib
+# RUN: ld.lld -o 2 main.o --start-lib 1.o strong_data_only.o --end-lib --fortran-common
 # RUN: llvm-objdump -D -j .data 2 | FileCheck --check-prefix=TEST1 %s
 
 # RUN: ld.lld -o 3 main.o 2.a
 # RUN: llvm-objdump -t 3 | FileCheck --check-prefix=BSS %s
 
 # RUN: ld.lld -o 4 main.o --start-lib 1.o weak_data_only.o --end-lib
 # RUN: llvm-objdump -t 4 | FileCheck --check-prefix=BSS %s
 
 # RUN: ld.lld -o 5 main.o 3.a --print-map | FileCheck --check-prefix=MAP %s
 
 # RUN: ld.lld -o 6 main.o 2.o 1.a
 # RUN: llvm-objdump -D -j .data 6 | FileCheck --check-prefix=TEST2 %s
 
 # RUN: ld.lld -o 7 main.o 2.o --start-lib 1.o strong_data_only.o --end-lib
 # RUN: llvm-objdump -D -j .data 7 | FileCheck --check-prefix=TEST2 %s
 
-# RUN: not ld.lld -o 8 main.o 1.a strong_data_only.o 2>&1 | \
+# RUN: not ld.lld -o 8 main.o 1.a strong_data_only.o --fortran-common 2>&1 | \
 # RUN:   FileCheck --check-prefix=ERR %s
 
-# RUN: not ld.lld -o 9 main.o --start-lib 1.o 2.o --end-lib  strong_data_only.o 2>&1 | \
+# RUN: not ld.lld -o 9 main.o --start-lib 1.o 2.o --end-lib  strong_data_only.o --fortran-common 2>&1 | \
 # RUN:   FileCheck --check-prefix=ERR %s
 
 # ERR: ld.lld: error: duplicate symbol: block
 
 # RUN: ld.lld --no-fortran-common -o 10 main.o 1.a
 # RUN: llvm-readobj --syms 10 | FileCheck --check-prefix=NFC %s
+# RUN: ld.lld -o 10 main.o 1.a
+# RUN: llvm-readobj --syms 10 | FileCheck --check-prefix=NFC %s
 
 # RUN: ld.lld --no-fortran-common -o 11 main.o --start-lib 1.o strong_data_only.o --end-lib
 # RUN: llvm-readobj --syms 11 | FileCheck --check-prefix=NFC %s
 
-# RUN: ld.lld -o - main.o 4.a --lto-emit-asm | FileCheck --check-prefix=ASM %s
+# RUN: ld.lld -o - main.o 4.a --fortran-common --lto-emit-asm | FileCheck --check-prefix=ASM %s
 
-# RUN: ld.lld -o - main.o  --start-lib 1.bc 2.bc --end-lib --lto-emit-asm | \
+# RUN: ld.lld -o - main.o  --start-lib 1.bc 2.bc --end-lib --fortran-common --lto-emit-asm | \
 # RUN:   FileCheck --check-prefix=ASM %s
 
 ## COMMON overrides weak. Don't extract 3.bc which provides a weak definition.
 # RUN: ld.lld -o /dev/null main.o --start-lib 1.bc 3.bc --end-lib -y block | FileCheck --check-prefix=LTO_WEAK %s
 
 ## Old FORTRAN that mixes use of COMMON blocks and BLOCK DATA requires that we
 ## search through archives for non-tentative definitions (from the BLOCK DATA)
 ## to replace the tentative definitions (from the COMMON block(s)).

lld/test/ELF/warn-backrefs.s

 
 ## Check common symbols. A common sym might later be replaced by a non-common definition.
 # RUN: echo '.comm obj, 4' | llvm-mc -filetype=obj -triple=x86_64 -o %tcomm.o
 # RUN: echo '.type obj,@object; .data; .globl obj; .p2align 2; obj: .long 55; .size obj, 4' | llvm-mc -filetype=obj -triple=x86_64 -o %tstrong.o
 # RUN: echo '.globl foo; foo: movl obj(%rip), %eax' | llvm-mc -triple=x86_64 -filetype=obj -o %t5.o
 # RUN: llvm-ar rcs %tcomm.a %tcomm.o
 # RUN: llvm-ar rcs %tstrong.a %tstrong.o
 # RUN: ld.lld --warn-backrefs %tcomm.a %t1.o %t5.o 2>&1 -o /dev/null | FileCheck --check-prefix=COMM %s
-# RUN: ld.lld --fatal-warnings --warn-backrefs %tcomm.a %t1.o %t5.o %tstrong.a 2>&1 -o /dev/null
+# RUN: ld.lld --fatal-warnings --fortran-common --warn-backrefs %tcomm.a %t1.o %t5.o %tstrong.a 2>&1 -o /dev/null
 # RUN: ld.lld --warn-backrefs --no-fortran-common %tcomm.a %t1.o %t5.o %tstrong.a 2>&1 -o /dev/null | FileCheck --check-prefix=COMM %s
 
 # COMM: ld.lld: warning: backward reference detected: obj in {{.*}}5.o refers to {{.*}}comm.a
 
 ## If a lazy definition appears after the backward reference, don't warn.
 ## A traditional Unix linker will resolve the reference to the later definition.
 # RUN: ld.lld --fatal-warnings --warn-backrefs %t2.a %t1.o %t2.a -o /dev/null