diff --git a/llvm/test/tools/llvm-libtool-darwin/L-and-l.test b/llvm/test/tools/llvm-libtool-darwin/L-and-l.test --- a/llvm/test/tools/llvm-libtool-darwin/L-and-l.test +++ b/llvm/test/tools/llvm-libtool-darwin/L-and-l.test @@ -62,6 +62,20 @@ # RUN: llvm-nm --print-armap %t.lib | \ # RUN: FileCheck %s --check-prefix=SINGLE-SYMBOLS -DPREFIX=%basename_t.tmp --match-full-lines +## Check that if two different files with the same names are explicitly +## specified, the command gives a warning. +# RUN: cp %t-input2.o %t/dirname +# RUN: llvm-libtool-darwin -static -o %t.lib \ +# RUN: %t/dirname/%basename_t.tmp-input2.o %t-input2.o 2>&1 | \ +# RUN: FileCheck %s --check-prefix=DUPLICATE-INPUT \ +# RUN: -DFILE=%basename_t.tmp-input2.o \ +# RUN: -DINPUTA=%t/dirname/%basename_t.tmp-input2.o \ +# RUN: -DINPUTB=%t-input2.o + +# DUPLICATE-INPUT: warning: file '[[FILE]]' was specified multiple times. +# DUPLICATE-INPUT-DAG: [[INPUTA]] +# DUPLICATE-INPUT-DAG: [[INPUTB]] + ## -l option combined with an input file: # RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o -linput2 -L%t/dirname # RUN: llvm-ar t %t.lib | \ @@ -69,22 +83,23 @@ # RUN: llvm-nm --print-armap %t.lib | \ # RUN: FileCheck %s --check-prefix=CHECK-SYMBOLS -DPREFIX=%basename_t.tmp --match-full-lines +## Specify the same file with a -l option and an input file: +# RUN: rm -rf %t/copy +# RUN: mkdir -p %t/copy +# RUN: cp %t-input1.o %t/copy +# RUN: llvm-libtool-darwin -static -o %t.lib \ +# RUN: %t/copy/%basename_t.tmp-input1.o -l%basename_t.tmp-input1.o -L%t/copy 2>&1 | \ +# RUN: FileCheck %s --check-prefix=DUPLICATE-INPUT -DFILE=%basename_t.tmp-input1.o \ +# RUN: -DINPUTA=%t/copy/%basename_t.tmp-input1.o \ +# RUN: -DINPUTB=%t/copy/%basename_t.tmp-input1.o + ## Specify same -l option twice: -## cctools' libtool raises a warning in this case. -## The warning is not yet implemented for llvm-libtool-darwin. -# RUN: llvm-libtool-darwin -static -o %t.lib -l%basename_t.tmp-input2.o -l%basename_t.tmp-input2.o -L%T -# RUN: llvm-ar t %t.lib | \ -# RUN: FileCheck %s --check-prefix=DOUBLE-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp -# RUN: llvm-nm --print-armap %t.lib | \ -# RUN: FileCheck %s --check-prefix=DOUBLE-SYMBOLS -DPREFIX=%basename_t.tmp --match-full-lines - -# DOUBLE-NAMES: [[PREFIX]]-input2.o -# DOUBLE-NAMES-NEXT: [[PREFIX]]-input2.o - -# DOUBLE-SYMBOLS: Archive map -# DOUBLE-SYMBOLS-NEXT: _symbol2 in [[PREFIX]]-input2.o -# DOUBLE-SYMBOLS-NEXT: _symbol2 in [[PREFIX]]-input2.o -# DOUBLE-SYMBOLS-EMPTY: +# RUN: llvm-libtool-darwin -static -o %t.lib -l%basename_t.tmp-input1.o \ +# RUN: -l%basename_t.tmp-input1.o -L%t/copy 2>&1 | \ +# RUN: FileCheck %s --check-prefix=DUPLICATE-INPUT \ +# RUN: -DFILE=%basename_t.tmp-input1.o \ +# RUN: -DINPUTA=%t/copy/%basename_t.tmp-input1.o \ +# RUN: -DINPUTB=%t/copy/%basename_t.tmp-input1.o ## Check that an error is thrown when the input library cannot be found: # RUN: not llvm-libtool-darwin -static -o %t.lib -lfile-will-not-exist.o 2>&1 | \ diff --git a/llvm/test/tools/llvm-libtool-darwin/archive-flattening.test b/llvm/test/tools/llvm-libtool-darwin/archive-flattening.test --- a/llvm/test/tools/llvm-libtool-darwin/archive-flattening.test +++ b/llvm/test/tools/llvm-libtool-darwin/archive-flattening.test @@ -59,6 +59,17 @@ # BOTH-SYMBOLS-NEXT: _symbol1 in [[PREFIX]]-input1.o # BOTH-SYMBOLS-EMPTY: +# RUN: llvm-libtool-darwin -static -o %t.lib %t-x86_64.bc %t.correct.ar %t-input1.o 2>&1 | \ +# RUN: FileCheck %s --check-prefix=DUPLICATE-INPUT -DFILEA=%basename_t.tmp-input1.o \ +# RUN: -DPATHA=%t-input1.o -DFILEB=%basename_t.tmp-x86_64.bc -DPATHB=%t-x86_64.bc -DPATHCORRECT=%t.correct.ar + +# DUPLICATE-INPUT: warning: file '[[FILEA]]' was specified multiple times. +# DUPLICATE-INPUT-DAG: [[PATHA]] +# DUPLICATE-INPUT-DAG: [[PATHCORRECT]] +# DUPLICATE-INPUT: file '[[FILEB]]' was specified multiple times. +# DUPLICATE-INPUT-DAG: [[PATHB]] +# DUPLICATE-INPUT-DAG: [[PATHCORRECT]] + ## Cannot read archive: # RUN: echo '!' > %t-invalid-archive.lib # RUN: echo 'invalid' >> %t-invalid-archive.lib diff --git a/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test b/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test --- a/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test +++ b/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test @@ -50,21 +50,19 @@ # OVERWRITE-SYMBOLS-EMPTY: ## Duplicate a binary: -## cctools' libtool raises a warning in this case. -## The warning is not yet implemented for llvm-libtool-darwin. # RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o %t-input2.o %t-input1.o 2>&1 | \ -# RUN: FileCheck %s --allow-empty --implicit-check-not={{.}} -# RUN: llvm-ar t %t.lib | \ -# RUN: FileCheck %s --check-prefix=DUPLICATE-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp -# RUN: llvm-nm --print-armap %t.lib | \ -# RUN: FileCheck %s --check-prefix=DUPLICATE-SYMBOLS -DPREFIX=%basename_t.tmp --match-full-lines +# RUN: FileCheck %s --check-prefix=DUPLICATE-INPUT -DFILE=%basename_t.tmp-input1.o \ +# RUN: -DINPUTA=%t-input1.o -DINPUTB=%t-input1.o -# DUPLICATE-NAMES: [[PREFIX]]-input1.o -# DUPLICATE-NAMES-NEXT: [[PREFIX]]-input2.o -# DUPLICATE-NAMES-NEXT: [[PREFIX]]-input1.o +# DUPLICATE-INPUT: warning: file '[[FILE]]' was specified multiple times. +# DUPLICATE-INPUT-DAG: [[INPUTA]] +# DUPLICATE-INPUT-DAG: [[INPUTB]] -# DUPLICATE-SYMBOLS: Archive map -# DUPLICATE-SYMBOLS-NEXT: _symbol1 in [[PREFIX]]-input1.o -# DUPLICATE-SYMBOLS-NEXT: _symbol2 in [[PREFIX]]-input2.o -# DUPLICATE-SYMBOLS-NEXT: _symbol1 in [[PREFIX]]-input1.o -# DUPLICATE-SYMBOLS-EMPTY: +## Make sure we can combine object files with the same name if +## they are for different architectures. +# RUN: mkdir -p %t/arm64 %t/armv7 +# RUN: llvm-as %S/Inputs/arm64-ios.ll -o %t/arm64/out.bc +# RUN: llvm-as %S/Inputs/armv7-ios.ll -o %t/armv7/out.bc +# # Command output should be empty. +# RUN: llvm-libtool-darwin -static %t/arm64/out.bc %t/armv7/out.bc -o %t.lib 2>&1 | \ +# RUN: FileCheck %s --implicit-check-not=warning: --allow-empty diff --git a/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp b/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp --- a/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp +++ b/llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp @@ -23,6 +23,7 @@ #include "llvm/Support/LineIterator.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/WithColor.h" +#include "llvm/Support/raw_ostream.h" #include "llvm/TextAPI/Architecture.h" #include #include @@ -203,9 +204,29 @@ } } +// NewArchiveMemberList instances serve as collections of archive members and +// information about those members. +class NewArchiveMemberList { + std::vector Members; + // This vector contains the file that each NewArchiveMember from Members came + // from. Therefore, it has the same size as Members. + std::vector Files; + +public: + // Add a NewArchiveMember and the file it came from to the list. + void pushBack(NewArchiveMember &&Member, StringRef File) { + Members.push_back(std::move(Member)); + Files.push_back(File); + } + + const std::vector &getMembers() const { return Members; } + + const std::vector &getFiles() const { return Files; } +}; + struct MembersPerArchitectureMap - : public std::map> { - typedef std::map> Parent; + : public std::map { + typedef std::map Parent; using Parent::Parent; MembersPerArchitectureMap(const MembersPerArchitectureMap &other) = delete; @@ -234,7 +255,7 @@ Expected build() { for (StringRef FileName : InputFiles) - if (Error E = addMember(FileName)) + if (Error E = AddMember(*this, FileName)()) return E; if (!ArchType.empty()) { @@ -249,24 +270,59 @@ } private: + class AddMember { + MembersBuilder& Builder; + StringRef FileName; + + public: + AddMember(MembersBuilder &Builder, StringRef FileName) : Builder(Builder), FileName(FileName) {} + + Error operator()() { + Expected NewMemberOrErr = + NewArchiveMember::getFile(FileName, Builder.C.Deterministic); + if (!NewMemberOrErr) + return createFileError(FileName, NewMemberOrErr.takeError()); + auto &NewMember = *NewMemberOrErr; + + // For regular archives, use the basename of the object path for the member + // name. + NewMember.MemberName = sys::path::filename(NewMember.MemberName); + file_magic Magic = identify_magic(NewMember.Buf->getBuffer()); + + // Flatten archives. + if (Magic == file_magic::archive) + return addArchiveMembers(std::move(NewMember), FileName); + + // Flatten universal files. + if (Magic == file_magic::macho_universal_binary) + return addUniversalMembers(std::move(NewMember), FileName); + + // Bitcode files. + if (Magic == file_magic::bitcode) + return verifyAndAddIRObject(std::move(NewMember)); + + return verifyAndAddMachOObject(std::move(NewMember)); + } + + private: // Check that a file's architecture [FileCPUType, FileCPUSubtype] // matches the architecture specified under -arch_only flag. bool acceptFileArch(uint32_t FileCPUType, uint32_t FileCPUSubtype) { - if (C.ArchCPUType != FileCPUType) + if (Builder.C.ArchCPUType != FileCPUType) return false; - switch (C.ArchCPUType) { + switch (Builder.C.ArchCPUType) { case MachO::CPU_TYPE_ARM: case MachO::CPU_TYPE_ARM64_32: case MachO::CPU_TYPE_X86_64: - return C.ArchCPUSubtype == FileCPUSubtype; + return Builder.C.ArchCPUSubtype == FileCPUSubtype; case MachO::CPU_TYPE_ARM64: - if (C.ArchCPUSubtype == MachO::CPU_SUBTYPE_ARM64_ALL) + if (Builder.C.ArchCPUSubtype == MachO::CPU_SUBTYPE_ARM64_ALL) return FileCPUSubtype == MachO::CPU_SUBTYPE_ARM64_ALL || FileCPUSubtype == MachO::CPU_SUBTYPE_ARM64_V8; else - return C.ArchCPUSubtype == FileCPUSubtype; + return Builder.C.ArchCPUSubtype == FileCPUSubtype; default: return true; @@ -303,7 +359,7 @@ WithColor::warning() << Member.MemberName + " has no symbols\n"; uint64_t FileCPUID = getCPUID(FileCPUType, FileCPUSubtype); - Data.NewMembers[FileCPUID].push_back(std::move(Member)); + Builder.Data.NewMembers[FileCPUID].pushBack(std::move(Member), FileName); return Error::success(); } @@ -334,13 +390,13 @@ } uint64_t FileCPUID = getCPUID(*FileCPUTypeOrErr, *FileCPUSubTypeOrErr); - Data.NewMembers[FileCPUID].push_back(std::move(Member)); + Builder.Data.NewMembers[FileCPUID].pushBack(std::move(Member), FileName); return Error::success(); } Error addChildMember(const object::Archive::Child &M) { Expected NewMemberOrErr = - NewArchiveMember::getOldMember(M, C.Deterministic); + NewArchiveMember::getOldMember(M, Builder.C.Deterministic); if (!NewMemberOrErr) return NewMemberOrErr.takeError(); auto &NewMember = *NewMemberOrErr; @@ -375,7 +431,7 @@ // Update vector FileBuffers with the MemoryBuffers to transfer // ownership. - Data.FileBuffers.push_back(std::move(NewMember.Buf)); + Builder.Data.FileBuffers.push_back(std::move(NewMember.Buf)); return Error::success(); } @@ -440,36 +496,11 @@ // Update vector FileBuffers with the MemoryBuffers to transfer // ownership. - Data.FileBuffers.push_back(std::move(NewMember.Buf)); + Builder.Data.FileBuffers.push_back(std::move(NewMember.Buf)); return Error::success(); } - Error addMember(StringRef FileName) { - Expected NewMemberOrErr = - NewArchiveMember::getFile(FileName, C.Deterministic); - if (!NewMemberOrErr) - return createFileError(FileName, NewMemberOrErr.takeError()); - auto &NewMember = *NewMemberOrErr; - - // For regular archives, use the basename of the object path for the member - // name. - NewMember.MemberName = sys::path::filename(NewMember.MemberName); - file_magic Magic = identify_magic(NewMember.Buf->getBuffer()); - - // Flatten archives. - if (Magic == file_magic::archive) - return addArchiveMembers(std::move(NewMember), FileName); - - // Flatten universal files. - if (Magic == file_magic::macho_universal_binary) - return addUniversalMembers(std::move(NewMember), FileName); - - // Bitcode files. - if (Magic == file_magic::bitcode) - return verifyAndAddIRObject(std::move(NewMember)); - - return verifyAndAddMachOObject(std::move(NewMember)); - } + }; MembersData Data; const Config &C; @@ -489,6 +520,42 @@ return Slices; } +static Error +checkForDuplicates(const MembersPerArchitectureMap &MembersPerArch) { + for (const auto &M : MembersPerArch) { + const auto &Members = M.second.getMembers(); + const auto &Files = M.second.getFiles(); + std::map> MembersToFiles; + for (auto iterators = std::make_pair(Members.cbegin(), Files.cbegin()); + iterators.first != Members.cend(); + // which should imply iterators.second != Files.cend() + ++iterators.first, ++iterators.second) { + MembersToFiles[iterators.first->MemberName].push_back(*iterators.second); + } + + std::string ErrorData; + raw_string_ostream ErrorStream(ErrorData); + for (const std::pair> &pair : + MembersToFiles) { + if (pair.second.size() > 1) { + ErrorStream << "file '" << pair.first.str() + << "' was specified multiple times.\n"; + + for (StringRef OriginalFile : pair.second) { + ErrorStream << "in: " << OriginalFile.str() << '\n'; + } + ErrorStream << '\n'; + } + } + + ErrorStream.flush(); + if (ErrorData.size() > 0) { + return createStringError(std::errc::invalid_argument, ErrorData.c_str()); + } + } + return Error::success(); +} + static Error createStaticLibrary(const Config &C) { MembersBuilder Builder(C); auto DataOrError = Builder.build(); @@ -498,18 +565,21 @@ const auto &NewMembers = DataOrError->NewMembers; + if (Error E = checkForDuplicates(NewMembers)) { + WithColor::defaultWarningHandler(std::move(E)); + } + if (NewMembers.size() == 1) { - return writeArchive(OutputFile, NewMembers.begin()->second, + return writeArchive(OutputFile, NewMembers.begin()->second.getMembers(), /*WriteSymtab=*/true, /*Kind=*/object::Archive::K_DARWIN, C.Deterministic, /*Thin=*/false); } SmallVector, 2> OutputBinaries; - for (const std::pair> &M : - NewMembers) { + for (const std::pair &M : NewMembers) { Expected> OutputBufferOrErr = - writeArchiveToBuffer(M.second, + writeArchiveToBuffer(M.second.getMembers(), /*WriteSymtab=*/true, /*Kind=*/object::Archive::K_DARWIN, C.Deterministic,