[llvm-ar] Rewrite the symbol table when it is invalid.

[rupprecht]

Summary

As an optimization, llvm-ar skips writing the symbol table when it already exists. This is a workaround for build systems that needlessly run ar/ranlib to create a symbol table, even if it already exists.

However, some tools manipulate the archive, and then use ar/ranlib to fix up the symbol table. (Nvidia's nvprune does this). This patch adds a heuristic to determine if the symbol table is invalid, and if so, will rewrite it. This still achieves the goal of the common case, avoiding needless writes when the symbol table is valid, although at the cost of a few more reads (iterating over all members of the symbol table instead of just checking that it exists).

[ruiu]

llvm/lib/Object/Archive.cpp

997

It seems that this function detect certain type of breakage. What kind of breakage does this logic can catch? Is that that common and no other types of breakage uncmmon?

[rupprecht]

This workaround is no longer needed for now -- I can revive it if we have some other use case for it.

llvm/include/llvm/Object/Archive.h

static bool classof(Binary const *v) {
     return v->isArchive();
   }
 
   // check if a symbol is in the archive
   Expected<Optional<Child>> findSym(StringRef name) const;
 
   bool isEmpty() const;
   bool hasSymbolTable() const;
+  // Checks that iterating over the symbol table won't result in any errors.
+  bool isSymbolTableValid() const;
   StringRef getSymbolTable() const { return SymbolTable; }
   StringRef getStringTable() const { return StringTable; }
   uint32_t getNumberOfSymbols() const;
 
   std::vector<std::unique_ptr<MemoryBuffer>> takeThinBuffers() {
     return std::move(ThinBuffers);
   }
 

llvm/lib/Object/Archive.cpp

Expected<Optional<Archive::Child>> Archive::findSym(StringRef name) const {
   }
   return Optional<Child>();
 }
 
 // Returns true if archive file contains no member file.
 bool Archive::isEmpty() const { return Data.getBufferSize() == 8; }
 
 bool Archive::hasSymbolTable() const { return !SymbolTable.empty(); }
+
+bool Archive::isSymbolTableValid() const {
+  for (const Archive::Symbol &Sym : symbols()) {
+    Expected<Archive::Child> C = Sym.getMember();
+    if (!C) {
+      consumeError(C.takeError());
+      return false;
+    }
+  }
+  return true;
+}

llvm/tools/llvm-ar/llvm-ar.cpp

Error E =
                    Kind, Deterministic, Thin, std::move(OldArchiveBuf));
   failIfError(std::move(E), ArchiveName);
 }
 
 static void createSymbolTable(object::Archive *OldArchive) {
   // When an archive is created or modified, if the s option is given, the
   // resulting archive will have a current symbol table. If the S option
   // is given, it will have no symbol table.
-  // In summary, we only need to update the symbol table if we have none.
-  // This is actually very common because of broken build systems that think
-  // they have to run ranlib.
-  if (OldArchive->hasSymbolTable())
+  // In summary, we only need to update the symbol table if we have none, or if
+  // the current one is invalid. This is actually very common because of broken
+  // build systems that think they have to run ranlib.
+  if (OldArchive->hasSymbolTable() && OldArchive->isSymbolTableValid())
     return;
 
   performWriteOperation(CreateSymTab, OldArchive, nullptr, nullptr);
 }
 
 static void performOperation(ArchiveOperation Operation,
                              object::Archive *OldArchive,
                              std::unique_ptr<MemoryBuffer> OldArchiveBuf,