[LLD] Search archives for non-tentative defintions.

[sfertile]

Summary

Work in progress patch to spur the discussion. Motivated by bfds common symbol handling in archives which is needed by FORTRAN libraries which use CommonBlock/BlockData .

When an object symbol is a tentative definition, we search archives for 'real' definitions to override the common symbol.

[NeHuang]

lld/ELF/InputFiles.cpp

1208

nit: remove the empty line.

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

2

The test cases are failing on linux and windows in phabricator unit test, are we missing -triple=powerpc64le or -triple=powerpc64 when running llvm-mc?

29

nit: definitions

29

nit: missing period.

81

nit: referenced

104

Can we remove the comments # in block definitions?

159

nit: initialized

161

nit: TEST1-LABEL:

167

nit: definition from the object file, and the definitions from the archive.

170

nit: TEST2-LABEL:

[stefanp]

The general idea makes sense to me.
I have a set of mostly minor comments here and there.

lld/ELF/InputFiles.cpp

1241

You can probably make this static since you only call it from one place that I can see.
Same with the function below.

lld/ELF/Symbols.cpp

711

You probably don't need to save and restore the type here.
You know based on the guard that the symbol is isObject() so you can just use:

replace(other);
type = llvm::ELF::STT_OBJECT;

[stefanp]

lld/ELF/InputFiles.cpp

1247

Question:
Why do you use Expected<> here?
Won't getName() always return a valid StringRef?

Since you have this here I assume you have come across a situation where you need it.

1249

I think that by definition we cannot have both sym.isDefined() and sym.isCommon() to be both true at the same time.

bool isCommon() const { return symbolKind == CommonKind; }
bool isDefined() const { return symbolKind == DefinedKind; }

You might be able to just use

return sym.isDefined();

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

32

nit:
Do you need the ABI version?

[MaskRay]

Alan Modra kindly shared with me the history
https://sourceware.org/pipermail/binutils/1999-December/002952.html See
elf_link_is_defined_archive_symbol in bfd/elflink.c, which checks whether
a common symbol is a regular definition in an archive member.
LLD's current semantics are compatible with gold, and GNU ld 20 years ago
(before https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a3a8c91d411abe91720a2ac92b8140e1bdb41282)
(I have to blame Sun/HP ld at that time... Ian Lance Taylor said
"It's hard to know what to do with common symbols, because they do not
have clearly defined semantics. Different people have different
expectations of them." It was so unfortunate they picked such strange expectations)

Before we jump directly into a fix, can you create a FORTRAN example where this
special common symbol handling is reasonably used? (I use uppercase FORTRAN
because I think this is FORTRAN 77 legacy...) I know that it matches GNU ld, but
this is rather bizarre. I hope the additional (very high; likely 100+ lines)
complexity in ArchiveFile and LazyObjFile's symbol resolution can be justified.

Note that regular overriding common has an analog: global overriding weak.
Suppose both libweak.a and libglobal.a define a symbol which is referenced by undef.o.
Let's consider two link orders:

• undef.o -lweak -lglobal will pick libweak.a and ignore libglobal.a if libglobal.a does not need fetching.
• -lglobal undef.o -lweak does not fetch libglobal.a at all

So to provide a weak definition while allowing an optional strong definition, the strong definition needs to be surrounded in --whole-archive. Can we ask the user to
do so the similar thing for the regular overriding common case? :)

There are 3 directions:

• We don't need to address it at all. However, it seems that you may raise a reasonable FORTRAN use case. Still, hope you can consider my previous global vs weak example.
• Add a Mach-O style -c option to llvm-ar (ranlib): don't add common symbols to the archive index. See below. (LazyObjFile will be inconsistent but your user does not likely use it I guess)
• We patch LLD.

If we end up patching LLD, we should avoid making Symbol::resolveLazy complex.
We need to think hard doing symbol resolution in batch, in
ArchiveFile/LazyObjFile::parse. For the tests, all the code is insignificant.
My example at https://sourceware.org/pipermail/binutils/2020-August/112878.html
is probably sufficient. As you can see, bitcode LazyObjFile requires special handling.
I need to think about the interaction and I may be able to send a patch.

[psmith]

In ELF the last paragraph of the section on STT_COMMON only mentions that a dynamic linker may change its resolution rules to prefer non-common over a common definition. I'm guessing extending this to static linker is something that some other linkers have done and there are some programs that currently depend on that behaviour.

Symbols with type STT_COMMON label uninitialized common blocks. In relocatable objects, these symbols are not allocated and must have the special section index SHN_COMMON (see below). In shared objects and executables these symbols must be allocated to some section in the defining object.

In relocatable objects, symbols with type STT_COMMON are treated just as other symbols with index SHN_COMMON. If the link-editor allocates space for the SHN_COMMON symbol in an output section of the object it is producing, it must preserve the type of the output symbol as STT_COMMON.

When the dynamic linker encounters a reference to a symbol that resolves to a definition of type STT_COMMON, it may (but is not required to) change its symbol resolution rules as follows: instead of binding the reference to the first symbol found with the given name, the dynamic linker searches for the first symbol with that name with type other than STT_COMMON. If no such symbol is found, it looks for the STT_COMMON definition of that name that has the largest size.

