[llvm-ar] Use target triple to deduce archive kind for bitcode inputs

[pirama]

Summary

When using full LTO on cross-compile settings, instead of generating the
default archive kind of the host platform, we could deduce the archive
kind based on the target triple.

This specifically addresses https://github.com/android/ndk/issues/1209
by making it possible to drop llvm-ar in place of GNU ar without extra
flags.

[danalbert]

Thanks for fixing this 👍

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

904

Looks like there are a few more options: https://llvm.org/doxygen/classllvm_1_1object_1_1Archive.html#ab288378fa8bfa0678dd25e36b5198a87. I think this regresses behavior for those other hosts.

[MaskRay]

https://llvm.org/doxygen/classllvm_1_1object_1_1Archive.html#ab288378fa8bfa0678dd25e36b5198a87. I think this regresses behavior for those other hosts.

*BSD may need the __.SYMDEF format.

Have you considered passing an explicit --format= via something like ARFLAGS?

[pirama]

Handle bitcode inputs with BSD target triple.

[pirama]

In D76461#1932516, @MaskRay wrote:

https://llvm.org/doxygen/classllvm_1_1object_1_1Archive.html#ab288378fa8bfa0678dd25e36b5198a87. I think this regresses behavior for those other hosts.

*BSD may need the __.SYMDEF format.

Have you considered passing an explicit --format= via something like ARFLAGS?

The goal is to avoid requiring all users of the NDK on Darwin from having to set --format= when switching to LLVM binutils. This format deduction only runs when no --format is provided and works for all cases except full LTO. Also based on the deduction based on ObjectFile at the beginning of this function, it seems that all BSD environments require --format=bsd to be specified because it uses GNU archive kind for all ELF files.

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

904

Note that getDefaultForHost is also incorrect on BSD (it returns K_GNU if the host triple is not Darwin). So this doesn't exactly regress that behavior. But it's good to handle this correctly for the narrow cases where we can. I will fix getDefaultForHost in a follow up as it'd require a different test.

I also don't think llvm-ar is set up to pass through the other archive kinds (GNU64, DARWIN64, COFF).

[pirama]

This comment was removed by pirama.

[pirama]

Apply clang-format.

[MaskRay]

llvm/test/tools/llvm-ar/Inputs/bsd.ll

1

The ModuleID comment is not needed now.

I suggest that you use RUN: echo -e '....' > %t.bsd.ll to inline the trivial test. Also drop dso_local

4

Since you dropped the attribute group, you need to drop the comment as well.

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

905

I will ask some BSD people whether this is correct.

