Produce cpio files for --reproduce

[• rafael]

Summary

We want --reproduce to

• not rewrite scripts and thin archives
• work with absolute paths

Given that, it pretty much has to create a full directory tree. On windows that is problematic because of the very short maximum path limit. On most cases users can still work around it with "--repro c:\r", but that is annoying and not viable for automated testing.

We then need to produce some form of archive with the files. The first option that comes to mind is .a files since we already have code for writing them. There are a few problems with them

• The format has a dedicated string table, so we cannot start writing it until all members are known.
• Regular implementations don't support creating directories. We could make llvm-ar support that, but that is probably not a good idea.

The next natural option would be tar. The problem is that to support long path names (which is how this started) it needs a "pax extended header" making this an annoying format to write.

The next option I looked at seems a natural fit: cpio files.

They are available on pretty much every unix, support directories and long path names and are really easy to write. The only slightly annoying part is a terminator, but at least gnu cpio only prints a warning if it is missing, which is handy for crashes. This patch still makes an effort to always create it.

[• rafael]

Fix test.

[• rafael]

Ah, another interesting format would be uncompressed .zip.

It is somewhat similar to cpio in structure, but instead of just a terminator it has a more complicated summary in the end, which makes the file less useful if we crash before finishing it.

In any case, converting to zip would reuse the general structure in this file, so we should probably do this first.

[ruiu]

Very interesting. The choice I thought after "ar" was tar, but cpio seems to be indeed simpler than tar. I knew that the file format exists but I didn't know the details about it.

I'd probably define Cpio class, but this is fine as well.

ELF/DriverUtils.cpp

130

If you are not going to fix it, it is not a fixme. Please change the comment to say that this should be unique, but since no one cares, we don't bother to set a unique number.

149–154

When storing pathnames to an archive, you don't want to prepend Config->Reproduce string, no? For example, if you are storing /home/ruiu/foo.obj, then you store the file as that path, instead of /home/ruiu/repro/home/ruiu/foo.obj.

206

Ditto. You want to store this as just "response.txt" instead of a full path.

ELF/Error.cpp

32

You want to do this for fatal() as well.

test/ELF/reproduce-linkerscript.s

9

I'm not sure if cpio is available everywhere. If not, you need to define "cpio" and "REQUIRES: cpio".

[• rafael]

address review comments.

[llvm-commits]

Comment at: ELF/DriverUtils.cpp:130
@@ +129,3 @@
+ OS << "000000"; c_dev
+ OS << "000000"; c_ino // FIXME: should be unique

+ OS << "100664"; // c_mode: C_ISREG | rw-rw-r--

If you are not going to fix it, it is not a fixme. Please change the comment to say that this should be unique, but since no one cares, we don't bother to set a unique number.

Done

Comment at: ELF/DriverUtils.cpp:145-146
@@ -122,4 +144,4 @@
+void elf::maybeCopyInputFile(StringRef Src, StringRef Data) {

std::string Dest = getDestPath(Src);

• StringRef Dir = path::parent_path(Dest);
• if (std::error_code EC = fs::create_directories(Dir)) {
• error(EC, Dir + ": can't create directory");

+ maybePrintCpioMember(Dest, Data);

+}

When storing pathnames to an archive, you don't want to prepend Config->Reproduce string, no? For example, if you are storing /home/ruiu/foo.obj, then you store the file as that path, instead of /home/ruiu/repro/home/ruiu/foo.obj.

It should be repro/home/ruiu/foo.obj. That way extracting the archive
only creates one directory and you can fakeroot into it.

Comment at: ELF/DriverUtils.cpp:202
@@ +201,3 @@
+ SmallString<128> Dest;
+ path::append(Dest, Config->Reproduce, "response.txt");

+ maybePrintCpioMember(Dest, Data);

Ditto. You want to store this as just "response.txt" instead of a full path.

This is just repro/response.txt.

Comment at: ELF/Error.cpp:32
@@ -31,2 +31,3 @@

HasError = true;

+ maybeCloseReproArchive();

}

You want to do this for fatal() as well.

Done

Comment at: test/ELF/reproduce-linkerscript.s:9
@@ -8,2 +8,3 @@

1. RUN: ld.lld build/foo.script -o bar --reproduce repro

+# RUN: cpio -id < repro.cpio

1. RUN: diff build/foo.script repro/%:t.dir/build/foo.script ----------------