Until recently -fcommon was the default for C programs in Clang and GCC as well, although not for C++. If we do change then this could have an affect wider than Fortran. I can think of a couple of things:

• Some programs will see different library members being selected, potentially changing behaviour.
• For every common definition in the program we are doing library searches and potentially expensive fetches of objects from libraries.

Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

[sfertile]

I've spent some time reading through the linked thread. Thanks. There is a lot to reply to so I hope I don't miss anything.

Before we jump directly into a fix, can you create a FORTRAN example where this special common symbol handling is reasonably used?

The Engineering and Scientific Subroutine Library (ESSL) uses common block and block data. One case that shows this problem is when using DRCFTD function, which calls an internal routine that uses a table of trig values. If we link the library into a shared object we get the expected behavior: the file with the block data overrides all the common blocks and the global symbol is properly initialized in the shared object. If we package the library into an archive and link against that then we end up missing the strong definition since an earlier object provides the tentative definition and the global symbol is zero initialized, producing broken runtime behavior.

Note that regular overriding common has an analog: global overriding weak.

I agree that the special handling for commons is ugly, and I really don't like the asymmetry it produces in relation to weak symbols any more then you do. If we approached the problem strictly from an ELF semantics perspective, then the consistency of golds behavior is arguably desirable. From a usability perspective though, our archive member handling breaks a common idiom in an old but still used dialect of a rather important language. The interaction of common symbols and archive member inclusion unfortunately breaks the FORTRAN language semantics. My understanding is there are lots of modern(ish) programs (not necessarily written in fortran) that link against old FORTRAN libraries and so IMO the trade off in consistency is worth it for the usability.

I don't think using --whole-archives to pull in the entire archive is a reasonable fix. With the libraries I was using, linking in whole archives for all the FORTRAN libraries failed due to multiply defined symbols. In the cases where a library is structured in a way that we can use the whole archive option it will lead to binary bloat in any program that needs to link against an old Fortran library.

In D86142#2225679, @psmith wrote:

Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

I agree the new behavior needs an option to enable it and disable it. Initially I considered having the Fortran driver pass the option to LLD, but after discussing it offline with @rzurob I think it might be best to enable the behavior by default. The consumer of these libraries aren't necessarily Fortran programs, and as you mentioned its likely to be lower risk because we are adopting a behavior already implemented in ld.bfd.

In D86142#2225447, @MaskRay wrote:

If we end up patching LLD, we should avoid making Symbol::resolveLazy complex.
We need to think hard doing symbol resolution in batch, in
ArchiveFile/LazyObjFile::parse. For the tests, all the code is insignificant.
My example at https://sourceware.org/pipermail/binutils/2020-August/112878.html
is probably sufficient. As you can see, bitcode LazyObjFile requires special handling.
I need to think about the interaction and I may be able to send a patch.

That is why I posted a WIP patch early. So we could figure out what the best direction is. I'm open to any suggestions.

[MaskRay]

In D86142#2234472, @sfertile wrote:

In D86142#2225679, @psmith wrote:

Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

I agree the new behavior needs an option to enable it and disable it. Initially I considered having the Fortran driver pass the option to LLD, but after discussing it offline with @rzurob I think it might be best to enable the behavior by default. The consumer of these libraries aren't necessarily Fortran programs, and as you mentioned its likely to be lower risk because we are adopting a behavior already implemented in ld.bfd.

There are 4 combinations: (ArchiveFile,LazyObjFile) x (ELF, BitcodeFile). LazyObjFile is for --start-lib/--end-lib. Do you think whether it is worth making LazyObjFile or BitcodeFile consistent with ArchiveFile x ELF behavior? If we add an option (say, --fortran-linking, the Dec 1999 binutils thread suggested but did not adopt), we will have better justification that we don't make LazyObjFile or BitcodeFile consistent.

I do worry a bit about the time complexity of the regular-overriding-common behavior, which may be a problem for my internal users (Bazel: --start-lib; we use very few ArchiveFiles) and external users (mostly ArchiveFiles).

If we use an option --fortran-linking, we can mostly get away with the time complexity problem, because most users will not pass the option and will not observe any potential performance problems. The time complexity problem can be solved by looking up symbols in batch but the overall complexity may be larger.

In D86142#2225447, @MaskRay wrote:

If we end up patching LLD, we should avoid making Symbol::resolveLazy complex.
We need to think hard doing symbol resolution in batch, in
ArchiveFile/LazyObjFile::parse. For the tests, all the code is insignificant.
My example at https://sourceware.org/pipermail/binutils/2020-August/112878.html
is probably sufficient. As you can see, bitcode LazyObjFile requires special handling.
I need to think about the interaction and I may be able to send a patch.

That is why I posted a WIP patch early. So we could figure out what the best direction is. I'm open to any suggestions.

Thanks!

[jyknight]

In D86142#2234882, @MaskRay wrote:

In D86142#2234472, @sfertile wrote:

In D86142#2225679, @psmith wrote:

Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

I agree the new behavior needs an option to enable it and disable it. Initially I considered having the Fortran driver pass the option to LLD, but after discussing it offline with @rzurob I think it might be best to enable the behavior by default. The consumer of these libraries aren't necessarily Fortran programs, and as you mentioned its likely to be lower risk because we are adopting a behavior already implemented in ld.bfd.

There are 4 combinations: (ArchiveFile,LazyObjFile) x (ELF, BitcodeFile). LazyObjFile is for --start-lib/--end-lib. Do you think whether it is worth making LazyObjFile or BitcodeFile consistent with ArchiveFile x ELF behavior? If we add an option (say, --fortran-linking, the Dec 1999 binutils thread suggested but did not adopt), we will have better justification that we don't make LazyObjFile or BitcodeFile consistent.

IMO, it is important to make it consistent.

I do worry a bit about the time complexity of the regular-overriding-common behavior, which may be a problem for my internal users (Bazel: --start-lib; we use very few ArchiveFiles) and external users (mostly ArchiveFiles).

I think there should not be a problem with time complexity, because there is only extra work to do in uncommon situations:

1. Only if you see a common symbol, do you need to try to find additional lazy definitions of the same symbol name. This will be very rare for C code now, due to -fno-common being the default.
2. Only if the common symbol's name is present in another archive symbol-table do you need to load the archive member to check whether that second definition is also common or not. This will be even more rare, since it will only trigger, in C code, when you have an actually-incorrect use (e.g. "int x" in a header instead of "extern int x").

If we use an option --fortran-linking, we can mostly get away with the time complexity problem, because most users will not pass the option and will not observe any potential performance problems. The time complexity problem can be solved by looking up symbols in batch but the overall complexity may be larger.

I think it's unnecessary to have an option, due to the above. C users should not generally observe performance problems with this feature, because it won't trigger for them.

[sfertile]

Extended the patch so that lazy object files are handled the same as archives, and bitcode files are handled same as native objects, both in archives and as lazy object files.

[MaskRay]

lld/ELF/InputFiles.cpp

1243

range-based for

1247

if is misaligned. equals -> ==

1255

static

1262

range based for

lld/ELF/Symbols.cpp

702

Delete isObject()

A common symbol is a subset of an object symbol. (If it doesn't, it is an error)

lld/test/ELF/Inputs/blockdata.ll

1 ↗

(On Diff #291913)

Use RUN: split-file for short files.

Replace double array with a simple i32.

lld/test/ELF/Inputs/commonblock.ll

6 ↗

(On Diff #291913)

Keeping one function is sufficient to demonstrate the effects.

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

67

The test is much more complex than it needs to be. You just need one reference. All the setup (TOC, addi, .localentry) and teardown (blr) instructions should be deleted not to harm readability.

[sfertile]

Address review comments, switched test to use split-file instead of other inputs and macros, and simplified the test as much as possible.

[MaskRay]

We usually use x86 for generic feature tests (sorry). I am fine with ppc but the test is probably worth a comment that some FORTRAN libraries on ppc require this behavior (i.e. if you no longer require this behavior and if we need some symbol table refactoring where this behavior gets in the way, we should have the right to revisit the decision. SHN_COMMON will get more and more obsoleted and hope we don't need this particular behavior in the future ;-) )

Gold behaves like LLD and internally we have much Fortran code (x86/ppc) which does not tickle this property:)

https://sourceware.org/pipermail/binutils/2020-August/112926.html

COMMON is also one of those dark corners with multiple implementations, so I'm not at all surprised to learn that some linkers handle it differently. Most code doesn't tickle those cases, so these differences generally go unnoticed.

In any case I have to do a large run internally because we still use -fcommon :)

lld/ELF/InputFiles.cpp

1245

sym.isGlobal() should be checked along with the name, because a local symbol should be ignored.

1259

Nit: parens around == are unneeded

1265

static

lld/ELF/Symbols.cpp

692

static

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

2

We usually use x86 for generic features (sorry). I am fine with ppc but this is probably worth a comment that some FORTRAN libraries on ppc require this behavior (i.e. if you no longer require this behavior and if we need some symbol table refactoring where this behavior gets in the way, we should have the right to revisit the decision. SHN_COMMON will get more and more obsoleted and hope we don't need this particular behavior in the future ;-) )

Gold behaves like LLD and internally we have much Fortran code (x86/ppc) which does not tickle this property:)

https://sourceware.org/pipermail/binutils/2020-August/112926.html

COMMON is also one of those dark corners with multiple implementations, so I'm not at all surprised to learn that some linkers handle it differently. Most code doesn't tickle those cases, so these differences generally go unnoticed.

In any case I have to do a large run internally because we still use -fcommon :)

61

For newer tests we use ## for non-RUN-non-CHECK comments.

95

.comm changes st_type, so .type is redundant.

135

Don't mix different indentations. 2 may be sufficient.

[sfertile]

• Add static on helper functions.
• Use ## for comments in lit test.
• Remove redundant parens
• Ignore locals when looking for overriding symbol def.

[sfertile]

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

2

In any case I have to do a large run internally because we still use -fcommon :)