(Honestly I don't like the logic much.)

[MaskRay]

llvm/include/llvm/ADT/Triple.h

506

We should not need this helper.

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

905

It seems that we can simply use K_GNU. Edited from Bdragon's words:

K_BSD, "__.SYMDEF", the 4.4BSD archive format (under development in 4.3BSD and finalized in 4.4BSD), is no longer as common use at the moment as people might think it is.

"openbsd appears to have tossed their ar and ranlib completely 6 years ago, and NetBSD switched to binutils ar 21 years ago."

So the only difference is between Darwin and GNU.

[pirama]

Simplify test.

[MaskRay]

llvm/test/tools/llvm-ar/lto-kind-from-triple.test

2

Newly added tests (e.g. symtab.test) start to use ## for comments and # for RUN/CHECK lines even if llvm-ar does not read anything from the text file.

3

Darwin archive format is the only different platform now. We can special case it and treat everything else GNU.

5

Use echo -e '...' to avoid quoting "

22

Can be deleted because 4.4BSD archive format is out of fashion nowadays.

[pirama]

Use K_GNU for all ELF targets.

[pirama]

Treat any bitode with non-Darwin triple as GNU.
Test cleanup.

[pirama]

llvm/test/tools/llvm-ar/lto-kind-from-triple.test

3

Assuming this is just a general observation and no action needed.

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

905

Thanks for following up on this. I've updated the latest patch to use K_GNU for all non-Darwin triples. It was previously set to K_GNU only when OSBinFormat was ELF. The new logic is in sync with what this deduction is doing everywhere else:

non-Darwin object file -> K_GNU
non-Darwin host -> K_GNU.

[pirama]

Use isOSDarwin() instead of isOSBinFormatMachO()

[MaskRay]

llvm/test/tools/llvm-ar/lto-kind-from-triple.test

21

Add another test demonstrating that adding %t.macho.ll will not change GNU format to darwin format.

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

897

Optional: you can write

if (auto ObjOrErr = ...) {
} else {

consumeError(ObjOrErr.takeError());

}

902

Optional: Triple(IRObject.getTargetTriple()).isOSDarwin()

906

Nit: capitalize the initial word in the comment.

[pirama]

Cleanup based on review comments.

[pirama]

Merged. Thanks for the review @MaskRay.

llvm/include/llvm/ADT/Triple.h

public:
   bool isOSOpenBSD() const {
     return getOS() == Triple::OpenBSD;
   }
 
   bool isOSFreeBSD() const {
     return getOS() == Triple::FreeBSD;
   }
 
+  bool isOSBSDLike() const {
+    return isOSNetBSD() || isOSOpenBSD() || isOSFreeBSD();
+  }
+
   bool isOSFuchsia() const {
     return getOS() == Triple::Fuchsia;
   }
 
   bool isOSDragonFly() const { return getOS() == Triple::DragonFly; }
 
   bool isOSSolaris() const {
     return getOS() == Triple::Solaris;

llvm/test/tools/llvm-ar/Inputs/bsd.ll

+; ModuleID = 'lto_gnu_obj.bc'
+target triple = "x86_64-unknown-freebsd"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @_Z3foov() {
+entry:
+  ret void
+}

llvm/test/tools/llvm-ar/Inputs/gnu.ll

+; ModuleID = 'lto_gnu_obj.bc'
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @_Z3foov() {
+entry:
+  ret void
+}

llvm/test/tools/llvm-ar/Inputs/macho.ll

+; ModuleID = 'lto_gnu_obj.bc'
+target triple = "x86_64-apple-macosx10.9"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local void @_Z3foov() {
+entry:
+  ret void
+}

llvm/test/tools/llvm-ar/lto-kind-from-triple.test

+Ensure that we generate a GNU style archive if the first input is a bitcode
+file with a GNU target triple (absence of __.SYMDEF in the archive).
+
+RUN: llvm-as -o %t.o %S/Inputs/gnu.ll
+
+RUN: rm -f %t.ar
+RUN: llvm-ar crs %t.ar %t.o
+RUN: not grep -q __.SYMDEF %t.ar
+
+Ensure that we generate a MachO style archive if the first input is a bitcode
+file with a MachO target triple (presence of __.SYMDEF in the archive).
+
+RUN: llvm-as -o %t.o %S/Inputs/macho.ll
+
+RUN: rm -f %t.ar
+RUN: llvm-ar crs %t.ar %t.o
+RUN: grep -q __.SYMDEF %t.ar
+
+Ensure that we generate a BSD style archive if the first input is a bitcode
+file with a BSD target triple (presence of __.SYMDEF in the archive).
+
+RUN: llvm-as -o %t.o %S/Inputs/bsd.ll
+
+RUN: rm -f %t.ar
+RUN: llvm-ar crs %t.ar %t.o
+RUN: grep -q __.SYMDEF %t.ar
+

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

 //===-- llvm-ar.cpp - LLVM archive librarian utility ----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Builds up (relatively) standard unix archive files (.a) containing LLVM
 // bitcode or other files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/BinaryFormat/Magic.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/SymbolicFile.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Host.h"

 
 static object::Archive::Kind getDefaultForHost() {
   return Triple(sys::getProcessTriple()).isOSDarwin()
              ? object::Archive::K_DARWIN
              : object::Archive::K_GNU;
 }
 
 static object::Archive::Kind getKindFromMember(const NewArchiveMember &Member) {
+  auto MemBufferRef = Member.Buf->getMemBufferRef();
   Expected<std::unique_ptr<object::ObjectFile>> OptionalObject =
-      object::ObjectFile::createObjectFile(Member.Buf->getMemBufferRef());
+      object::ObjectFile::createObjectFile(MemBufferRef);
 
   if (OptionalObject)
     return isa<object::MachOObjectFile>(**OptionalObject)
                ? object::Archive::K_DARWIN
                : object::Archive::K_GNU;
 
   // squelch the error in case we had a non-object file
   consumeError(OptionalObject.takeError());
+
+  // If we're adding a bitcode file to the archive, detect the Archive kind
+  // based on the target triple.
+  LLVMContext Context;
+  if (identify_magic(MemBufferRef.getBuffer()) == file_magic::bitcode) {
+    auto ObjOrErr = object::SymbolicFile::createSymbolicFile(
+        MemBufferRef, file_magic::bitcode, &Context);
+    if (ObjOrErr) {
+      auto &IRObject = cast<object::IRObjectFile>(**ObjOrErr);
+      Triple TargetTriple(IRObject.getTargetTriple());
+      if (TargetTriple.isOSBinFormatMachO())
+        return object::Archive::K_DARWIN;
+      else if (TargetTriple.isOSBinFormatELF()) {
+        return TargetTriple.isOSBSDLike() ? object::Archive::K_BSD
+                                          : object::Archive::K_GNU;
+      }
+    }
+    // squelch the error in case this was not a SymbolicFile.
+    consumeError(ObjOrErr.takeError());
+  }
+
   return getDefaultForHost();
 }
 
 static void performWriteOperation(ArchiveOperation Operation,
                                   object::Archive *OldArchive,
                                   std::unique_ptr<MemoryBuffer> OldArchiveBuf,
                                   std::vector<NewArchiveMember> *NewMembersP) {
   std::vector<NewArchiveMember> NewMembers;