[llvm-lib] Write object files in reversed order.

[jacek]

Summary

I don't strictly needed, but this matches how MSVC lib.exe writes to archives, so this makes llvm-lib more compatible and simplifies comparing output between tools.

[efriedma]

Not sure if I have a strong opinion on whether we should do this... but I guess this is reasonable.

llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp

142

llvm::reverse.

[hans]

llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp

143

Do we have a test covering the order of these printed filenames?

It's surprising to me that we need to reverse here, but if it matches msvc it sounds good to me.

[jacek]

Updated using llvm::reverse.

[jacek]

Yes, it's covered by tests, llvm-lib/list.test and two other tests fail if I'd change the order written to the archive, but don't change printed order. I also manually confirmed that with VS 2022.

[hans]

lgtm

[mstorsjo]

In D143536#4115060, @hans wrote:

lgtm

@jacek If you don't have commit access, I can push the patch for you - what's your preferred git author line for these patches?

[jacek]

In D143536#4115102, @mstorsjo wrote:

@jacek If you don't have commit access, I can push the patch for you - what's your preferred git author line for these patches?

Yes, I don't have commit access. Please use "Jacek Caban <[e-mail]>". Thanks!

llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp

static void doList(opt::InputArgList& Args) {
   // lib.exe doesn't print an error if no .lib files are passed.
   if (!B)
     return;
 
   Error Err = Error::success();
   object::Archive Archive(B.get()->getMemBufferRef(), Err);
   fatalOpenError(std::move(Err), B->getBufferIdentifier());
 
+  std::vector<StringRef> Names;
   for (auto &C : Archive.children(Err)) {
     Expected<StringRef> NameOrErr = C.getName();
     fatalOpenError(NameOrErr.takeError(), B->getBufferIdentifier());
-    StringRef Name = NameOrErr.get();
-    llvm::outs() << Name << '\n';
+    Names.push_back(NameOrErr.get());
   }
+  for (auto Name : reverse(Names))
+    llvm::outs() << Name << '\n';
   fatalOpenError(std::move(Err), B->getBufferIdentifier());
 }
 
 static Expected<COFF::MachineTypes> getCOFFFileMachine(MemoryBufferRef MB) {
   std::error_code EC;
   auto Obj = object::COFFObjectFile::create(MB);
   if (!Obj)
     return Obj.takeError();
for (NewArchiveMember &Member : Members) {
     if (sys::path::is_relative(Member.MemberName)) {
       Expected<std::string> PathOrErr =
           computeArchiveRelativePath(OutputPath, Member.MemberName);
       if (PathOrErr)
         Member.MemberName = Saver.save(*PathOrErr);
     }
   }
 
+  // For compatibility with MSVC, reverse member vector after de-duplication.
+  std::reverse(Members.begin(), Members.end());
+
   if (Error E =
           writeArchive(OutputPath, Members,
                        /*WriteSymtab=*/true, object::Archive::K_GNU,
                        /*Deterministic*/ true, Args.hasArg(OPT_llvmlibthin))) {
     handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
       llvm::errs() << OutputPath << ": " << EI.message() << "\n";
     });
     return 1;
   }
 
   return 0;
 }

llvm/test/tools/llvm-lib/Inputs/abc.s

+.globl c
+c:
+.globl a
+a:
+.globl b
+b:

llvm/test/tools/llvm-lib/duplicate.test

 If the same file is specified more than once as an input file,
 llvm-lib should ignore all but the first occurrence of the file.
 
 RUN: rm -rf %t
 RUN: mkdir -p %t
 
 RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/foo.o %S/Inputs/a.s
 RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/bar.o %S/Inputs/b.s
-RUN: llvm-lib -out:%t/foo.lib %t/foo.o %t/foo.o %t/bar.o
+RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/abc.o %S/Inputs/abc.s
+RUN: llvm-lib -out:%t/foo.lib %t/foo.o %t/foo.o %t/abc.o %t/bar.o  %t/foo.o %t/foo.o
 
 RUN: llvm-ar t %t/foo.lib | FileCheck %s
-CHECK: foo.o
-CHECK-NOT: foo.o
 CHECK: bar.o
+CHECK-NEXT: abc.o
+CHECK-NEXT: foo.o
+CHECK-NOT: foo.o

llvm/test/tools/llvm-lib/nest.test

 If an archive file is specified as an input file, its members
 are added to an output file. This test verifies that beahvior.
 
 RUN: rm -rf %t
 RUN: mkdir -p %t
 
 RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/foo.o %S/Inputs/a.s
 RUN: llvm-lib -out:%t/foo.lib %t/foo.o
 
 RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t/bar.o %S/Inputs/b.s
 RUN: llvm-lib -out:%t/bar.lib %t/foo.lib %t/bar.o
 
-RUN: llvm-ar t %t/bar.lib | FileCheck %s
+RUN: llvm-lib -list %t/bar.lib | FileCheck %s
 CHECK: foo.o
 CHECK: bar.o

llvm/test/tools/llvm-lib/use-paths.test

 llvm-lib should behave like "link.exe /lib" and use relative paths to describe
 archive members.
 
 First, get in a clean working directory.
 RUN: rm -rf %t && mkdir -p %t && cd %t
 
 Make foo/a.obj and foo/b.obj.
 RUN: mkdir foo
 RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o foo/a.obj %S/Inputs/a.s
 RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o foo/b.obj %S/Inputs/b.s
 
 RUN: llvm-lib -out:foo.lib foo/a.obj foo/b.obj
 RUN: llvm-ar t foo.lib | FileCheck %s
 
 FIXME: We should probably use backslashes on Windows to better match MSVC tools.
-CHECK: foo/a.obj
 CHECK: foo/b.obj
+CHECK: foo/a.obj
 
 Do it again with absolute paths and see that we get something.
 RUN: llvm-lib -out:foo.lib %t/foo/a.obj %t/foo/b.obj
 RUN: llvm-ar t foo.lib | FileCheck %s --check-prefix=ABS
 
-ABS: {{.*}}/foo/a.obj
 ABS: {{.*}}/foo/b.obj
+ABS: {{.*}}/foo/a.obj

llvm/test/tools/llvm-lib/xfghashmap-list.test

 # RUN: rm -rf %t && mkdir -p %t && cd %t
 # RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o a.obj %S/Inputs/a.s
 # RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o b.obj %S/Inputs/b.s
-# RUN: llvm-lib /out:xfghashmap.lib a.obj b.obj
+# RUN: llvm-lib /out:xfghashmap.lib b.obj a.obj
 
 ## Replace a section in the library file with /<XFGHASHMAP>/ emulating
 ## a library from the Windows SDK for Windows 11.
 # RUN: %python %s xfghashmap.lib b.obj/
 
 ## This should print the /<XFGHASHMAP>/ section as well as an .obj one.
 # RUN: llvm-lib /list %t/xfghashmap.lib | FileCheck %s
 
-# CHECK: a.obj
 # CHECK: /<XFGHASHMAP>/
 # CHECK-NOT: b.obj
+# CHECK: a.obj
 
 import sys
 
 if len(sys.argv) < 3:
   print("Use: python3 xfghashmap-list.test <LIBRARY_FILE> <TEMPLATE>")
   exit(1)
 
 template = bytes(sys.argv[2], 'utf-8')