Sorry for the churn. On the bright side if this change impacts any internal projects you can use it to help convince them to stop using -fcommon :).

this is probably worth a comment that some FORTRAN libraries on ppc require this behavior.

The motivating example is ESSL on PPC, but this isn't a PPC distinct problem. From what our fortran compiler devs tell me, its not unusual for heavy fortran users to have one or two ancient libraries they depend on that were written in pre-F90, and no one touches them to port to newer language dialects.

[MaskRay]

In any case I have to do a large run internally because we still use -fcommon :)

Sorry for the churn. On the bright side if this change impacts any internal projects you can use it to help convince them to stop using -fcommon :).

Indeed I have noticed some dependent behaviors. Only two problems (potential ODR violation in an openssl; a qemu not using -fno-common) with a little noise. It is likely we can fix the issues in one week.

[MaskRay]

lld/ELF/InputFiles.h

317

name

lld/ELF/Symbols.cpp

712

dyn_cast -> cast ?

[sfertile]

• Added comment in the test describing why the behaviour change is needed.
• Converted a dyn_cast to a cast.
• Fixed paramater name to use lowercase.
• Fixed .size directives in the test to refer to the correct object.

[MaskRay]

In D86142#2225679, @psmith wrote:

...
Given that BFD supports this behaviour but Gold and LLD do not I suspect that the risk of breaking existing programs is small, especially as LLD tends to be used with more recent programs where -fno-common is the default. However I think we should be cautious and perhaps enable under a command line option, especially as it looks like this is a convention rather than part of a specification. If anyone has better references, then I'm happy to retract.

The code change looks good to me, but I hope @psmith and @grimar can take a look. The question from Peter stands (if we adopt an option it could be --fortran-linking)

lld/ELF/InputFiles.cpp

1240

The two functions need a detailed explanation why it does so. It can quote the thread https://sourceware.org/pipermail/binutils/2020-August/112878.html

[psmith]

I've left a few small suggestions but overall no objections. Given that this is the default BFD behaviour and programs that have only been linked with LLD are not likely to use common; I think that if this behaviour is to be put under an option, it is on by default. This will permit users to use both GNU ld and lld with the same command line.

lld/ELF/InputFiles.cpp

1240

Uncommon could be interpreted as rare rather than not. Perhaps use Noncommon or NonCommon instead?

lld/ELF/InputFiles.h

315

strong isn't defined in ELF, although intuitively I know what you mean. Consider:
"Check if a non-common symbol should be fetched to override a common definition."

338

Same comment about strong

lld/ELF/Symbols.cpp

711

Could this be else if (auto *loSym = dyn_cast<LazyObject>(&other)) ?

[sfertile]

• Add comments about why we have the special lookup behaviour for common in archives..
• Use NonCommon as opposed to UnCommon
• switch second if to be an else if instead.

[sfertile]

In D86142#2360178, @psmith wrote:

I think that if this behaviour is to be put under an option, it is on by default. This will permit users to use both GNU ld and lld with the same command line.

Going to add this now, but I think I'll use --fortran-commons instead of --fortran-linking if there are no objections.

[MaskRay]

In D86142#2376580, @sfertile wrote:

In D86142#2360178, @psmith wrote:

I think that if this behaviour is to be put under an option, it is on by default. This will permit users to use both GNU ld and lld with the same command line.

Going to add this now, but I think I'll use --fortran-commons instead of --fortran-linking if there are no objections.

Use the singular form --fortran-common?

[sfertile]

• Added a detailed comment trying to capture some of the linked discussion and distill why we want the more complicated lookup behaviour (at least by default).
• Added an option -fortran-common to toggle the new behaviour on/off.

[MaskRay]

lld/ELF/InputFiles.cpp

1242

typo: tentative

1251

GOLD -> Gold or gold

1256

BFD-> GNU ld

lld/ELF/Options.td

68

69

lld/ELF/Symbols.cpp

694

Do you have a --warn-backrefs test for backwardReferences.erase(&oldSym); ?

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

8

21

113

[sfertile]

• Add testing for warn-backrefs interaction.
• Spelling fixes
• Fix comments to use ## instead of # in lit test.

[MaskRay]

Thanks for the update. LGTM. It'd be wise to wait for @grimar or @psmith's opinion.

lld/ELF/Options.td

69

Delete the leading space

[MaskRay]

Please also update docs/ld.lld.1 for the new option --no-fortran-common (we usually just document the non-default option in the manpage)

[sfertile]

• Removed leading space in option description
• Updated the manpage doc.

[sfertile]

Fixed one too many hyphens in man page change.

[sfertile]

If there is no objections, I'll go ahead and commit later this week.

[dmajor]

We're seeing errors about duplicate gcov symbols in Rust code after this change: https://bugs.llvm.org/show_bug.cgi?id=49226

Is it expected that this commit would affect non-Fortran code?

[MaskRay]

In D86142#2568879, @dmajor wrote:

We're seeing errors about duplicate gcov symbols in Rust code after this change: https://bugs.llvm.org/show_bug.cgi?id=49226

Is it expected that this commit would affect non-Fortran code?

@dmajor Yes, any object file with common symbols (SHN_COMMON or (rare) STT_COMMON) may observed changed behaviors.
clang 11 and gcc 10 default to -fno-common, so you probably will not observe SHN_COMMON.

LLD<12, gold, macOS ld64 and GNU ld before 1999-12 have the --no-fortran-common behavior.
LLD>=12, GNU ld since 1999-12 have the --fortran-common behavior.

[MaskRay]

Folks, I realized that --fortran-common is not a good default and created D122450 to restore the previous behavior.

lld/ELF/Config.h

struct Configuration {
   bool emitLLVM;
   bool emitRelocs;
   bool enableNewDtags;
   bool executeOnly;
   bool exportDynamic;
   bool fixCortexA53Errata843419;
   bool fixCortexA8;
   bool formatBinary = false;
+  bool fortranCommon;
   bool gcSections;
   bool gdbIndex;
   bool gnuHash = false;
   bool gnuUnique;
   bool hasDynSymTab;
   bool ignoreDataAddressEquality;
   bool ignoreFunctionAddressEquality;
   bool ltoCSProfileGenerate;

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->exportDynamic =
       args.hasFlag(OPT_export_dynamic, OPT_no_export_dynamic, false);
   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);
   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/ELF/InputFiles.h

LazyObjFile(MemoryBufferRef m, StringRef archiveName,
     this->archiveName = std::string(archiveName);
   }
 
   static bool classof(const InputFile *f) { return f->kind() == LazyObjKind; }
 
   template <class ELFT> void parse();
   void fetch();
 
+  // Check if a non-common symbol should be fetched to override a common
+  // definition.
+  bool shouldFetchForCommon(const StringRef &name);
+
   bool fetched = false;
 
 private:
   uint64_t offsetInArchive;
 };
 
 // An ArchiveFile object represents a .a file.
 class ArchiveFile : public InputFile {
 public:
   explicit ArchiveFile(std::unique_ptr<Archive> &&file);
   static bool classof(const InputFile *f) { return f->kind() == ArchiveKind; }
   void parse();
 
   // Pulls out an object file that contains a definition for Sym and
   // returns it. If the same file was instantiated before, this
   // function does nothing (so we don't instantiate the same file
   // more than once.)
   void fetch(const Archive::Symbol &sym);
 
+  // Check if a non-common symbol should be fetched to override a common
+  // definition.
+  bool shouldFetchForCommon(const Archive::Symbol &sym);
+
   size_t getMemberCount() const;
   size_t getFetchedMemberCount() const { return seen.size(); }
 
   bool parsed = false;
 
 private:
   std::unique_ptr<Archive> file;
   llvm::DenseSet<uint64_t> seen;

lld/ELF/InputFiles.cpp

template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
   }
 }
 
 ArchiveFile::ArchiveFile(std::unique_ptr<Archive> &&file)
     : InputFile(ArchiveKind, file->getMemoryBufferRef()),
       file(std::move(file)) {}
 
 void ArchiveFile::parse() {
   for (const Archive::Symbol &sym : file->symbols())
     symtab->addSymbol(LazyArchive{*this, sym});
 
   // Inform a future invocation of ObjFile<ELFT>::initializeSymbols() that this
   // archive has been processed.
   parsed = true;
 }
 
 // Returns a buffer pointing to a member file containing a given symbol.
void ArchiveFile::fetch(const Archive::Symbol &sym) {
   if (tar && c.getParent()->isThin())
     tar->append(relativeToRoot(CHECK(c.getFullName(), this)), mb.getBuffer());
 
   InputFile *file = createObjectFile(mb, getName(), c.getChildOffset());
   file->groupId = groupId;
   parseFile(file);
 }
 
