[ELF] Introduce --reproduce flag

[davide]

Summary

Preface: lld is very likely to become the default linker in FreeBSD (in the next year or such).
In order to help the transition, we're going to ask FreeBSD developers to try out the new linker and report bugs.
--reproduce dumps the object files in a directory chosen (preserving the file system layout) and the linker invocation so that people can create an archive and upload to bugzilla.

[ruiu]

ELF/Config.h

51

Remove = "". Strings are initialized to empty string by default. Also please rename Reproduce as the members of Config have the same name as command line options.

ELF/Driver.cpp

101–107

Why do you need to make a path full path?

115

I think you want to use path::append instead of appending "/".

118

Let's create a std::string for the directory name so that we don't need to repeat this string concatenation twice.

125

Let's make it explicit that the feature is optional.

if (Config->Reproduce)
  dumpFile(Path);

254

error(EC, "--reproduce: can't create directory");

261

auto -> const char *

262

This would leave a space character at end of the line and does not terminate the line with '\n'. As this is not a speed-critical path, I'd do

OS << llvm::join(Args.begin(), Args.end(), " ") << "\n";

283

Guard with if (Config->Reproduce) as well.

434–435

This error would be caught in dumpLinkerInvocation, no?

[• rafael]

For what it is worth, something like this would have been very handy
at creating the test directories I use for benchmarking :-)

[davide]

ELF/Driver.cpp

101–107

I don't think I need. Fixed.

262

I don't think your solution works because the way join works right now.
In fact, it doesnt' even build.

In file included from ../tools/lld/ELF/Driver.cpp:21:
../include/llvm/ADT/StringExtras.h:177:20: error: member reference base type 'const char' is not a structure or union

Len += (*Begin).size();
       ~~~~~~~~^~~~~

../include/llvm/ADT/StringExtras.h:192:10: note: in instantiation of function template specialization 'llvm::join_impl<const char *>' requested here

return join_impl(Begin, End, Separator, tag());
       ^

../tools/lld/ELF/Driver.cpp:271:15: note: in instantiation of function template specialization 'llvm::join<const char *>' requested here

OS << llvm::join(*Args.begin(), *Args.end(), " ");
            ^

1 error generated.

We might want to fix in StringExtras.h, but I'd rather do that later.
I added a '\n' at the end.

434–435

It doesn't seem so, i.e. if I remove the check:

[davide@localhost build-clang]$ rm -rf /home/davide/work/llvm/tools/lld/test/ELF/repro2
[davide@localhost build-clang]$ mkdir /home/davide/work/llvm/tools/lld/test/ELF/repro2
[davide@localhost build-clang]$ /home/davide/work/llvm/build-clang/./bin/not /home/davide/work/llvm/build-clang/./bin/ld.lld /home/davide/work/llvm/build-clang/tools/lld/test/ELF/Output/reproduce.s.tmp -o /home/davide/work/llvm/build-clang/tools/lld/test/ELF/Output/reproduce.s.tmp2 --reproduce /home/davide/work/llvm/tools/lld/test/ELF/repro2
[davide@localhost build-clang]$

[ruiu]

ELF/Driver.cpp

251

Remove trailing . and pass EC to error. Then error will format the message for you by appending a colon and EC.message().

error(EC, "--reproduce: can't create directory");

262

OK, but please do not emit trailing whitespace to the generated file.

431–432

Looks like create_directories accepts IgnoreExisting parameter as the second parameter.

std::error_code create_directories(const Twine &path,
                                   bool IgnoreExisting = true,
                                   perms Perms = owner_all | group_all);

You want to pass false then you can remove this code.

[davide]

Addressed second round of comments.

[ruiu]

ELF/Driver.cpp

102–103

Remove.

105

Please do not add llvm:: for consistency.

106–107

if (std::error_code EC = sys::fs::create_directories(DirName))

108

Please pass EC as the first argument at this place too.

114–115

if (std::error_code EC = sys::fs::copy_file(SrcPath, DestPathName))

256

Please fix this place as well. You want to use sys::path::append instead of textually appending path fragments.

261

