[llvm-ifs] Add option to use InterfaceStub library

[haowei]

Summary

This change add an option to allow llvm-ifs to use InterfaceStub library (introduced in llvm-elfabi) when generate stub ELF file. This is the first step trying to unify llvm-ifs and llvm-elfabi tools.

There are a few questions I would like to ask before I make further unification changes:

1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

2. One of the design goal of InterfaceStub is to generate minimal ELF stub that is OK for linking. So it strips away sections like .data, .text, .rodata , which are present (but empty) in the ELF stub generated from yaml2obj . Is there any use cases that require these sections?

Thanks.

[plotfi]

1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

2. One of the design goal of InterfaceStub is to generate minimal ELF stub that is OK for linking. So it strips away sections like .data, .text, .rodata , which are present (but empty) in the ELF stub generated from yaml2obj . Is there any use cases that require these sections?

Nah, I think those can go.

BTW, thanks for working on this. I will add further comments once I read over the code.

[plotfi]

In D94461#2491946, @plotfi wrote:

1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

I meant to add, as part of llvm-ifs being invoked by clang the idea was that clang would just forward its triple and llvm-ifs would decide what to do with it because we wanted the ifs format to be kinda platform agnostic but we wanted llvm-ifs to potentially be able to generate stubs for multiple platforms in the future (like MS .libs and tbds etc too).

2. One of the design goal of InterfaceStub is to generate minimal ELF stub that is OK for linking. So it strips away sections like .data, .text, .rodata , which are present (but empty) in the ELF stub generated from yaml2obj . Is there any use cases that require these sections?

Nah, I think those can go.

BTW, thanks for working on this. I will add further comments once I read over the code.

[plotfi]

llvm/tools/llvm-ifs/llvm-ifs.cpp

58

Put a comment here to the effect that the InterfaceStubs backend is newer than "Clang InterfaceStubs" which originally used the yaml2obj backend. Eventually I assume we want to flip the default for this too. Can also you explicitly set the default here to make that clear?

573

What happens if UseInterfaceStub == true but Action is not "write-bin" ? Shouldn't it generate a merged llvm-elftapi text file or something like that or at least error out?

[phosek]

In D94461#2491947, @plotfi wrote:

In D94461#2491946, @plotfi wrote:

1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

I meant to add, as part of llvm-ifs being invoked by clang the idea was that clang would just forward its triple and llvm-ifs would decide what to do with it because we wanted the ifs format to be kinda platform agnostic but we wanted llvm-ifs to potentially be able to generate stubs for multiple platforms in the future (like MS .libs and tbds etc too).

Would you need a full triple in that case? Other binary tools like linkers doesn't use a full triple either, they use just e_machine and e_ident. Would that be sufficient?

[plotfi]

In D94461#2492369, @phosek wrote:

In D94461#2491947, @plotfi wrote:

In D94461#2491946, @plotfi wrote:

1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

I meant to add, as part of llvm-ifs being invoked by clang the idea was that clang would just forward its triple and llvm-ifs would decide what to do with it because we wanted the ifs format to be kinda platform agnostic but we wanted llvm-ifs to potentially be able to generate stubs for multiple platforms in the future (like MS .libs and tbds etc too).

Would you need a full triple in that case? Other binary tools like linkers doesn't use a full triple either, they use just e_machine and e_ident. Would that be sufficient?

That could work.

[haowei]

fix some code style issues.

[haowei]

llvm/tools/llvm-ifs/llvm-ifs.cpp

58

I slightly modified the flag desc, is that enough? Also I explicitly set the default to false.

573

In diff 1, when UseInterfaceStub == true but Action is not "write-bin", it will fallback to function writeIfso function, which will generate an ifs text file correctly. But I think it miss the case when UseInterfaceStub == true but output format is TBD, which will output nothing.

In diff3, InterfaceStub will only be used when UseInterfaceStub == true && Action == "write-bin" && Format != "TBD".

