[llvm] Improve the performance of getSymbols used by ArchiveWriter

[alexander-shaposhnikov]

Summary

In this diff we adjust the code of getSymbols to avoid creating LLVMContext when it's not necessary.
Without this patch when the function getSymbols was called on a MachO object with a __bitcode section
it was parsing the embedded bitcode and then ignoring the result.

Test plan: make check-all (on both OSX and Linux)

[abdulras]

lib/Object/ArchiveWriter.cpp

376–397

Can't this be sunk into the condition on L378? That will avoid the unnecessary construction.

[alexander-shaposhnikov]

Add a comment

[alexander-shaposhnikov]

update the comment

lib/Object/ArchiveWriter.cpp

static void writeSymbolTable(raw_ostream &Out, object::Archive::Kind Kind,
 
   while (Pad--)
     Out.write(uint8_t(0));
 }
 
 static Expected<std::vector<unsigned>>
 getSymbols(MemoryBufferRef Buf, raw_ostream &SymNames, bool &HasObject) {
   std::vector<unsigned> Ret;
-  LLVMContext Context;
 
-  Expected<std::unique_ptr<object::SymbolicFile>> ObjOrErr =
-      object::SymbolicFile::createSymbolicFile(Buf, llvm::file_magic::unknown,
-                                               &Context);
+  // In the scenario when LLVMContext is getting populated
+  // SymbolicFile will contain a reference to it,
+  // thus SymbolicFile should be destroyed first.
+  LLVMContext Context;
+  std::unique_ptr<object::SymbolicFile> Obj;
+  if (identify_magic(Buf.getBuffer()) == file_magic::bitcode) {
+    auto ObjOrErr = object::SymbolicFile::createSymbolicFile(
+        Buf, file_magic::bitcode, &Context);
+    if (!ObjOrErr) {
+      // FIXME: check only for "not an object file" errors.
+      consumeError(ObjOrErr.takeError());
+      return Ret;
+    }
+    Obj = std::move(*ObjOrErr);
+  } else {
+    auto ObjOrErr = object::SymbolicFile::createSymbolicFile(Buf);
     if (!ObjOrErr) {
       // FIXME: check only for "not an object file" errors.
       consumeError(ObjOrErr.takeError());
       return Ret;
     }
+    Obj = std::move(*ObjOrErr);
+  }
 
   HasObject = true;
-  object::SymbolicFile &Obj = *ObjOrErr.get();
-  for (const object::BasicSymbolRef &S : Obj.symbols()) {
+  for (const object::BasicSymbolRef &S : Obj->symbols()) {
     if (!isArchiveSymbol(S))
       continue;
     Ret.push_back(SymNames.tell());
     if (auto EC = S.printName(SymNames))
       return errorCodeToError(EC);
     SymNames << '\0';
   }
   return Ret;
Error llvm::writeArchive(StringRef ArcName,
 
   if (!StringTableBuf.empty())
     Data.insert(Data.begin(), computeStringTable(StringTableBuf));
 
   // We would like to detect if we need to switch to a 64-bit symbol table.
   if (WriteSymtab) {
     uint64_t MaxOffset = 0;
     uint64_t LastOffset = MaxOffset;
-    for (const auto& M : Data) {
+    for (const auto &M : Data) {
       // Record the start of the member's offset
       LastOffset = MaxOffset;
       // Account for the size of each part associated with the member.
       MaxOffset += M.Header.size() + M.Data.size() + M.Padding.size();
       // We assume 32-bit symbols to see if 32-bit symbols are possible or not.
       MaxOffset += M.Symbols.size() * 4;
     }