I'd do this (because ARGV[0] must exist)

OS << Args[0];
for (size_t I = 1, E = Args.size(); I < E; ++I)
  OS << " " << Args[I];
OS << "\n";

[ruiu]

LGTM with a nit.

ELF/Driver.cpp

429–430

Replace this with

Config->Reproduce = getString(Args, OPT_reproduce, "");

and move to the group at line 350-356 (and sort the group).

[silvas]

ELF/Driver.cpp

263

An argument might contain a space, so let's separate these by newlines instead.
(an argument can technically contain a newline but I think that's sufficiently rare that we don't need to worry about that in a first patch.)

[ruiu]

ELF/Driver.cpp

263

Pathname with spaces are also rare (except on Windows), no? An upside of writing all options in one line is you can (re-)execute the command just by passing the file to the shell.

[silvas]

ELF/Driver.cpp

263

Pathname with spaces are also rare (except on Windows), no?

Exactly. Windows is one of our supported platforms.

An upside of writing all options in one line is you can (re-)execute the command just by passing the file to the shell.

For now, let's go ahead with how Davide had it (one line). We can make this more robust in future patches.

ELF/Config.h

struct Configuration {
   llvm::StringRef Entry;
   llvm::StringRef Emulation;
   llvm::StringRef Fini;
   llvm::StringRef Init;
   llvm::StringRef OutputFile;
   llvm::StringRef SoName;
   llvm::StringRef Sysroot;
   std::string RPath;
+  std::string Reproduce;
   std::vector<llvm::StringRef> DynamicList;
   std::vector<llvm::StringRef> SearchPaths;
   std::vector<llvm::StringRef> Undefined;
   std::vector<llvm::StringRef> VersionScriptGlobals;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
   bool Bsymbolic;
   bool BsymbolicFunctions;

ELF/Driver.cpp

