[LLD] [COFF] Add an lld specific option /includeoptional

[mstorsjo]

Summary

This works like /include, but is not fatal if the requested symbol wasn't found. This allows implementing the GNU ld option -u.

[ruiu]

Our algorithm for the ELF -u option is as follows:

1. resolve all symbols normally
2. for each symbol given by -u, look up the symbol table, and if a returned symbol is a lazy symbol, call fetch

This way seems better than the way how it is implemented in this patch for the following two reasons:

1. we don't need any Config member as -u option values are consumed in-place
2. The cost of -u is proportional to the number of -u options instead of the number of (undefined) symbols

[mstorsjo]

COFF/SymbolTable.cpp

247

This won't do the right thing, if there actually are proper undefined to the same symbol as well. Any ideas/suggestions, @rnk @ruiu?

[ruiu]

I guess that my previous comment addresses the issue?

[mstorsjo]

In D62976#1533841, @ruiu wrote:

I guess that my previous comment addresses the issue?

Yes, probably - thanks!

Since we fetch objects iteratively in the COFF linker, we probably need to wait until the normal link has converged, then fetch lazy objs for -u, and then keep iterating until it converges again?

[ruiu]

In D62976#1533960, @mstorsjo wrote:

In D62976#1533841, @ruiu wrote:

I guess that my previous comment addresses the issue?

Yes, probably - thanks!

Since we fetch objects iteratively in the COFF linker, we probably need to wait until the normal link has converged, then fetch lazy objs for -u, and then keep iterating until it converges again?

Yeah, I guess so. Technically, fetching an object file can add a new archive to the input file set via a .drctive section which could in turn make other -u symbols can be resolved, so we need to iterate the whole process until it converges. That said, I don't think we have to cover that case though.

[mstorsjo]

Adjusted to make it work more like how this is implemented in the ELF linker; still failing (and properly so) if there's real undefined references to a symbol that was requested with the new option (-u in GNU ld).

[ruiu]

LGTM

COFF/Driver.cpp

1687–1690

I think you can write them in one line

if (auto *Sym = dyn_cast_or_null<Lazy>(Symtab->find(Arg->getValue()))

COFF/Options.td

170

We use the same spellings for identifiers representing a symbol name, so I'd name this include_optional.

COFF/Config.h

struct Configuration {
 
   std::set<StringRef> NoDefaultLibs;
   bool NoDefaultLibAll = false;
 
   // True if we are creating a DLL.
   bool DLL = false;
   StringRef Implib;
   std::vector<Export> Exports;
+  std::set<std::string> AllowUnresolved;
   std::set<std::string> DelayLoads;
   std::map<std::string, int> DLLOrder;
   Symbol *DelayLoadHelper = nullptr;
 
   bool SaveTemps = false;
 
   // /guard:cf
   GuardCFLevel GuardCF = GuardCFLevel::Off;

COFF/Driver.cpp

   // Handle /alternatename
   for (auto *Arg : Args.filtered(OPT_alternatename))
     parseAlternateName(Arg->getValue());
 
   // Handle /include
   for (auto *Arg : Args.filtered(OPT_incl))
     addUndefined(Arg->getValue());
 
+  // Handle /includeoptional
+  for (auto *Arg : Args.filtered(OPT_incl_optional)) {
+    addUndefined(Arg->getValue());
+    Config->AllowUnresolved.insert(Arg->getValue());
+  }
+
   // Handle /implib
   if (auto *Arg = Args.getLastArg(OPT_implib))
     Config->Implib = Arg->getValue();
 
   // Handle /opt.
   bool DoGC = Debug == DebugKind::None || Args.hasArg(OPT_profile);
   unsigned ICFLevel =
       Args.hasArg(OPT_profile) ? 0 : 1; // 0: off, 1: limited, 2: on

     return;
 
   // Do LTO by compiling bitcode input files to a set of native COFF files then
   // link those files.
   Symtab->addCombinedLTOObjects();
   run();
 
   if (Config->MinGW) {
     // Load any further object files that might be needed for doing automatic
     // imports.
     //
     // For cases with no automatically imported symbols, this iterates once
     // over the symbol table and doesn't do anything.
     //
     // For the normal case with a few automatically imported symbols, this
     // should only need to be run once, since each new object file imported
     // is an import library and wouldn't add any new undefined references,
     // but there's nothing stopping the __imp_ symbols from coming from a
     // normal object file as well (although that won't be used for the
     // actual autoimport later on). If this pass adds new undefined references,

COFF/Options.td

 def help_q : Flag<["/??", "-??", "/?", "-?"], "">, Alias<help>;
 
 // LLD extensions
 def exclude_all_symbols : F<"exclude-all-symbols">;
 def export_all_symbols : F<"export-all-symbols">;
 defm demangle : B<"demangle",
     "Demangle symbols in output (default)",
     "Do not demangle symbols in output">;
+def incl_optional : Joined<["/", "-", "/?", "-?"], "includeoptional:">,
+    HelpText<"Add symbol as undefined, but allow it to remain undefined">;
 def kill_at : F<"kill-at">;
 def lldmingw : F<"lldmingw">;
 def output_def : Joined<["/", "-", "/?", "-?"], "output-def:">;
 def pdb_source_path : P<"pdbsourcepath",
                         "Base path used to make relative source file path absolute in PDB">;
 def rsp_quoting : Joined<["--"], "rsp-quoting=">,
   HelpText<"Quoting style for response files, 'windows' (default) or 'posix'">;
 def dash_dash_version : Flag<["--"], "version">,

COFF/SymbolTable.cpp

for (auto &I : SymMap) {
     // We don't want to report missing Microsoft precompiled headers symbols.
     // A proper message will be emitted instead in PDBLinker::aquirePrecompObj
     if (Name.contains("_PchSym_"))
       continue;
 
     if (Config->MinGW && handleMinGWAutomaticImport(Sym, Name))
       continue;
 
+    if (Config->AllowUnresolved.count(Name))
+      continue;
+
     // Remaining undefined symbols are not fatal if /force is specified.
     // They are replaced with dummy defined symbols.
     if (Config->ForceUnresolved)
       replaceSymbol<DefinedAbsolute>(Sym, Name, 0);
     Undefs.insert(Sym);
   }
 
   if (Undefs.empty() && LocalImports.empty())

test/COFF/includeoptional.yaml

+# RUN: yaml2obj < %s > %t.main.obj
+# RUN: yaml2obj < %p/Inputs/include1c.yaml > %t.lib.obj
+# RUN: rm -f %t.lib.lib
+# RUN: llvm-ar cru %t.lib.lib %t.lib.obj
+# RUN: lld-link /out:%t.exe /entry:main /includeoptional:bar /includeoptional:actuallymissing %t.main.obj %t.lib.lib /verbose >& %t.log
+# RUN: FileCheck %s < %t.log
+
+# CHECK: includeoptional.yaml.tmp.main.obj
+# CHECK: includeoptional.yaml.tmp.lib.lib
+# CHECK: includeoptional.yaml.tmp.lib.lib(includeoptional.yaml.tmp.lib.obj) for bar
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     B800000000506800000000680000000050E80000000050E800000000
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          28
+      NumberOfRelocations: 4
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            main
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...