[llvm-objcopy] Cache gnu_debuglink's target CRC

.gnu_debuglink section contains information regarding file with
debugging symbols, identified by its CRC32. This target file is not
intended to ever change or it would invalidate the stored checksum, yet
the checksum is calculated over and over again for each of the objects
inside the archive, usually hundreds of times.

This patch precomputes the CRC32 of the target once and then reuses the
value where required, saving lots of redundant I/O.

The error message reported should stay the same, although now it might
be reported earlier.

Reviewed by: jhenderson, jakehehrlich, MaskRay

Differential Revision: https://reviews.llvm.org/D61343

Patch by Michal Janiszewski

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360661 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/tools/llvm-objcopy/CopyConfig.cpp b/tools/llvm-objcopy/CopyConfig.cpp
index 94c0a5bc6316b8ae4efe0c0be5395682b52b0427..0b0023f52d8ab1c67f517f6cc9e4684a20ec7f5d 100644 (file)
--- a/tools/llvm-objcopy/CopyConfig.cpp
+++ b/tools/llvm-objcopy/CopyConfig.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Errc.h"
+#include "llvm/Support/JamCRC.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/StringSaver.h"
 #include <memory>
@@ -490,6 +491,22 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   }
 
   Config.AddGnuDebugLink = InputArgs.getLastArgValue(OBJCOPY_add_gnu_debuglink);
+  // The gnu_debuglink's target is expected to not change or else its CRC would
+  // become invalidated and get rejected. We can avoid recalculating the
+  // checksum for every target file inside an archive by precomputing the CRC
+  // here. This prevents a significant amount of I/O.
+  if (!Config.AddGnuDebugLink.empty()) {
+    auto DebugOrErr = MemoryBuffer::getFile(Config.AddGnuDebugLink);
+    if (!DebugOrErr)
+      return createFileError(Config.AddGnuDebugLink, DebugOrErr.getError());
+    auto Debug = std::move(*DebugOrErr);
+    JamCRC CRC;
+    CRC.update(
+        ArrayRef<char>(Debug->getBuffer().data(), Debug->getBuffer().size()));
+    // The CRC32 value needs to be complemented because the JamCRC doesn't
+    // finalize the CRC32 value.
+    Config.GnuDebugLinkCRC32 = ~CRC.getCRC();
+  }
   Config.BuildIdLinkDir = InputArgs.getLastArgValue(OBJCOPY_build_id_link_dir);
   if (InputArgs.hasArg(OBJCOPY_build_id_link_input))
     Config.BuildIdLinkInput =

diff --git a/tools/llvm-objcopy/CopyConfig.h b/tools/llvm-objcopy/CopyConfig.h
index 65d1d27e841c9b31b4f99d7f282a5fefee810f81..7607d3b34a35008c90b456d5f2fb32773feb3ce2 100644 (file)
--- a/tools/llvm-objcopy/CopyConfig.h
+++ b/tools/llvm-objcopy/CopyConfig.h
@@ -115,6 +115,8 @@ struct CopyConfig {
 
   // Advanced options
   StringRef AddGnuDebugLink;
+  // Cached gnu_debuglink's target CRC
+  uint32_t GnuDebugLinkCRC32;
   StringRef BuildIdLinkDir;
   Optional<StringRef> BuildIdLinkInput;
   Optional<StringRef> BuildIdLinkOutput;

diff --git a/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
index bed2414559f797120924b2546e996fba1ca0ee22..27b56f4622dc4841683f4f1069cd709f8d7a9c38 100644 (file)
--- a/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
+++ b/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
@@ -685,7 +685,8 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj,
   }
 
   if (!Config.AddGnuDebugLink.empty())
-    Obj.addSection<GnuDebugLinkSection>(Config.AddGnuDebugLink);
+    Obj.addSection<GnuDebugLinkSection>(Config.AddGnuDebugLink,
+                                        Config.GnuDebugLinkCRC32);
 
   for (const NewSymbolInfo &SI : Config.SymbolsToAdd) {
     SectionBase *Sec = Obj.findSection(SI.SectionName);

diff --git a/tools/llvm-objcopy/ELF/Object.cpp b/tools/llvm-objcopy/ELF/Object.cpp
index d66948a0f46729b3f78b70029a044f7c8eac0837..09d6d8274652ea46d7cd942d1b6fdbef24729498 100644 (file)
--- a/tools/llvm-objcopy/ELF/Object.cpp
+++ b/tools/llvm-objcopy/ELF/Object.cpp
@@ -755,7 +755,7 @@ void Section::initialize(SectionTableRef SecTable) {
 
 void Section::finalize() { this->Link = LinkSection ? LinkSection->Index : 0; }
 
-void GnuDebugLinkSection::init(StringRef File, StringRef Data) {
+void GnuDebugLinkSection::init(StringRef File) {
   FileName = sys::path::filename(File);
   // The format for the .gnu_debuglink starts with the file name and is
   // followed by a null terminator and then the CRC32 of the file. The CRC32
@@ -770,21 +770,12 @@ void GnuDebugLinkSection::init(StringRef File, StringRef Data) {
   // establish the order that sections should go in. By using the maximum
   // possible offset we cause this section to wind up at the end.
   OriginalOffset = std::numeric_limits<uint64_t>::max();
-  JamCRC CRC;
-  CRC.update(ArrayRef<char>(Data.data(), Data.size()));
-  // The CRC32 value needs to be complemented because the JamCRC dosn't
-  // finalize the CRC32 value. It also dosn't negate the initial CRC32 value
-  // but it starts by default at 0xFFFFFFFF which is the complement of zero.
-  CRC32 = ~CRC.getCRC();
-}
-
-GnuDebugLinkSection::GnuDebugLinkSection(StringRef File) : FileName(File) {
-  // Read in the file to compute the CRC of it.
-  auto DebugOrErr = MemoryBuffer::getFile(File);
-  if (!DebugOrErr)
-    error("'" + File + "': " + DebugOrErr.getError().message());
-  auto Debug = std::move(*DebugOrErr);
-  init(File, Debug->getBuffer());
+}
+
+GnuDebugLinkSection::GnuDebugLinkSection(StringRef File,
+                                         uint32_t PrecomputedCRC)
+    : FileName(File), CRC32(PrecomputedCRC) {
+  init(File);
 }
 
 template <class ELFT>

diff --git a/tools/llvm-objcopy/ELF/Object.h b/tools/llvm-objcopy/ELF/Object.h
index 54e72d29aba866999ba6366221f9746a7ae10a55..9298518bcce9bbc98555ac56daf63f699a2b9775 100644 (file)
--- a/tools/llvm-objcopy/ELF/Object.h
+++ b/tools/llvm-objcopy/ELF/Object.h
@@ -18,7 +18,6 @@
 #include "llvm/MC/StringTableBuilder.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/FileOutputBuffer.h"
-#include "llvm/Support/JamCRC.h"
 #include <cstddef>
 #include <cstdint>
 #include <functional>
@@ -703,11 +702,11 @@ private:
   StringRef FileName;
   uint32_t CRC32;
 
-  void init(StringRef File, StringRef Data);
+  void init(StringRef File);
 
 public:
   // If we add this section from an external source we can use this ctor.
-  explicit GnuDebugLinkSection(StringRef File);
+  explicit GnuDebugLinkSection(StringRef File, uint32_t PrecomputedCRC);
   void accept(SectionVisitor &Visitor) const override;
   void accept(MutableSectionVisitor &Visitor) override;
 };