[compnerd]

The full triple is needed in some cases: armv7-unknown-linux-gnueabi vs armv7-unknown-linux-gnueabihf vs armv7-unknown-linux-eabihf vs armv7-unknown-linux-eabi are all different. Additionally, there is nothing that prevents you from doing something like: armv7-unknown-linux-gnueabihf-coff or armv7-unknown-linux-gnueabihf-macho to generate COFF or MachO object files that are reading in additional bits of ABI information and the libc from the target triple. I really think that preserving the target triple is best.

[plotfi]

In D94461#2494667, @compnerd wrote:

The full triple is needed in some cases: armv7-unknown-linux-gnueabi vs armv7-unknown-linux-gnueabihf vs armv7-unknown-linux-eabihf vs armv7-unknown-linux-eabi are all different. Additionally, there is nothing that prevents you from doing something like: armv7-unknown-linux-gnueabihf-coff or armv7-unknown-linux-gnueabihf-macho to generate COFF or MachO object files that are reading in additional bits of ABI information and the libc from the target triple. I really think that preserving the target triple is best.

I think what @phosek means is, they want to generate an elf to reuse for multiple targets. Assuming you have to use whatever you're given as the name mangling ABI would it be possible to just use some kind of " e_machine and e_ident" triple?

[compnerd]

You mean use armv7---elf as a triple? While that is possible, keep in mind that it might change the ABI of the generated interfaces. While it is unlikely to matter as the ifso cannot be used at runtime, if the linker attempts to do something like symbolic resolution of ABI dependent specialization (e.g. strcmp.eabi vs strcmp where the latter is generic), that could matter. However, adding support to explicitly override the encoded triple based on the user's input is reasonable (though might deserve a warning that the triple is being overridden).

[plotfi]

In D94461#2494710, @compnerd wrote:

You mean use armv7---elf as a triple? While that is possible, keep in mind that it might change the ABI of the generated interfaces. While it is unlikely to matter as the ifso cannot be used at runtime, if the linker attempts to do something like symbolic resolution of ABI dependent specialization (e.g. strcmp.eabi vs strcmp where the latter is generic), that could matter. However, adding support to explicitly override the encoded triple based on the user's input is reasonable (though might deserve a warning that the triple is being overridden).

I think an override is reasonable.

[plotfi]

Ping? Haven't seen any chatter in a while. Anything new here? I don't want to hold you folks up.

[haowei]

In D94461#2534861, @plotfi wrote:

Ping? Haven't seen any chatter in a while. Anything new here? I don't want to hold you folks up.

Sorry I was on call for past week and thing got a bit busy.

For this single patch, I don't see there is anything blocking it from landing, since it it just adding an optional backend for ELF generation. So could you give me a LGTM so I can land it please?

In D94461#2494710, @compnerd wrote:

You mean use armv7---elf as a triple? While that is possible, keep in mind that it might change the ABI of the generated interfaces. While it is unlikely to matter as the ifso cannot be used at runtime, if the linker attempts to do something like symbolic resolution of ABI dependent specialization (e.g. strcmp.eabi vs strcmp where the latter is generic), that could matter. However, adding support to explicitly override the encoded triple based on the user's input is reasonable (though might deserve a warning that the triple is being overridden).

Could you explain the use case a little bit more please? I am not an expert on ELF and have very little knowledge on ARM specifics so please correct me if I said something wrong. I think putting some triple like data in the Text file and use command line options as a override is a reasonable solution. But it looks like the current writeElfStub function only use the triple to write e_ident[EI_CLASS], e_ident[EI_DATA] and e_machine. If that the case, would e_ident and e_machine just be sufficient? Do we really need a whole triple here? For the case of strcmp.eabi vs strcmp, how do these 2 encoded in the ELF file? It looks like both ifs and elfabi put a 0 in e_ident[EI_OSABI].

[plotfi]