I'm not sure if cpio is available everywhere. If not, you need to define "cpio" and "REQUIRES: cpio".

It is included in the gnuwin32 package that we require for testing on
windows, so I think we are good.

I will upload a new patch in one sec.

Cheers,
Rafael

---

llvm-commits mailing list
[e-mail]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

[ruiu]

LGTM with a nit.

Thank you for doing this. This should be much more reliable than moving files on the actual file system.

ELF/Driver.cpp

267

You can move maybeCloseReproArchive here because all input files (referred directly or indirectly) should be read until this point.

494

Remove.

[ruiu]

I noticed one more thing. The test will probably fail on the bot because of it.

test/ELF/reproduce.s

8

It may fail to extract files under the temporary directory because it could exceeds 255 characters.

[• rafael]

---

You can move maybeCloseReproArchive here because all input files (referred directly or indirectly) should be read until this point.

That is too early. We haven't even added _start to the undefined list yet.

Cheers,
Rafael

[emaste]

test/ELF/reproduce-linkerscript.s

9

I believe cpio was specified by POSIX at one point but was dropped later on due to limitations in the format and for most normal archiving cases tar is a better choice.

Anyhow, I suspect cpio will be widely, but not universally available and likely does need a REQUIRES: cpio

FWIW, with BSD cpio (libarchive) tests pass with the changes here.

[llvm-commits]

The cpio command was dropped, but the format is still documented. The pax
command has to support it.

• msg-27828-33.txt162 BDownload

[emaste]

The cpio command was dropped, but the format is still documented. The pax command has to support it.

Ah, yes - although we should perhaps use pax in the tests instead of cpio then?

[Eugene.Zelenko]

Committed in r268404.

ELF/Driver.h

 //===- Driver.h -------------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_ELF_DRIVER_H
 #define LLD_ELF_DRIVER_H
 
 #include "SymbolTable.h"
 #include "lld/Core/LLVM.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace lld {
 namespace elf {
 
 extern class LinkerDriver *Driver;
 
 class LinkerDriver {
 public:
   void main(ArrayRef<const char *> Args);
   void addFile(StringRef Path);
   void addLibrary(StringRef Name);
   llvm::LLVMContext Context;
 
+  // for --reproduce
+  std::unique_ptr<llvm::raw_fd_ostream> ReproduceArchive;
+  llvm::StringSet<> IncludedFiles;
+
 private:
   std::vector<MemoryBufferRef> getArchiveMembers(MemoryBufferRef MB);
   llvm::Optional<MemoryBufferRef> readFile(StringRef Path);
   void readConfigs(llvm::opt::InputArgList &Args);
   void createFiles(llvm::opt::InputArgList &Args);
   template <class ELFT> void link(llvm::opt::InputArgList &Args);
 
   // True if we are in --whole-archive and --no-whole-archive.

 #include "Options.inc"
 #undef OPTION
 };
 
 void printHelp(const char *Argv0);
 void printVersion();
 
 void createResponseFile(const llvm::opt::InputArgList &Args);
-void copyInputFile(StringRef Path);
+void maybeCopyInputFile(StringRef Path, StringRef Buffer);
 
 std::string findFromSearchPaths(StringRef Path);
 std::string searchLibrary(StringRef Path);
 std::string buildSysrootedPath(llvm::StringRef Dir, llvm::StringRef File);
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/Driver.cpp

 }
 
 // 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 sys::fs;
   if (Config->Verbose)
     outs() << Path << "\n";
-  if (!Config->Reproduce.empty())
-    copyInputFile(Path);
 
   Optional<MemoryBufferRef> Buffer = readFile(Path);
   if (!Buffer.hasValue())
     return;
   MemoryBufferRef MBRef = *Buffer;
 
