[PATCH] D118931: [llvm-libtool-darwin] Add -warnings_as_errors

[Keith Smiley via Phabricator via llvm-commits]

keith created this revision.
keith added reviewers: jhenderson, alexander-shaposhnikov, Roger, smeenai, thevinster.
Herald added a subscriber: kristof.beyls.
keith requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

libtool can currently produce 2 warnings:

1. No symbols were in the object file
2. An object file with the same basename was specified multiple times

The first warning here is often harmless and may just mean you have some
translation units with no symbols for the target you're building for.
The second warning can lead to real issues like those mentioned in
https://reviews.llvm.org/D113130 where ODR violations can slip in.

This introduces a new -warnings_as_errors flag that can be used by build
systems that want to verify they never hit these warnings. For example
with bazel the libtool caller first uniques names to make sure the
duplicate base name case is not possible, but if that doesn't work as
expected, having it fail would be preferred.

It's also worth noting that llvm-libtool-darwin works around an issue
that cctools libtool experiences related to debug info and duplicate
basenames, the workaround is described here:
https://github.com/llvm/llvm-project/blob/30baa5d2a450d5e302d8cba3fc7a26a59d4b7ae1/llvm/lib/Object/ArchiveWriter.cpp#L424-L465
And it avoids this bug:
https://github.com/keith/radars/tree/f0cbbb1c37126ec6528c132510b29e08566377a7/DuplicateBasenameIssue


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118931

Files:
  llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
  llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test
  llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp


Index: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
===================================================================
--- llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
+++ llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
@@ -98,6 +98,11 @@
     cl::desc("Do not warn about files that have no symbols"),
     cl::cat(LibtoolCategory), cl::init(false));
 
+static cl::opt<bool> WarningsAsErrors("warnings_as_errors",
+                                      cl::desc("Treat warnings as errors"),
+                                      cl::cat(LibtoolCategory),
+                                      cl::init(false));
+
 static const std::array<std::string, 3> StandardSearchDirs{
     "/lib",
     "/usr/lib",
@@ -370,10 +375,19 @@
         return Error::success();
       }
 
-      if (!NoWarningForNoSymbols && O->symbols().empty())
+      if (!NoWarningForNoSymbols && O->symbols().empty()) {
+        if (WarningsAsErrors)
+          return createFileError(
+              Member.MemberName,
+              createStringError(
+                  std::errc::invalid_argument,
+                  "has no symbols for architecture %s",
+                  O->getArchTriple().getArchName().str().c_str()));
+
         WithColor::warning() << "'" + Member.MemberName +
                                     "': has no symbols for architecture " +
                                     O->getArchTriple().getArchName() + "\n";
+      }
 
       uint64_t FileCPUID = getCPUID(FileCPUType, FileCPUSubtype);
       Builder.Data.MembersPerArchitecture[FileCPUID].push_back(
@@ -581,8 +595,11 @@
 
   const auto &NewMembers = DataOrError->MembersPerArchitecture;
 
-  if (Error E = checkForDuplicates(NewMembers))
+  if (Error E = checkForDuplicates(NewMembers)) {
+    if (WarningsAsErrors)
+      return E;
     WithColor::defaultWarningHandler(std::move(E));
+  }
 
   if (NewMembers.size() == 1)
     return writeArchive(OutputFile, NewMembers.begin()->second.getMembers(),
Index: llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test
===================================================================
--- llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test
+++ llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test
@@ -11,6 +11,11 @@
 
 # WARNING: warning: '[[PREFIX]]-x86_64-empty.o': has no symbols for architecture x86_64
 
+# RUN: not llvm-libtool-darwin -static -warnings_as_errors -o %t-error.lib %t-x86_64-empty.o 2>&1 | \
+# RUN:   FileCheck --check-prefix=ERROR %s -DPREFIX=%basename_t.tmp
+
+# ERROR: error: '[[PREFIX]]-x86_64-empty.o': has no symbols for architecture x86_64
+
 # RUN: llvm-libtool-darwin -no_warning_for_no_symbols -static -o %t.lib \
 # RUN:   %t-x86_64-empty.o 2>&1 | \
 # RUN:   FileCheck %s --allow-empty --implicit-check-not='warning:'
Index: llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
===================================================================
--- llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
+++ llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
@@ -58,6 +58,14 @@
 # DUPLICATE-INPUT-DAG: [[INPUTA]]
 # DUPLICATE-INPUT-DAG: [[INPUTB]]
 
+# RUN: not llvm-libtool-darwin -warnings_as_errors -static -o %t.lib %t-input1.o %t-input2.o %t-input1.o 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=ERROR-DUPLICATE-INPUT -DFILE=%basename_t.tmp-input1.o \
+# RUN:     -DINPUTA=%t-input1.o -DINPUTB=%t-input1.o
+
+# ERROR-DUPLICATE-INPUT:     error: file '[[FILE]]' was specified multiple times.
+# ERROR-DUPLICATE-INPUT-DAG: [[INPUTA]]
+# ERROR-DUPLICATE-INPUT-DAG: [[INPUTB]]
+
 ## Make sure we can combine object files with the same name if
 ## they are for different architectures.
 # RUN: mkdir -p %t/arm64 %t/armv7


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118931.405727.patch
Type: text/x-patch
Size: 3732 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220203/27ed0b06/attachment.bin>