If @compnerd has no gripes go ahead and land. LGTM.
Before landing, please make sure to run the check-clang tests as well because the interface stubs generation on the clang driver side invokes llvm-ifs.

Thanks Guys!

llvm/test/tools/llvm-ifs/func.ifs

 # RUN: llvm-ifs -action write-ifs -o - %s %S/object.ifs | \
 # RUN: FileCheck %s --check-prefixes=CHECK-IFS
 
 # RUN: llvm-ifs -action write-bin -o - %s %S/object.ifs | \
 # RUN: llvm-readelf --all - | FileCheck %s --check-prefixes=CHECK-ELF
 
+# RUN: llvm-ifs -action write-bin -o - %s %S/object.ifs -use-interfacestub | \
+# RUN: llvm-readelf --all - | FileCheck %s --check-prefixes=CHECK-ELF
+
 # RUN: llvm-ifs -action write-bin -force-format TBD -o - %s %S/object.ifs | \
 # RUN: FileCheck %s --check-prefixes=CHECK-DARWIN-TBD3
 
 # RUN: llvm-ifs -action write-ifs -o - %s %s | \
 # RUN: FileCheck %s --check-prefixes=CHECK-MERGE-IFS
 
 # CHECK-IFS: --- !experimental-ifs-v2
 # CHECK-IFS-NEXT: IfsVersion: 2.0

llvm/tools/llvm-ifs/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
+  InterfaceStub
   Object
   Support
   TextAPI
   ObjectYAML
   )
 
 add_llvm_tool(llvm-ifs
   llvm-ifs.cpp
   )

llvm/tools/llvm-ifs/llvm-ifs.cpp

 //===- llvm-ifs.cpp -------------------------------------------------------===//
 //
 // 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
 //
 //===-----------------------------------------------------------------------===/
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/InterfaceStub/ELFObjHandler.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"

 
 static cl::list<std::string> InputFilenames(cl::Positional,
                                             cl::desc("<input ifs files>"),
                                             cl::ZeroOrMore);
 
 static cl::opt<std::string> OutputFilename("o", cl::desc("<output file>"),
                                            cl::value_desc("path"));
 
+static cl::opt<bool> UseInterfaceStub(
+    "use-interfacestub",
+    cl::desc("Write output ELF file using latest InterfaceStub backend"),
+    cl::init(false));
+
 enum class IFSSymbolType {
   NoType = 0,
   Object,
   Func,
   // Type information is 4 bits, so 16 is safely out of range.
   Unknown = 16,
 };
 
static int writeElfStub(const Triple &T, const std::vector<IFSSymbol> &Symbols,
 
   yaml::Input YIn(YamlStr);
   auto ErrHandler = [](const Twine &Msg) {
     WithColor::error(errs(), "llvm-ifs") << Msg << "\n";
   };
   return convertYAML(YIn, Out, ErrHandler) ? 0 : 1;
 }
 
