From 4c12a75e69927c40b992d568187504b2ee268e92 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 2 Feb 2022 17:46:11 -0800 Subject: [PATCH] [llvm-libtool-darwin] Add -warnings_as_errors 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 Differential Revision: https://reviews.llvm.org/D118931 --- .../docs/CommandGuide/llvm-libtool-darwin.rst | 4 +++ .../create-static-lib.test | 8 ++++++ .../no-symbols-warning.test | 5 ++++ .../llvm-libtool-darwin.cpp | 25 +++++++++++++++---- 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/llvm/docs/CommandGuide/llvm-libtool-darwin.rst b/llvm/docs/CommandGuide/llvm-libtool-darwin.rst index 0fdd51da3092..87fa17688350 100644 --- a/llvm/docs/CommandGuide/llvm-libtool-darwin.rst +++ b/llvm/docs/CommandGuide/llvm-libtool-darwin.rst @@ -68,6 +68,10 @@ OPTIONS Do not warn about files that have no symbols. +.. option:: -warnings_as_errors + + Produce a non-zero exit status if any warnings are emitted. + .. option:: -o Specify the output file name. Must be specified exactly once. diff --git a/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test b/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test index 916f569fa985..8453e786e4cc 100644 --- a/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test +++ b/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 diff --git a/llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test b/llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test index 9dfc33453786..d1572b920910 100644 --- a/llvm/test/tools/llvm-libtool-darwin/no-symbols-warning.test +++ b/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:' diff --git a/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp b/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp index cd56e10a30af..0c429c0494db 100644 --- a/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp +++ b/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp @@ -98,6 +98,11 @@ static cl::opt NoWarningForNoSymbols( cl::desc("Do not warn about files that have no symbols"), cl::cat(LibtoolCategory), cl::init(false)); +static cl::opt WarningsAsErrors("warnings_as_errors", + cl::desc("Treat warnings as errors"), + cl::cat(LibtoolCategory), + cl::init(false)); + static const std::array StandardSearchDirs{ "/lib", "/usr/lib", @@ -370,10 +375,17 @@ private: return Error::success(); } - if (!NoWarningForNoSymbols && O->symbols().empty()) - WithColor::warning() << "'" + Member.MemberName + - "': has no symbols for architecture " + - O->getArchTriple().getArchName() + "\n"; + if (!NoWarningForNoSymbols && O->symbols().empty()) { + Error E = createFileError( + Member.MemberName, + createStringError(std::errc::invalid_argument, + "has no symbols for architecture %s", + O->getArchTriple().getArchName().str().c_str())); + + if (WarningsAsErrors) + return E; + WithColor::defaultWarningHandler(std::move(E)); + } uint64_t FileCPUID = getCPUID(FileCPUType, FileCPUSubtype); Builder.Data.MembersPerArchitecture[FileCPUID].push_back( @@ -581,8 +593,11 @@ static Error createStaticLibrary(const Config &C) { 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(),