+  maybeCopyInputFile(Path, MBRef.getBuffer());
+
   switch (identify_magic(MBRef.getBuffer())) {
   case file_magic::unknown:
     readLinkerScript(MBRef);
     return;
   case file_magic::archive:
     if (WholeArchive) {
       for (MemoryBufferRef MB : getArchiveMembers(MBRef))
         Files.push_back(createObjectFile(MB, Path));
void LinkerDriver::main(ArrayRef<const char *> ArgsArr) {
   if (Args.hasArg(OPT_version)) {
     printVersion();
     return;
   }
 
   readConfigs(Args);
   initLLVM(Args);
 
-  if (!Config->Reproduce.empty())
+  if (!Config->Reproduce.empty()) {
+    std::error_code EC;
+    ReproduceArchive = make_unique<raw_fd_ostream>(Config->Reproduce + ".cpio",
+                                                   EC, fs::F_None);
+    check(EC);
     createResponseFile(Args);
+  }
 
   createFiles(Args);
   checkOptions(Args);
   if (HasError)
     return;
 
   switch (Config->EKind) {
   case ELF32LEKind:
     link<ELF32LE>(Args);
     return;
   case ELF32BEKind:
     link<ELF32BE>(Args);
     return;
   case ELF64LEKind:
template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   Symtab.scanDynamicList();
   Symtab.scanVersionScript();
 
   Symtab.addCombinedLtoObject();
 
   for (auto *Arg : Args.filtered(OPT_wrap))
     Symtab.wrap(Arg->getValue());
 
+  maybeCloseReproArchive();
+
   // Write the result to the file.
   if (Config->GcSections)
     markLive<ELFT>();
   if (Config->ICF)
     doIcf<ELFT>();
   writeResult<ELFT>(&Symtab);
 }

ELF/DriverUtils.cpp

 
 static std::string getDestPath(StringRef Path) {
   std::string Relpath = relativeToRoot(Path);
   SmallString<128> Dest;
   path::append(Dest, Config->Reproduce, Relpath);
   return Dest.str();
 }
 
-// Copies file Src to {Config->Reproduce}/Src.
-void elf::copyInputFile(StringRef Src) {
-  std::string Dest = getDestPath(Src);
-  StringRef Dir = path::parent_path(Dest);
-  if (std::error_code EC = fs::create_directories(Dir)) {
-    error(EC, Dir + ": can't create directory");
+static void maybePrintCpioMember(StringRef Path, StringRef Data) {
+  if (Config->Reproduce.empty())
+    return;
+
+  if (!Driver->IncludedFiles.insert(Path).second)
     return;
-  }
-  if (std::error_code EC = fs::copy_file(Src, Dest))
-    error(EC, "failed to copy file: " + Dest);
+
+  raw_fd_ostream &OS = *Driver->ReproduceArchive;
+  OS << "070707"; // c_magic
+
+  // The c_dev/c_ino pair should be unique according to the spec, but no one
+  // seems to care.
+  OS << "000000"; // c_dev
+  OS << "000000"; // c_ino
+
+  OS << "100664";                        // c_mode: C_ISREG | rw-rw-r--
+  OS << "000000";                        // c_uid
+  OS << "000000";                        // c_gid
+  OS << "000001";                        // c_nlink
+  OS << "000000";                        // c_rdev
+  OS << "00000000000";                   // c_mtime
+  OS << format("%06o", Path.size() + 1); // c_namesize
+  OS << format("%011o", Data.size());    // c_filesize
+  OS << Path << '\0';                    // c_name
+  OS << Data;                            // c_filedata
+
+// Write file Src with content Data to the archive.
+void elf::maybeCopyInputFile(StringRef Src, StringRef Data) {
+  std::string Dest = getDestPath(Src);
+  maybePrintCpioMember(Dest, Data);
+}
+
+void elf::maybeCloseReproArchive() {
+  if (!Driver->ReproduceArchive)
+    return;
+  maybePrintCpioMember("TRAILER!!!", "");
+  Driver->ReproduceArchive.reset();
 }
 
 // Quote a given string if it contains a space character.
 static std::string quote(StringRef S) {
   if (S.find(' ') == StringRef::npos)
     return S;
   return ("\"" + S + "\"").str();
 }
 
 static std::string rewritePath(StringRef S) {
   if (fs::exists(S))
     return relativeToRoot(S);
   return S;
 }
 
 // Copies all input files to Config->Reproduce directory and
 // create a response file as "response.txt", so that you can re-run
 // the same command with the same inputs just by executing
 // "ld.lld @response.txt". Used by --reproduce. This feature is
 // supposed to be used by users to report an issue to LLD developers.
 void elf::createResponseFile(const opt::InputArgList &Args) {
-  // Create the output directory.
-  if (std::error_code EC =
-          fs::create_directories(Config->Reproduce, /*IgnoreExisting=*/false)) {
-    error(EC, Config->Reproduce + ": can't create directory");
-    return;
-  }
-
-  // Open "response.txt".
-  SmallString<128> Path;
-  path::append(Path, Config->Reproduce, "response.txt");
-  std::error_code EC;
-  raw_fd_ostream OS(Path, EC, fs::OpenFlags::F_None);
-  check(EC);
-
+  SmallString<0> Data;
+  raw_svector_ostream OS(Data);
   // Copy the command line to response.txt while rewriting paths.
   for (auto *Arg : Args) {
     switch (Arg->getOption().getID()) {
     case OPT_reproduce:
       break;
     case OPT_INPUT:
       OS << quote(rewritePath(Arg->getValue())) << "\n";
       break;
     case OPT_L:
     case OPT_dynamic_list:
     case OPT_rpath:
     case OPT_script:
     case OPT_version_script:
       OS << Arg->getSpelling() << " "
          << quote(rewritePath(Arg->getValue())) << "\n";
       break;
     default:
       OS << Arg->getSpelling();
       if (Arg->getNumValues() > 0)
         OS << " " << quote(Arg->getValue());
       OS << "\n";
     }
   }
+
+  SmallString<128> Dest;
+  path::append(Dest, Config->Reproduce, "response.txt");
+  maybePrintCpioMember(Dest, Data);
 }
 
 std::string elf::findFromSearchPaths(StringRef Path) {
   for (StringRef Dir : Config->SearchPaths) {
     std::string FullPath = buildSysrootedPath(Dir, Path);
     if (fs::exists(FullPath))
       return FullPath;
   }

ELF/Error.h

 #include "lld/Core/LLVM.h"
 
 namespace lld {
 namespace elf {
 
 extern bool HasError;
 extern llvm::raw_ostream *ErrorOS;
 
+void maybeCloseReproArchive();
+
 void log(const Twine &Msg);
 void warning(const Twine &Msg);
 
 void error(const Twine &Msg);
 void error(std::error_code EC, const Twine &Prefix);
 
 template <typename T> void error(const ErrorOr<T> &V, const Twine &Prefix) {
   error(V.getError(), Prefix);

ELF/Error.cpp

if (Config->Verbose)
     llvm::outs() << Msg << "\n";
 }
 
 void warning(const Twine &Msg) { llvm::errs() << Msg << "\n"; }
 
 void error(const Twine &Msg) {
   *ErrorOS << Msg << "\n";
   HasError = true;
+  maybeCloseReproArchive();
 }
 
 void error(std::error_code EC, const Twine &Prefix) {
   if (EC)
     error(Prefix + ": " + EC.message());
 }
 
 void fatal(const Twine &Msg) {
   llvm::errs() << Msg << "\n";
+  maybeCloseReproArchive();
   exit(1);
 }
 
 void fatal(const Twine &Msg, const Twine &Prefix) {
   fatal(Prefix + ": " + Msg);
 }
 
 void check(std::error_code EC) {
   if (EC)
     fatal(EC.message());
 }
 
 } // namespace elf
 } // namespace lld

ELF/InputFiles.cpp

 MemoryBufferRef ArchiveFile::getMember(const Archive::Symbol *Sym) {
   Archive::Child C =
       check(Sym->getMember(),
             "could not get the member for symbol " + Sym->getName());
 
   if (!Seen.insert(C.getChildOffset()).second)
     return MemoryBufferRef();
 
-  if (!Config->Reproduce.empty() && C.getParent()->isThin())
-    copyInputFile(check(C.getFullName()));
-
-  return check(C.getMemoryBufferRef(),
-               "could not get the buffer for the member defining symbol " +
-                   Sym->getName());
+  MemoryBufferRef Ret =
+      check(C.getMemoryBufferRef(),
+
+  if (C.getParent()->isThin())
+    maybeCopyInputFile(check(C.getFullName()), Ret.getBuffer());
+
+  return Ret;
 }
 
 template <class ELFT>
 SharedFile<ELFT>::SharedFile(MemoryBufferRef M)
     : ELFFileBase<ELFT>(Base::SharedKind, M), AsNeeded(Config->AsNeeded) {}
 
 template <class ELFT>
 const typename ELFT::Shdr *

test/ELF/reproduce-error.s

+# RUN: rm -rf %t.dir
+# RUN: mkdir -p %t.dir
+# RUN: cd %t.dir
+
+# RUN: not ld.lld --reproduce repro abc -o t 2>&1 | FileCheck %s
+# CHECK: cannot open abc: No such file or directory
+
+# RUN: grep TRAILER repro.cpio
+# RUN: cpio -id < repro.cpio
+# RUN: FileCheck --check-prefix=RSP %s < repro/response.txt
+# RSP: abc
+# RSP: -o t

test/ELF/reproduce-linkerscript.s

 # REQUIRES: x86, shell
 
 # RUN: rm -rf %t.dir
 # RUN: mkdir -p %t.dir/build
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.dir/build/foo.o
 # RUN: echo "INPUT(\"%t.dir/build/foo.o\")" > %t.dir/build/foo.script
 # RUN: cd %t.dir
 # RUN: ld.lld build/foo.script -o bar --reproduce repro
+# RUN: cpio -id < repro.cpio
 # RUN: diff build/foo.script repro/%:t.dir/build/foo.script
 # RUN: diff build/foo.o repro/%:t.dir/build/foo.o
 
 .globl _start
 _start:
   mov $60, %rax
   mov $42, %rdi
   syscall

test/ELF/reproduce-thin-archive.s

 # REQUIRES: x86, shell
 
 # RUN: rm -rf %t.dir
 # RUN: mkdir -p %t.dir
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.dir/foo.o
 # RUN: cd %t.dir
 # RUN: llvm-ar --format=gnu rcT foo.a foo.o
 # RUN: ld.lld -m elf_x86_64 foo.a -o bar --reproduce repro
+# RUN: cpio -id < repro.cpio
 # RUN: diff foo.a repro/%:t.dir/foo.a
 # RUN: diff foo.o repro/%:t.dir/foo.o
 
 .globl _start
 _start:
   nop

test/ELF/reproduce.s

 # REQUIRES: x86
 
-# XXX: Temporary hack to work around windows path length limitation due to
-# the build dir for llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast.
-# When we directly generate an archive this won't be an issue.
-# REQUIRES: shell
-
 # RUN: rm -rf %t.dir
 # RUN: mkdir -p %t.dir/build1
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.dir/build1/foo.o
 # RUN: cd %t.dir
 # RUN: ld.lld --hash-style=gnu build1/foo.o -o bar -shared --as-needed --reproduce repro
+# RUN: cpio -id < repro.cpio
 # RUN: diff build1/foo.o repro/%:t.dir/build1/foo.o
 
 # RUN: FileCheck %s --check-prefix=RSP < repro/response.txt
 # RSP: {{^}}--hash-style gnu{{$}}
 # RSP-NOT: repro{{[/\\]}}
 # RSP-NEXT: {{[/\\]}}foo.o
 # RSP-NEXT: -o bar
 # RSP-NEXT: -shared
 # RSP-NEXT: --as-needed
 
 # RUN: mkdir -p %t.dir/build2/a/b/c
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.dir/build2/foo.o
 # RUN: cd %t.dir/build2/a/b/c
 # RUN: ld.lld ./../../../foo.o -o bar -shared --as-needed --reproduce repro
+# RUN: cpio -id < repro.cpio
 # RUN: diff %t.dir/build2/foo.o repro/%:t.dir/build2/foo.o
 
 # RUN: echo "{ local: *; };" >  ver
 # RUN: echo > dyn
 # RUN: echo > file
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o 'foo bar'
 # RUN: ld.lld --reproduce repro2 'foo bar' -L"foo bar" -Lfile \
 # RUN:   --dynamic-list dyn -rpath file --script file --version-script ver \
 # RUN:   --dynamic-linker "some unusual/path"
+# RUN: cpio -id < repro2.cpio
 # RUN: FileCheck %s --check-prefix=RSP2 < repro2/response.txt
 # RSP2:      "{{.*}}foo bar"
 # RSP2-NEXT: -L "{{.*}}foo bar"
 # RSP2-NEXT: -L {{.+}}file
 # RSP2-NEXT: --dynamic-list {{.+}}dyn
 # RSP2-NEXT: -rpath {{.+}}file
 # RSP2-NEXT: --script {{.+}}file
 # RSP2-NEXT: --version-script {{.+}}ver
 # RSP2-NEXT: --dynamic-linker "some unusual/path"
 
-# RUN: not ld.lld build1/foo.o -o bar -shared --as-needed --reproduce . 2>&1 \
-# RUN:   | FileCheck --check-prefix=ERROR %s
-# ERROR: can't create directory
-
 .globl _start
 _start:
   mov $60, %rax
   mov $42, %rdi
   syscall