+// The handling of tentative definitions (COMMON symbols) in archives is murky.
+// A tentative defintion will be promoted to a global definition if there are no
+// non-tentative definitions to dominate it. When we hold a tentative definition
+// to a symbol and are inspecting archive memebers for inclusion there are 2
+// ways we can proceed:
+//
+// 1) Consider the tentative definition a 'real' definition (ie promotion from
+//    tentative to real definition has already happened) and not inspect
+//    archive members for Global/Weak definitions to replace the tentative
+//    definition. An archive member would only be included if it satisfies some
+//    other undefined symbol. This is the behavior Gold uses.
+//
+// 2) Consider the tentative definition as still undefined (ie the promotion to
+//    a real definiton happens only after all symbol resolution is done).
+//    The linker searches archive memebers for global or weak definitions to
+//    replace the tentative definition with. This is the behavior used by
+//    GNU ld.
+//
+//  The second behavior is inherited from SysVR4, which based it on the FORTRAN
+//  COMMON BLOCK model. This behavior is needed for proper initalizations in old
+//  (pre F90) FORTRAN code that is packaged into an archive.
+//
+//  The following functions search archive members for defintions to replace
+//  tentative defintions (implementing behavior 2).
+static bool isBitcodeNonCommonDef(MemoryBufferRef mb, StringRef symName,
+                                  StringRef archiveName) {
+  IRSymtabFile symtabFile = check(readIRSymtab(mb));
+  for (const irsymtab::Reader::SymbolRef &sym :
+       symtabFile.TheReader.symbols()) {
+    if (sym.isGlobal() && sym.getName() == symName)
+      return !sym.isUndefined() && !sym.isCommon();
+  }
+  return false;
+}
+
+template <class ELFT>
+static bool isNonCommonDef(MemoryBufferRef mb, StringRef symName,
+                           StringRef archiveName) {
+  ObjFile<ELFT> *obj = make<ObjFile<ELFT>>(mb, archiveName);
+  StringRef stringtable = obj->getStringTable();
+
+  for (auto sym : obj->template getGlobalELFSyms<ELFT>()) {
+    Expected<StringRef> name = sym.getName(stringtable);
+    if (name && name.get() == symName)
+      return sym.isDefined() && !sym.isCommon();
+  }
+  return false;
+}
+
+static bool isNonCommonDef(MemoryBufferRef mb, StringRef symName,
+                           StringRef archiveName) {
+  switch (getELFKind(mb, archiveName)) {
+  case ELF32LEKind:
+    return isNonCommonDef<ELF32LE>(mb, symName, archiveName);
+  case ELF32BEKind:
+    return isNonCommonDef<ELF32BE>(mb, symName, archiveName);
+  case ELF64LEKind:
+    return isNonCommonDef<ELF64LE>(mb, symName, archiveName);
+  case ELF64BEKind:
+    return isNonCommonDef<ELF64BE>(mb, symName, archiveName);
+  default:
+    llvm_unreachable("getELFKind");
+  }
+}
+
+bool ArchiveFile::shouldFetchForCommon(const Archive::Symbol &sym) {
+  Archive::Child c =
+      CHECK(sym.getMember(), toString(this) +
+                                 ": could not get the member for symbol " +
+                                 toELFString(sym));
+  MemoryBufferRef mb =
+      CHECK(c.getMemoryBufferRef(),
+            toString(this) +
+                ": could not get the buffer for the member defining symbol " +
+                toELFString(sym));
+
+  if (isBitcode(mb))
+    return isBitcodeNonCommonDef(mb, sym.getName(), getName());
+
+  return isNonCommonDef(mb, sym.getName(), getName());
+}
+
 size_t ArchiveFile::getMemberCount() const {
   size_t count = 0;
   Error err = Error::success();
   for (const Archive::Child &c : file->children(err)) {
     (void)c;
     ++count;
   }
   // This function is used by --print-archive-stats=, where an error does not
for (Symbol *sym : this->symbols) {
       // to the instantiated ObjFile.
       if (fetched)
         return;
     }
     return;
   }
 }
 
+bool LazyObjFile::shouldFetchForCommon(const StringRef &name) {
+  if (isBitcode(mb))
+    return isBitcodeNonCommonDef(mb, name, archiveName);
+
+  return isNonCommonDef(mb, name, archiveName);
+}
+
 std::string elf::replaceThinLTOSuffix(StringRef path) {
   StringRef suffix = config->thinLTOObjectSuffixReplace.first;
   StringRef repl = config->thinLTOObjectSuffixReplace.second;
 
   if (path.consume_back(suffix))
     return (path + repl).str();
   return std::string(path);
 }

lld/ELF/Options.td

defm compress_debug_sections:
   MetaVarName<"[none,zlib]">;
 
 defm defsym: Eq<"defsym", "Define a symbol alias">, MetaVarName<"<symbol>=<value>">;
 
 defm optimize_bb_jumps: BB<"optimize-bb-jumps",
     "Remove direct jumps at the end to the next basic block",
     "Do not remove any direct jumps at the end to the next basic block (default)">;
 
+defm fortran_common : BB<"fortran-common",
+    "Search archive members for definitions to override COMMON symbols (default)",
+    "Do not search archive members for definitions to override COMMON symbols">;
+
 defm split_stack_adjust_size
     : Eq<"split-stack-adjust-size",
          "Specify adjustment to stack size when a split-stack function calls a "
          "non-split-stack function">,
       MetaVarName<"<value>">;
 
 defm library_path:
   Eq<"library-path", "Add a directory to the library search path">, MetaVarName<"<dir>">;

lld/ELF/Symbols.cpp

void Symbol::resolveDefined(const Defined &other) {
   if (cmp > 0)
     replace(other);
   else if (cmp == 0)
     reportDuplicate(this, other.file,
                     dyn_cast_or_null<InputSectionBase>(other.section),
                     other.value);
 }
 