-static int writeIfso(const IFSStub &Stub, bool IsWriteIfs, raw_ostream &Out) {
+static elfabi::ELFTarget convertIFSStub(const IFSStub &IfsStub,
+                                        elfabi::ELFStub &ElfStub) {
+  ElfStub.TbeVersion = IfsStub.IfsVersion;
+  ElfStub.SoName = IfsStub.SOName;
+  // TODO: Support more archs and targets.
+  Triple IFSTriple(IfsStub.Triple);
+  elfabi::ELFTarget Target = elfabi::ELFTarget::ELF64LE;
+  switch (IFSTriple.getArch()) {
+  case Triple::ArchType::aarch64:
+    ElfStub.Arch = (elfabi::ELFArch)ELF::EM_AARCH64;
+    break;
+  case Triple::ArchType::x86_64:
+    ElfStub.Arch = (elfabi::ELFArch)ELF::EM_X86_64;
+    break;
+  default:
+    ElfStub.Arch = (elfabi::ELFArch)ELF::EM_NONE;
+  }
+  ElfStub.NeededLibs = IfsStub.NeededLibs;
+  for (const IFSSymbol &IfsSymbol : IfsStub.Symbols) {
+    elfabi::ELFSymbol ElfSymbol(IfsSymbol.Name);
+    switch (IfsSymbol.Type) {
+    case IFSSymbolType::Func:
+      ElfSymbol.Type = elfabi::ELFSymbolType::Func;
+      break;
+    case IFSSymbolType::NoType:
+      ElfSymbol.Type = elfabi::ELFSymbolType::NoType;
+      break;
+    case IFSSymbolType::Object:
+      ElfSymbol.Type = elfabi::ELFSymbolType::Object;
+      break;
+    default:
+      ElfSymbol.Type = elfabi::ELFSymbolType::Unknown;
+      break;
+      // TODO: Add support for TLS?
+    }
+    ElfSymbol.Size = IfsSymbol.Size;
+    ElfSymbol.Undefined = false;
+    ElfSymbol.Weak = IfsSymbol.Weak;
+    ElfSymbol.Warning = IfsSymbol.Warning;
+    ElfStub.Symbols.insert(ElfSymbol);
+  }
+  return Target;
+}
+
+static int writeIfso(const IFSStub &Stub, bool IsWriteIfs) {
+  std::string ObjectFileFormat =
+      ForceFormat.empty() ? Stub.ObjectFileFormat : ForceFormat;
+
+  // Use InterfaceStub library if the option is enabled and output
+  // format is ELF.
+  if (UseInterfaceStub && (!IsWriteIfs) && ObjectFileFormat != "TBD") {
+    elfabi::ELFStub ElfStub;
+    elfabi::ELFTarget Target = convertIFSStub(Stub, ElfStub);
+    Error BinaryWriteError =
+        elfabi::writeBinaryStub(OutputFilename, ElfStub, Target);
+    if (BinaryWriteError) {
+      return -1;
+    }
+    return 0;
+  }
+
+  // Open file for writing.
+  std::error_code SysErr;
+  raw_fd_ostream Out(OutputFilename, SysErr);
+  if (SysErr) {
+    WithColor::error() << "Couldn't open " << OutputFilename
+                       << " for writing.\n";
+    return -1;
+  }
+
   if (IsWriteIfs) {
     yaml::Output YamlOut(Out, NULL, /*WrapColumn =*/0);
     YamlOut << const_cast<IFSStub &>(Stub);
     return 0;
   }
 
-  std::string ObjectFileFormat =
-      ForceFormat.empty() ? Stub.ObjectFileFormat : ForceFormat;
-
   if (ObjectFileFormat == "ELF" || ForceFormat == "ELFOBJYAML")
     return writeElfStub(llvm::Triple(Stub.Triple), Stub.Symbols,
                         Stub.ObjectFileFormat, Out);
   if (ObjectFileFormat == "TBD")
     return writeTbdStub(llvm::Triple(Stub.Triple), Stub.Symbols,
                         Stub.ObjectFileFormat, Out);
 
   WithColor::error()
if (Stub.IfsVersion.getMajor() != IFSVersionCurrent.getMajor()) {
                          << Stub.IfsVersion << ", llvm-ifs supported version: "
                          << IFSVersionCurrent << ".\n";
       return -1;
     }
 
   for (auto &Entry : SymbolMap)
     Stub.Symbols.push_back(Entry.second);
 
-  std::error_code SysErr;
-
-  // Open file for writing.
-  raw_fd_ostream Out(OutputFilename, SysErr);
-  if (SysErr) {
-    WithColor::error() << "Couldn't open " << OutputFilename
-                       << " for writing.\n";
-    return -1;
-  }
-
-  return writeIfso(Stub, (Action == "write-ifs"), Out);
+  return writeIfso(Stub, (Action == "write-ifs"));
 }