 #include "InputFiles.h"
 #include "LinkerScript.h"
 #include "SymbolListFile.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Writer.h"
 #include "lld/Driver/Driver.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include <utility>
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
 
LinkerDriver::getArchiveMembers(MemoryBufferRef MB) {
 
   // Take ownership of memory buffers created for members of thin archives.
   for (std::unique_ptr<MemoryBuffer> &MB : File->takeThinBuffers())
     OwningMBs.push_back(std::move(MB));
 
   return V;
 }
 
+static void dumpFile(StringRef SrcPath) {
+  std::error_code EC;
+
+  SmallString<128> DirName;
+  llvm::sys::path::append(DirName, Config->Reproduce, llvm::sys::path::parent_path(SrcPath));
+  EC = llvm::sys::fs::create_directories(DirName);
+  if (EC) {
+    error("--reproduce: can't create directory");
+    return;
+  }
+
+  SmallString<128> DestPathName;
+  llvm::sys::path::append(DestPathName, Config->Reproduce, SrcPath);
+  EC = llvm::sys::fs::copy_file(SrcPath, DestPathName);
+  if (EC)
+    error(EC, "--reproduce: can't copy file");
+}
+
 // Opens and parses a file. Path has to be resolved already.
 // Newly created memory buffers are owned by this driver.
 void LinkerDriver::addFile(StringRef Path) {
   using namespace llvm::sys::fs;
   if (Config->Verbose)
     llvm::outs() << Path << "\n";
+  if (!Config->Reproduce.empty())
+    dumpFile(Path);
+
   Optional<MemoryBufferRef> Buffer = readFile(Path);
   if (!Buffer.hasValue())
     return;
   MemoryBufferRef MBRef = *Buffer;
 
   switch (identify_magic(MBRef.getBuffer())) {
   case file_magic::unknown:
     readLinkerScript(MBRef);

 
 static bool hasZOption(opt::InputArgList &Args, StringRef Key) {
   for (auto *Arg : Args.filtered(OPT_z))
     if (Key == Arg->getValue())
       return true;
   return false;
 }
 
+static void dumpLinkerInvocation(ArrayRef<const char *> Args) {
+  std::error_code EC = llvm::sys::fs::create_directories(Config->Reproduce,
+    false /* IgnoreExisting */);
+  if (EC) {
+    error(EC, "--reproduce: can't create directory");
+    return;
+  }
+
+  raw_fd_ostream OS(Config->Reproduce + "/invocation.txt", EC,
+    sys::fs::OpenFlags::F_None);
+  check(EC);
+
+  unsigned Size = Args.size();
+  for (unsigned I = 0; I < Size; ++I) {
+    OS << Args[I];
+    if (I < Size - 1)
+      OS << " ";
+  }
+  OS << "\n";
+}
+
 void LinkerDriver::main(ArrayRef<const char *> ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
   if (Args.hasArg(OPT_help)) {
     printHelp(ArgsArr[0]);
     return;
   }
   if (Args.hasArg(OPT_version)) {
     printVersion();
     return;
   }
 
   initLLVM(Args);
   readConfigs(Args);
+
+  if (!Config->Reproduce.empty())
+    dumpLinkerInvocation(ArgsArr);
+
   createFiles(Args);
   checkOptions(Args);
   if (HasError)
     return;
 
   switch (Config->EKind) {
   case ELF32LEKind:
     link<ELF32LE>(Args);
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   for (auto *Arg : Args.filtered(OPT_export_dynamic_symbol))
     Config->DynamicList.push_back(Arg->getValue());
 
   Config->VersionScript = Args.hasArg(OPT_version_script);
   if (Config->VersionScript)
     if (Optional<MemoryBufferRef> Buffer =
             readFile(getString(Args, OPT_version_script)))
       parseVersionScript(*Buffer);
+
+  if (auto *Arg = Args.getLastArg(OPT_reproduce))
+    Config->Reproduce = Arg->getValue();
 }
 
 void LinkerDriver::createFiles(opt::InputArgList &Args) {
   for (auto *Arg : Args) {
     switch (Arg->getOption().getID()) {
     case OPT_l:
       addLibrary(Arg->getValue());
       break;
     case OPT_INPUT:
     case OPT_script:
       addFile(Arg->getValue());
       break;
     case OPT_as_needed:

ELF/Options.td

def o : JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
   HelpText<"Path to file to write output">;
 
 def pie : Flag<["-"], "pie">,
   HelpText<"Create a position independent executable">;
 
 def print_gc_sections: Flag<["--"], "print-gc-sections">,
   HelpText<"List removed unused sections">;
 
+def reproduce : Separate<["--"], "reproduce">,
+  HelpText<"Dump linker invocation and input files for debugging">;
+
 def rpath : Separate<["-"], "rpath">,
   HelpText<"Add a DT_RUNPATH to the output">;
 
 def relocatable : Flag<["--"], "relocatable">,
   HelpText<"Create relocatable object file">;
 
 def script : Separate<["--"], "script">,
   HelpText<"Read linker script">;

test/ELF/reproduce.s

+# REQUIRES: x86
+
+# RUN: rm -rf %S/repro
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: ld.lld %t -o %t2 -shared --as-needed --reproduce %S/repro
+# RUN: llvm-objdump -d %S/repro/%t | FileCheck %s --check-prefix=DUMP
+# RUN: cat %S/repro/invocation.txt | FileCheck %s --check-prefix=INVOCATION
+
+.globl _start;
+_start:
+  mov $60, %rax
+  mov $42, %rdi
+  syscall
+
+# DUMP: Disassembly of section .text:
+# DUMP: _start:
+# DUMP:        0:       48 c7 c0 3c 00 00 00    movq    $60, %rax
+# DUMP:        7:       48 c7 c7 2a 00 00 00    movq    $42, %rdi
+# DUMP:        e:       0f 05   syscall
+
+# INVOCATION: lld {{.*}}reproduce.s{{.*}} -o {{.*}} -shared --as-needed --reproduce
+
+# RUN: rm -rf %S/repro2
+# RUN: mkdir %S/repro2
+# RUN: not ld.lld %t -o %t2 --reproduce %S/repro2 2>&1 | FileCheck --check-prefix=EDIR %s
+# EDIR: --reproduce: can't create directory