+template <class LazyT>
+static void replaceCommon(Symbol &oldSym, const LazyT &newSym) {
+  backwardReferences.erase(&oldSym);
+  oldSym.replace(newSym);
+  newSym.fetch();
+}
+
 template <class LazyT> void Symbol::resolveLazy(const LazyT &other) {
+  // For common objects, we want to look for global or weak definitions that
+  // should be fetched as the cannonical definition instead.
+  if (isCommon() && elf::config->fortranCommon) {
+    if (auto *laSym = dyn_cast<LazyArchive>(&other)) {
+      ArchiveFile *archive = cast<ArchiveFile>(laSym->file);
+      const Archive::Symbol &archiveSym = laSym->sym;
+      if (archive->shouldFetchForCommon(archiveSym)) {
+        replaceCommon(*this, other);
+        return;
+      }
+    } else if (auto *loSym = dyn_cast<LazyObject>(&other)) {
+      LazyObjFile *obj = cast<LazyObjFile>(loSym->file);
+      if (obj->shouldFetchForCommon(loSym->getName())) {
+        replaceCommon(*this, other);
+        return;
+      }
+    }
+  }
+
   if (!isUndefined()) {
     // See the comment in resolveUndefined().
     if (isDefined())
       backwardReferences.erase(this);
     return;
   }
 
   // An undefined weak will not fetch archive members. See comment on Lazy in

lld/docs/ld.lld.1

 .It Fl -no-define-common
 Do not assign space to common symbols.
 .It Fl -no-demangle
 Do not demangle symbol names.
 .It Fl -no-dynamic-linker
 Inhibit output of an
 .Li .interp
 section.
+.It Fl -no-fortran-common
+Do not search archive members for definitions to override COMMON symbols.
 .It Fl -no-gc-sections
 Disable garbage collection of unused sections.
 .It Fl -no-gnu-unique
 Disable STB_GNU_UNIQUE symbol binding.
 .It Fl -no-merge-exidx-entries
 Disable merging .ARM.exidx entries.
 .It Fl -no-nmagic
 Page align sections.

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

+# REQUIRES: ppc
+
+# RUN: rm -rf %t.dir
+# RUN: split-file %s %t.dir
+# RUN: cd %t.dir
+
+## Object files.
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj ref.s -o %t1.o
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj refanddef.s -o %t2.o
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj def.s -o %tstrong_data_only.o
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj weak.s -o %tweak_data_only.o
+
+# RUN: llvm-mc -triple=powerpc64le -filetype=obj main.s -o %tmain.o
+
+## Object file archives.
+# RUN: llvm-ar crs %t1.a %t1.o %tstrong_data_only.o
+# RUN: llvm-ar crs %t2.a %t1.o %tweak_data_only.o
+# RUN: llvm-ar crs %t3.a %t2.o %tstrong_data_only.o
+
+## Bitcode files.
+# RUN: llvm-as -o %t1.bc commonblock.ll
+# RUN: llvm-as -o %t2.bc blockdata.ll
+
+## Bitcode archive.
+# RUN: llvm-ar crs %t4.a %t1.bc %t2.bc
+
+# RUN: ld.lld -o %t1 %tmain.o %t1.a
+# RUN: llvm-objdump -D -j .data %t1 | FileCheck --check-prefix=TEST1 %s
+
+# RUN: ld.lld -o %t2 %tmain.o --start-lib %t1.o %tstrong_data_only.o --end-lib
+# RUN: llvm-objdump -D -j .data %t2 | FileCheck --check-prefix=TEST1 %s
+
+# RUN: ld.lld -o %t3 %tmain.o %t2.a
+# RUN: llvm-objdump -D -j .data %t3 | FileCheck --check-prefix=TEST1 %s
+
+# RUN: ld.lld -o %t4 %tmain.o --start-lib %t1.o %tweak_data_only.o --end-lib
+# RUN: llvm-objdump -D -j .data %t4 | FileCheck --check-prefix=TEST1 %s
+
+# RUN: ld.lld -o %t5 %tmain.o %t3.a --print-map | FileCheck --check-prefix=MAP %s
+
+# RUN: ld.lld -o %t6 %tmain.o %t2.o %t1.a
+# RUN: llvm-objdump -D -j .data %t6 | FileCheck --check-prefix=TEST2 %s
+
+# RUN: ld.lld -o %t7 %tmain.o %t2.o --start-lib %t1.o %tstrong_data_only.o --end-lib
+# RUN: llvm-objdump -D -j .data %t7 | FileCheck --check-prefix=TEST2 %s
+
+# RUN: not ld.lld -o %t8 %tmain.o %t1.a %tstrong_data_only.o 2>&1 | \
+# RUN:   FileCheck --check-prefix=ERR %s
+
+# RUN: not ld.lld -o %t9 %tmain.o --start-lib %t1.o %t2.o --end-lib  %tstrong_data_only.o 2>&1 | \
+# RUN:   FileCheck --check-prefix=ERR %s
+
+# ERR: ld.lld: error: duplicate symbol: block
+
+# RUN: ld.lld --no-fortran-common -o %t10 %tmain.o %t1.a
+# RUN: llvm-readobj --syms %t10 | FileCheck --check-prefix=NFC %s
+
+# RUN: ld.lld --no-fortran-common -o %t11 %tmain.o --start-lib %t1.o %tstrong_data_only.o --end-lib
+# RUN: llvm-readobj --syms %t11 | FileCheck --check-prefix=NFC %s
+
+# RUN: ld.lld -o - %tmain.o %t4.a --lto-emit-asm | FileCheck --check-prefix=ASM %s
+
+# RUN: ld.lld -o - %tmain.o  --start-lib %t1.bc %t2.bc --end-lib --lto-emit-asm | \
+# RUN:   FileCheck --check-prefix=ASM %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)).
+
+## Ensure we have used the initialized definition of 'block' instead of a
+## common definition.
+# TEST1-LABEL:  Disassembly of section .data:
+# TEST1:          <block>:
+# TEST1-NEXT:       ea 2e 44 54
+# TEST1-NEXT:       fb 21 09 40
+# TEST1-NEXT:       ...
+
+
+# NFC:       Name: block
+# NFC-NEXT:  Value:
+# NFC-NEXT:  Size: 40
+# NFC-NEXT:  Binding: Global (0x1)
+# NFC-NEXT:  Type: Object (0x1)
+# NFC-NEXT:  Other: 0
+# NFC-NEXT:  Section: .bss
+
+## Expecting the strong definition from the object file, and the defintions from
+## the archive do not interfere.
+# TEST2-LABEL: Disassembly of section .data:
+# TEST2:         <block>:
+# TEST2-NEXT:     03 57 14 8b
+# TEST2-NEXT:     0a bf 05 40
+# TEST2-NEXT:     ...
+
+# MAP:       28 8 {{.*}}tmp3.a(common-archive-lookup.s.tmp2.o):(.data)
+# MAP-NEXT:  28 1 block
+
+# ASM:         .type   block,@object
+# ASM:       block:
+# ASM-NEXT:    .long 5
+# ASM:         .size   block, 20
+
+#--- ref.s
+  .text
+  .abiversion 2
+  .global bar
+  .type bar,@function
+bar:
+  addis 4, 2, block@toc@ha
+  addi  4, 4, block@toc@l
+
+## Tentative definition of 'block'.
+  .comm block,40,8
+
+#--- refanddef.s
+## An alternate strong definition of block, in the same file as
+## a different referenced symbol.
+  .text
+  .abiversion 2
+  .global bar
+  .type bar,@function
+bar:
+  addis 4, 2, block@toc@ha
+  addi  4, 4, block@toc@l
+
+  .data
+  .type block,@object
+  .global block
+  .p2align 3
+block:
+  .quad   0x4005bf0a8b145703              # double 2.7182818284589998
+  .space  32
+  .size   block, 40
+
+#--- def.s
+## Strong definition of 'block'.
+  .data
+  .type block,@object
+  .global block
+  .p2align 3
+block:
+  .quad   0x400921fb54442eea              # double 3.1415926535900001
+  .space  32
+  .size   block, 40
+
+#--- weak.s
+## Weak definition of `block`.
+  .data
+  .type block,@object
+  .weak block
+  .p2align 3
+block:
+  .quad   0x400921fb54442eea              # double 3.1415926535900001
+  .space  32
+  .size   block, 40
+
+#--- main.s
+  .global _start
+_start:
+  bl bar
+  blr
+
+
+#--- blockdata.ll
+target datalayout = "e-m:e-i64:64-n32:64-v256:256:256-v512:512:512"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+@block = dso_local local_unnamed_addr global [5 x i32] [i32 5, i32 0, i32 0, i32 0, i32 0], align 4
+
+#--- commonblock.ll
+target datalayout = "e-m:e-i64:64-n32:64-v256:256:256-v512:512:512"
+target triple = "powerpc64le-unknown-linux-gnu"
+
+@block =  common dso_local local_unnamed_addr global [5 x i32] zeroinitializer, align 4
+
+define dso_local i32 @bar(i32 signext %i) local_unnamed_addr {
+entry:
+  %idxprom = sext i32 %i to i64
+  %arrayidx = getelementptr inbounds [5 x i32], [5 x i32]* @block, i64 0, i64 %idxprom
+  %0 = load i32, i32* %arrayidx, align 8
+  ret i32 %0
+}

lld/test/ELF/warn-backrefs.s

 # RUN: echo '.globl bar; bar:' | llvm-mc -filetype=obj -triple=x86_64 - -o %t3.o
 # RUN: echo '.globl foo; foo: call bar' | llvm-mc -filetype=obj -triple=x86_64 - -o %t4.o
 # RUN: ld.lld --fatal-warnings --warn-backrefs %t1.o --start-lib %t3.o %t4.o --end-lib -o /dev/null
 
 ## We don't report backward references to weak symbols as they can be overridden later.
 # RUN: echo '.weak foo; foo:' | llvm-mc -filetype=obj -triple=x86_64 - -o %tweak.o
 # RUN: ld.lld --fatal-warnings --warn-backrefs --start-lib %tweak.o --end-lib %t1.o %t2.o -o /dev/null
 
+## 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 --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
 
 ## lld fetches the archive while GNU ld resolves the reference to the shared definition.
 ## Warn because the resolution rules are different.
 # RUN: ld.lld --warn-backrefs %t2.a %t1.o %t2.so -o /dev/null 2>&1 | FileCheck %s