[llvm-ranlib] Handle -D and -U command line flag

[arichardson]

Summary

I have been trying to build CheriBSD (a fork for FreeBSD for the CHERI
CPU) with LLVM binutils instead of the default elftoolchain utilities.
I noticed that building static archives was failing because ranlib is
invoked with the -D flag. This failed with llvm-ranlib since it parses
the -D flag as the archive path and reports an error that more than one
archive has been passed.

This fixes https://llvm.org/PR41707

[arichardson]

Drop unrelated change

[merge_guards_bot]

Unit tests: fail. 60962 tests passed, 1 failed and 726 were skipped.

failed: LLVM.tools/llvm-ranlib/D-flag.test

clang-format: pass.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

[merge_guards_bot]

Unit tests: fail. 60962 tests passed, 1 failed and 726 were skipped.

failed: LLVM.tools/llvm-ranlib/D-flag.test

clang-format: pass.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

[arichardson]

Force UTC timezone for reproducible results

[merge_guards_bot]

Unit tests: pass. 60963 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: diff.json, CMakeCache.txt, console-log.txt, test-results.xml

[emaste]

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

69

file mode made reproducible too?

[arichardson]

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

69

Does not seem like it, if I run chmod 400 on the .o file it still reports r-------- with -D.

[rupprecht]

Looks good for D/U, but looks like --help and --version options are also supported as combined short args; do you mind adding those too while you're here?

Thanks for the patch!

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

68

-v --version

1164–1176

} else if (arg.front() == 'h') {
  printHelpMessage();
  return 0;
} else if (arg.front() == 'v') {
  cl::PrintVersionMessage();
  return 0;
}

Actually, ranlib -vh and ranlib -hv (on my machine at least) both print the help message. I don't think we need *that* level of compatibility though -- we can go with first-one-wins.

[arichardson]

Also handle -h/-v as short options. Does the adjusted test look okay?

[merge_guards_bot]

Unit tests: fail. 60990 tests passed, 1 failed and 728 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_class/lock.pass.cpp

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[arichardson]

clang-format

[thakis]

The test fails on Windows: http://45.33.8.238/win/4948/step_11.txt

Please take a look, and if takes a while to fix, revert in the meantime.

[arichardson]

In D71554#1801203, @thakis wrote:

The test fails on Windows: http://45.33.8.238/win/4948/step_11.txt

Please take a look, and if takes a while to fix, revert in the meantime.

I'll just relax the permissions check. We only care about the timestamp in this test so there is no need to check the rwx values.

[arichardson]

Should be fixed in https://github.com/llvm/llvm-project/commit/a4f3847f3d5742cfab7acdc614e7ca54643e0c85

[thakis]

In D71554#1801246, @arichardson wrote:

Should be fixed in https://github.com/llvm/llvm-project/commit/a4f3847f3d5742cfab7acdc614e7ca54643e0c85

Thanks! Now it fails with a different error: http://45.33.8.238/win/4962/step_11.txt

[rupprecht]

In D71554#1789360, @arichardson wrote:

Also handle -h/-v as short options. Does the adjusted test look okay?

Sorry, didn't have time to take a second look before the holiday break -- yep, looks good, I didn't anticipate so many buildbot failures. Would be nice if the precommit bot tested Windows too.

llvm/test/tools/llvm-ranlib/D-flag.test

+## Test the -D and -U flags of llvm-ranlib
+## Create an archive with timestamps but without symbol table
+## Important: all `llvm-ar tv` calls must use TZ=UTC to produce identical values
+# RUN: yaml2obj %S/../llvm-ar/Inputs/add-lib1.yaml -o %t.o
+# RUN: env TZ=UTC touch -t 200001020304 %t.o
+# RUN: rm -f %t.a %t-no-index.a && llvm-ar cqSU %t-no-index.a %t.o
+
+## Check that the intial listing has real values:
+# RUN: env TZ=UTC llvm-ar tv %t-no-index.a | FileCheck %s --check-prefix=REAL-VALUES
+
+## Check that the -D flag clears the timestamps:
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -D %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
+
+## Check that the -U flag maintains the timestamps:
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
+
+## Check that we accept multiple values and the last one wins:
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -UDU %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -UUD %t.a
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
+
+## Check arguments can be passed before and after the file name
+# RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a -D -U
+# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
+
+## Check that the -D/-U option is only accepted with a single dash. This matches
+## the GNU ranlib behaviour.
+# RUN: not llvm-ranlib --D %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-D
+# BAD-OPT-D: llvm-ranlib: error: Invalid option: '--D'
+# RUN: not llvm-ranlib --U %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-U
+# BAD-OPT-U: llvm-ranlib: error: Invalid option: '--U'
+# RUN: not llvm-ranlib -x %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-x
+# BAD-OPT-x: llvm-ranlib: error: Invalid option: '-x'
+
+## If the first argument starts with value flags, the error message only shows
+## the remainder of the parsed string:
+# RUN: not llvm-ranlib -Dx %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-x
+# RUN: not llvm-ranlib -DxD %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-xD
+# BAD-OPT-xD: llvm-ranlib: error: Invalid option: '-xD'
+
+# DETERMINISTIC-VALUES: rw-r--r-- 0/0    712 Jan  1 00:00 1970 D-flag.test.tmp.o
+# REAL-VALUES: rw-r--r-- {{[0-9]+}}/{{[0-9]+}} 712 Jan  2 03:04 2000 D-flag.test.tmp.o

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

 
 const char RanlibHelp[] = R"(OVERVIEW: LLVM Ranlib (llvm-ranlib)
 
   This program generates an index to speed access to archives
 
 USAGE: llvm-ranlib <archive-file>
 
 OPTIONS:
-  -h --help                         - Display available options
-  --version                         - Display the version of this program
+  -h --help             - Display available options
+  --version             - Display the version of this program
+  -D                    - Use zero for timestamps and uids/gids (default)
+  -U                    - Use actual timestamps and uids/gids
 )";
 
 const char ArHelp[] = R"(OVERVIEW: LLVM Archiver
 
 USAGE: llvm-ar [options] [-]<operation>[modifiers] [relpos] [count] <archive> [files]
        llvm-ar -M [<mri-script]
 
 OPTIONS:
static int ar_main(int argc, char **argv) {
   }
   ArchiveOperation Operation = parseCommandLine();
   return performOperation(Operation, nullptr);
 }
 
 static int ranlib_main(int argc, char **argv) {
   bool ArchiveSpecified = false;
   for (int i = 1; i < argc; ++i) {
-    if (handleGenericOption(argv[i])) {
+    StringRef arg(argv[i]);
+    if (handleGenericOption(arg)) {
       return 0;
+    } else if (arg.consume_front("-")) {
+      // Handle the -D/-U flag
+      while (!arg.empty()) {
+        if (arg.front() == 'D') {
+          Deterministic = true;
+        } else if (arg.front() == 'U') {
+          Deterministic = false;
+        } else {
+          // TODO: GNU ranlib also supports a -t flag
+          fail("Invalid option: '-" + arg + "'");
+        }
+        arg = arg.drop_front(1);
+      }
     } else {
       if (ArchiveSpecified)
         fail("exactly one archive should be specified");
       ArchiveSpecified = true;
-      ArchiveName = argv[i];
+      ArchiveName = arg.str();
     }
   }
   if (!ArchiveSpecified) {
     badUsage("an archive name must be specified");
   }
   return performOperation(CreateSymTab, nullptr);
 }