[PATCH] D68204: [llvm-lib] Correctly handle .lib input files

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp:329
 
-    Expected<NewArchiveMember> MOrErr =
-        NewArchiveMember::getFile(Saver.save(Path), /*Deterministic=*/true);
-    if (!MOrErr) {
-      handleAllErrors(MOrErr.takeError(), [&](const ErrorInfoBase &EIB) {
-        llvm::errs() << Arg->getValue() << ": " << EIB.message() << "\n";
-      });
-      return 1;
-    }
+    // Open a file
+    ErrorOr<std::unique_ptr<MemoryBuffer>> MOrErr =
----------------
Full stop


================
Comment at: llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp:335
 
-    file_magic Magic = identify_magic(MOrErr->Buf->getBuffer());
-    if (Magic != file_magic::coff_object && Magic != file_magic::bitcode &&
-        Magic != file_magic::windows_resource) {
-      llvm::errs() << Arg->getValue()
-                   << ": not a COFF object, bitcode or resource file\n";
-      return 1;
-    }
-
-    // Check that all input files have the same machine type.
-    // Mixing normal objects and LTO bitcode files is fine as long as they
-    // have the same machine type.
-    // Doing this here duplicates the header parsing work that writeArchive()
-    // below does, but it's not a lot of work and it's a bit awkward to do
-    // in writeArchive() which needs to support many tools, can't assume the
-    // input is COFF, and doesn't have a good way to report errors.
-    COFF::MachineTypes FileMachine = COFF::IMAGE_FILE_MACHINE_UNKNOWN;
-    if (Magic == file_magic::coff_object) {
-      std::error_code EC;
-      object::COFFObjectFile Obj(*MOrErr->Buf, EC);
-      if (EC) {
-        llvm::errs() << Arg->getValue() << ": failed to open: " << EC.message()
-                     << '\n';
-        return 1;
-      }
-      uint16_t Machine = Obj.getMachine();
-      if (Machine != COFF::IMAGE_FILE_MACHINE_I386 &&
-          Machine != COFF::IMAGE_FILE_MACHINE_AMD64 &&
-          Machine != COFF::IMAGE_FILE_MACHINE_ARMNT &&
-          Machine != COFF::IMAGE_FILE_MACHINE_ARM64) {
-        llvm::errs() << Arg->getValue() << ": unknown machine: " << Machine
-                     << '\n';
-        return 1;
-      }
-      FileMachine = static_cast<COFF::MachineTypes>(Machine);
-    } else if (Magic == file_magic::bitcode) {
-      Expected<std::string> TripleStr = getBitcodeTargetTriple(*MOrErr->Buf);
-      if (!TripleStr) {
-        llvm::errs() << Arg->getValue()
-                     << ": failed to get target triple from bitcode\n";
-        return 1;
-      }
-      switch (Triple(*TripleStr).getArch()) {
-      case Triple::x86:
-        FileMachine = COFF::IMAGE_FILE_MACHINE_I386;
-        break;
-      case Triple::x86_64:
-        FileMachine = COFF::IMAGE_FILE_MACHINE_AMD64;
-        break;
-      case Triple::arm:
-        FileMachine = COFF::IMAGE_FILE_MACHINE_ARMNT;
-        break;
-      case Triple::aarch64:
-        FileMachine = COFF::IMAGE_FILE_MACHINE_ARM64;
-        break;
-      default:
-        llvm::errs() << Arg->getValue() << ": unknown arch in target triple "
-                     << *TripleStr << '\n';
-        return 1;
-      }
-    }
-
-    // FIXME: Once lld-link rejects multiple resource .obj files:
-    // Call convertResToCOFF() on .res files and add the resulting
-    // COFF file to the .lib output instead of adding the .res file, and remove
-    // this check. See PR42180.
-    if (FileMachine != COFF::IMAGE_FILE_MACHINE_UNKNOWN) {
-      if (LibMachine == COFF::IMAGE_FILE_MACHINE_UNKNOWN) {
-        LibMachine = FileMachine;
-        LibMachineSource = std::string(" (inferred from earlier file '") +
-                           Arg->getValue() + "')";
-      } else if (LibMachine != FileMachine) {
-        llvm::errs() << Arg->getValue() << ": file machine type "
-                     << machineToStr(FileMachine)
-                     << " conflicts with library machine type "
-                     << machineToStr(LibMachine) << LibMachineSource << '\n';
-        return 1;
-      }
-    }
+    // Append a file
+    appendFile(Members, LibMachine, LibMachineSource, MBRef);
----------------
Full stop


================
Comment at: llvm/test/tools/llvm-lib/nest.test:12
+RUN: llvm-lib -out:%t/bar.lib %t/foo.lib %t/bar.o
+
+RUN: llvm-ar t %t/bar.lib | FileCheck %s
----------------
What if `%t/foo.lib` is specified twice? It may worth a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68204/new/

https://reviews.llvm.org/D68204