[llvm] [readtapi] Add Merge functionality (PR #72656)

Cyndy Ishida via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 17 07:06:42 PST 2023


https://github.com/cyndyishida created https://github.com/llvm/llvm-project/pull/72656

Merge allows a user to merge different files (tbds or dylibs) to emit out a single tbd with all input contents. This does require that all inputs represent the same library.

>From 7231953f0b561244d46ae40b8093db0ab508239b Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Tue, 14 Nov 2023 15:00:30 -0800
Subject: [PATCH] [readtapi] Add Merge functionality

Merge allows a user to merge different files (tbds or dylibs) to emit
out a single tbd with all input contents. This does require that all
inputs represent the same library.
---
 llvm/include/llvm/TextAPI/TextAPIWriter.h     | 14 +++
 .../compare-incorrect-format.test             |  2 +-
 .../tools/llvm-readtapi/merge-invalid.test    | 52 ++++++++++
 llvm/test/tools/llvm-readtapi/merge.test      | 99 +++++++++++++++++++
 llvm/tools/llvm-readtapi/DiffEngine.cpp       | 10 +-
 llvm/tools/llvm-readtapi/DiffEngine.h         |  8 +-
 llvm/tools/llvm-readtapi/TapiOpts.td          |  3 +
 llvm/tools/llvm-readtapi/llvm-readtapi.cpp    | 77 ++++++++++-----
 8 files changed, 231 insertions(+), 34 deletions(-)
 create mode 100644 llvm/test/tools/llvm-readtapi/merge-invalid.test
 create mode 100644 llvm/test/tools/llvm-readtapi/merge.test

diff --git a/llvm/include/llvm/TextAPI/TextAPIWriter.h b/llvm/include/llvm/TextAPI/TextAPIWriter.h
index 89fc984854dbae0..6c582b8bc0cca7e 100644
--- a/llvm/include/llvm/TextAPI/TextAPIWriter.h
+++ b/llvm/include/llvm/TextAPI/TextAPIWriter.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_TEXTAPI_TEXTAPIWRITER_H
 #define LLVM_TEXTAPI_TEXTAPIWRITER_H
 
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/TextAPI/InterfaceFile.h"
 
 namespace llvm {
@@ -32,6 +33,19 @@ class TextAPIWriter {
   static Error writeToStream(raw_ostream &OS, const InterfaceFile &File,
                              const FileType FileKind = FileType::Invalid,
                              bool Compact = false);
+
+  /// Get TAPI Text FileType of string input.
+  ///
+  /// \param FT String of input to map to FileType.
+  static FileType parseFileType(const StringRef FT) {
+    return StringSwitch<FileType>(FT)
+        .Case("tbd-v1", FileType::TBD_V1)
+        .Case("tbd-v2", FileType::TBD_V2)
+        .Case("tbd-v3", FileType::TBD_V3)
+        .Case("tbd-v4", FileType::TBD_V4)
+        .Case("tbd-v5", FileType::TBD_V5)
+        .Default(FileType::Invalid);
+  }
 };
 
 } // end namespace MachO.
diff --git a/llvm/test/tools/llvm-readtapi/compare-incorrect-format.test b/llvm/test/tools/llvm-readtapi/compare-incorrect-format.test
index 32a350b28eb1e40..09dc768518ebc79 100644
--- a/llvm/test/tools/llvm-readtapi/compare-incorrect-format.test
+++ b/llvm/test/tools/llvm-readtapi/compare-incorrect-format.test
@@ -2,6 +2,6 @@
 ; RUN: yaml2obj %S/Inputs/macho.yaml -o %t/macho.dylib
 ; RUN: not llvm-readtapi --compare %S/Inputs/v4A.tbd %t/macho.dylib 2>&1 | FileCheck %s
 
-; CHECK: error: {{.*}}macho.dylib' unsupported file format
+; CHECK: error: {{.*}}macho.dylib' unsupported file type 
 ; CHECK-NOT: error:
 ; CHECK-NOT: warning:
diff --git a/llvm/test/tools/llvm-readtapi/merge-invalid.test b/llvm/test/tools/llvm-readtapi/merge-invalid.test
new file mode 100644
index 000000000000000..08dc0b66d581cd3
--- /dev/null
+++ b/llvm/test/tools/llvm-readtapi/merge-invalid.test
@@ -0,0 +1,52 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+; RUN: not llvm-readtapi -merge %t/libfoo.tbd %t/libbar.tbd 2>&1| FileCheck %s --allow-empty --check-prefix DIFF
+; RUN: not llvm-readtapi -merge %t/libfoo.tbd 2>&1 | FileCheck %s --allow-empty --check-prefix INPUT
+
+; DIFF: install names do not match
+; INPUT: merge requires at least two input files
+
+;--- libfoo.tbd
+{
+  "main_library": {
+    "allowable_clients": [
+      {
+        "clients": [
+          "ClientAll"
+        ]
+      }
+    ],
+    "install_names": [
+      {
+        "name": "/usr/lib/libfoo.dylib"
+      }
+    ],
+    "target_info": [
+      {
+        "min_deployment": "13.1",
+        "target": "x86_64-macos"
+      }
+    ]
+  },
+  "tapi_tbd_version": 5
+}
+
+;--- libbar.tbd
+--- !tapi-tbd
+tbd-version:     4
+targets:         [ arm64-macos ]
+install-name:    '/usr/lib/libbar.dylib'
+allowable-clients:
+  - targets:         [ arm64-macos ]
+    clients:         [ ClientAll ]
+reexported-libraries:
+  - targets:         [ arm64-macos ]
+    libraries:       [ '/usr/lib/liball.dylib' ]
+exports:
+  - targets:         [ arm64-macos ]
+    symbols:         [ _sym1 ]
+    objc-classes:    [ _A ]
+    objc-ivars:      [ _A._ivar1 ]
+    weak-symbols:    [ _weak1 ]
+    thread-local-symbols: [ _tlv1 ]
+...
diff --git a/llvm/test/tools/llvm-readtapi/merge.test b/llvm/test/tools/llvm-readtapi/merge.test
new file mode 100644
index 000000000000000..59953f4bf5f4e8e
--- /dev/null
+++ b/llvm/test/tools/llvm-readtapi/merge.test
@@ -0,0 +1,99 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+; RUN: llvm-readtapi -merge %t/i386.tbd %t/x86_64.tbd %t/arm64.tbd --filetype tbd-v5 -o %t/out.tbd -compact 2>&1 | FileCheck %s --allow-empty 
+; RUN: llvm-readtapi -merge %t/i386.tbd %t/x86_64.tbd %t/arm64.tbd --filetype=tbd-v5 --o %t/out.tbd -compact 2>&1 | FileCheck %s --allow-empty 
+; RUN: llvm-readtapi -compare %t/out.tbd %t/expected.tbd 2>&1 | FileCheck %s --allow-empty 
+
+; CHECK-NOT: error 
+; CHECK-NOT: warning
+
+;--- expected.tbd
+{"main_library":{"allowable_clients":[{"clients":["ClientAll"]}],"exported_symbols":[{"data":{"global":["_sym1"],"objc_class":["_A"],"thread_local":["_tlv1"],"weak":["_weak1"]}},{"data":{"objc_ivar":["_A._ivar1"]},"targets":["x86_64-macos","arm64-macos"]}],"install_names":[{"name":"/usr/lib/libfat.dylib"}],"reexported_libraries":[{"names":["/usr/lib/liball.dylib"]}],"target_info":[{"min_deployment":"0","target":"i386-macos"},{"min_deployment":"13.1","target":"x86_64-macos"},{"min_deployment":"0","target":"arm64-macos"}]},"tapi_tbd_version":5}
+
+;--- i386.tbd
+--- !tapi-tbd-v3
+archs:           [ i386 ]
+platform:        macosx
+install-name:    /usr/lib/libfat.dylib
+exports:         
+  - archs:           [ i386 ]
+    allowable-clients: [ ClientAll ]
+    re-exports:      [ /usr/lib/liball.dylib ]
+    symbols:         [ _sym1 ]
+    objc-classes:    [ _A ]
+    weak-def-symbols: [ _weak1 ]
+    thread-local-symbols: [ _tlv1 ]
+...
+
+;--- x86_64.tbd
+{
+  "main_library": {
+    "allowable_clients": [
+      {
+        "clients": [
+          "ClientAll"
+        ]
+      }
+    ],
+    "exported_symbols": [
+      {
+        "data": {
+          "global": [
+            "_sym1"
+          ],
+          "objc_class": [
+            "_A"
+          ],
+          "objc_ivar": [
+            "_A._ivar1"
+          ],
+          "thread_local": [
+            "_tlv1"
+          ],
+          "weak": [
+            "_weak1"
+          ]
+        }
+      }
+    ],
+    "install_names": [
+      {
+        "name": "/usr/lib/libfat.dylib"
+      }
+    ],
+    "reexported_libraries": [
+      {
+        "names": [
+          "/usr/lib/liball.dylib"
+        ]
+      }
+    ],
+    "target_info": [
+      {
+        "min_deployment": "13.1",
+        "target": "x86_64-macos"
+      }
+    ]
+  },
+  "tapi_tbd_version": 5
+}
+
+;--- arm64.tbd
+--- !tapi-tbd
+tbd-version:     4
+targets:         [ arm64-macos ]
+install-name:    '/usr/lib/libfat.dylib'
+allowable-clients:
+  - targets:         [ arm64-macos ]
+    clients:         [ ClientAll ]
+reexported-libraries:
+  - targets:         [ arm64-macos ]
+    libraries:       [ '/usr/lib/liball.dylib' ]
+exports:
+  - targets:         [ arm64-macos ]
+    symbols:         [ _sym1 ]
+    objc-classes:    [ _A ]
+    objc-ivars:      [ _A._ivar1 ]
+    weak-symbols:    [ _weak1 ]
+    thread-local-symbols: [ _tlv1 ]
+...
diff --git a/llvm/tools/llvm-readtapi/DiffEngine.cpp b/llvm/tools/llvm-readtapi/DiffEngine.cpp
index 5f4b25ca6c0b2de..40722d2da180fd1 100644
--- a/llvm/tools/llvm-readtapi/DiffEngine.cpp
+++ b/llvm/tools/llvm-readtapi/DiffEngine.cpp
@@ -560,13 +560,11 @@ void DiffEngine::printDifferences(raw_ostream &OS,
 }
 
 bool DiffEngine::compareFiles(raw_ostream &OS) {
-  const auto *IFLHS = &(FileLHS->getInterfaceFile());
-  const auto *IFRHS = &(FileRHS->getInterfaceFile());
-  if (*IFLHS == *IFRHS)
+  if (*FileLHS == *FileRHS)
     return false;
-  OS << "< " << std::string(IFLHS->getPath().data()) << "\n> "
-     << std::string(IFRHS->getPath().data()) << "\n\n";
-  std::vector<DiffOutput> Diffs = findDifferences(IFLHS, IFRHS);
+  OS << "< " << std::string(FileLHS->getPath().data()) << "\n> "
+     << std::string(FileRHS->getPath().data()) << "\n\n";
+  std::vector<DiffOutput> Diffs = findDifferences(FileLHS, FileRHS);
   printDifferences(OS, Diffs, 0);
   return true;
 }
diff --git a/llvm/tools/llvm-readtapi/DiffEngine.h b/llvm/tools/llvm-readtapi/DiffEngine.h
index 27b72573d011e62..5f7c29c920814f7 100644
--- a/llvm/tools/llvm-readtapi/DiffEngine.h
+++ b/llvm/tools/llvm-readtapi/DiffEngine.h
@@ -141,14 +141,14 @@ class InlineDoc : public AttributeDiff {
 /// output of the differences found in the files.
 class DiffEngine {
 public:
-  DiffEngine(object::TapiUniversal *InputFileNameLHS,
-             object::TapiUniversal *InputFileNameRHS)
+  DiffEngine(MachO::InterfaceFile *InputFileNameLHS,
+             MachO::InterfaceFile *InputFileNameRHS)
       : FileLHS(InputFileNameLHS), FileRHS(InputFileNameRHS){};
   bool compareFiles(raw_ostream &);
 
 private:
-  object::TapiUniversal *FileLHS;
-  object::TapiUniversal *FileRHS;
+  MachO::InterfaceFile *FileLHS;
+  MachO::InterfaceFile *FileRHS;
 
   /// Function that prints the differences found in the files.
   void printDifferences(raw_ostream &, const std::vector<DiffOutput> &, int);
diff --git a/llvm/tools/llvm-readtapi/TapiOpts.td b/llvm/tools/llvm-readtapi/TapiOpts.td
index 932a21f7d071199..82c8ede2809997e 100644
--- a/llvm/tools/llvm-readtapi/TapiOpts.td
+++ b/llvm/tools/llvm-readtapi/TapiOpts.td
@@ -12,6 +12,9 @@ multiclass JS<string name, string help, string var = ""> {
 //
 def help : FF<"help", "display this help">;
 defm output: JS<"o", "write output to <file>","<file>">;
+def compact: FF<"compact", "write compact tapi output file">;
+defm filetype: JS<"filetype", "specify the output file type (tbd-v3, tbd-v4 or tbd-v5)","<value>">;
+def merge: FF<"merge", "merge the input files that represent the same library">;
 
 //
 // Compare options
diff --git a/llvm/tools/llvm-readtapi/llvm-readtapi.cpp b/llvm/tools/llvm-readtapi/llvm-readtapi.cpp
index 1307916120e98e6..71cf5bf70df63c4 100644
--- a/llvm/tools/llvm-readtapi/llvm-readtapi.cpp
+++ b/llvm/tools/llvm-readtapi/llvm-readtapi.cpp
@@ -10,7 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 #include "DiffEngine.h"
-#include "llvm/Object/TapiUniversal.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
@@ -21,6 +20,8 @@
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TextAPI/TextAPIError.h"
+#include "llvm/TextAPI/TextAPIReader.h"
+#include "llvm/TextAPI/TextAPIWriter.h"
 #include <cstdlib>
 
 using namespace llvm;
@@ -59,15 +60,22 @@ class TAPIOptTable : public opt::GenericOptTable {
 struct Context {
   std::vector<std::string> Inputs;
   std::unique_ptr<llvm::raw_fd_stream> OutStream;
+  FileType WriteFT = FileType::TBD_V5;
+  bool Compact = false;
 };
 
-Expected<std::unique_ptr<Binary>>
-convertFileToBinary(const StringRef Filename) {
+std::unique_ptr<InterfaceFile> getInterfaceFile(const StringRef Filename,
+                                                ExitOnError &ExitOnErr) {
+  ExitOnErr.setBanner("error: '" + Filename.str() + "' ");
   ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
-      MemoryBuffer::getFileOrSTDIN(Filename);
+      MemoryBuffer::getFile(Filename);
   if (BufferOrErr.getError())
-    return errorCodeToError(BufferOrErr.getError());
-  return createBinary(BufferOrErr.get()->getMemBufferRef());
+    ExitOnErr(errorCodeToError(BufferOrErr.getError()));
+  Expected<std::unique_ptr<InterfaceFile>> IF =
+      TextAPIReader::get((*BufferOrErr)->getMemBufferRef());
+  if (!IF)
+    ExitOnErr(IF.takeError());
+  return std::move(*IF);
 }
 
 // Use unique exit code to differentiate failures not directly caused from
@@ -81,28 +89,37 @@ bool handleCompareAction(const Context &Ctx) {
     ExitOnErr(make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat,
                                        "compare only supports 2 input files"));
   }
-  StringRef InputFileName = Ctx.Inputs.front();
-  ExitOnErr.setBanner("error: '" + InputFileName.str() + "' ");
-  auto BinLHS = ExitOnErr(convertFileToBinary(InputFileName));
-
-  TapiUniversal *FileLHS = dyn_cast<TapiUniversal>(BinLHS.get());
-  if (!FileLHS) {
-    ExitOnErr(createStringError(std::errc::executable_format_error,
-                                "unsupported file format"));
-  }
 
-  StringRef CompareInputFileName = Ctx.Inputs.at(1);
-  ExitOnErr.setBanner("error: '" + CompareInputFileName.str() + "' ");
-  auto BinRHS = ExitOnErr(convertFileToBinary(CompareInputFileName));
+  auto LeftIF = getInterfaceFile(Ctx.Inputs.front(), ExitOnErr);
+  auto RightIF = getInterfaceFile(Ctx.Inputs.at(1), ExitOnErr);
+
+  raw_ostream &OS = Ctx.OutStream ? *Ctx.OutStream : outs();
+  return DiffEngine(LeftIF.get(), RightIF.get()).compareFiles(OS);
+}
 
-  TapiUniversal *FileRHS = dyn_cast<TapiUniversal>(BinRHS.get());
-  if (!FileRHS) {
-    ExitOnErr(createStringError(std::errc::executable_format_error,
-                                "unsupported file format"));
+bool handleMergeAction(const Context &Ctx) {
+  ExitOnError ExitOnErr("error: ");
+  if (Ctx.Inputs.size() < 2) {
+    ExitOnErr(
+        make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat,
+                                 "merge requires at least two input files"));
+  }
+  std::unique_ptr<InterfaceFile> Out;
+  for (StringRef FileName : Ctx.Inputs) {
+    auto IF = getInterfaceFile(FileName, ExitOnErr);
+    if (!Out) {
+      Out = std::move(IF);
+      continue;
+    }
+    auto ResultIF = Out->merge(IF.get());
+    if (!ResultIF)
+      ExitOnErr(ResultIF.takeError());
+    Out = std::move(ResultIF.get());
   }
 
   raw_ostream &OS = Ctx.OutStream ? *Ctx.OutStream : outs();
-  return DiffEngine(FileLHS, FileRHS).compareFiles(OS);
+  ExitOnErr(TextAPIWriter::writeToStream(OS, *Out, Ctx.WriteFT, Ctx.Compact));
+  return EXIT_SUCCESS;
 }
 
 } // anonymous namespace
@@ -138,8 +155,22 @@ int main(int Argc, char **Argv) {
     }
   }
 
+  if (Args.hasArg(OPT_compact))
+    Ctx.Compact = true;
+
+  if (opt::Arg *A = Args.getLastArg(OPT_filetype_EQ)) {
+    Ctx.WriteFT = TextAPIWriter::parseFileType(A->getValue());
+    if (Ctx.WriteFT < FileType::TBD_V3 || Ctx.WriteFT == FileType::Invalid) {
+      llvm::errs() << "error: unsupported filetype '" << A->getValue() << "'\n";
+      return EXIT_FAILURE;
+    }
+  }
+
   if (Args.hasArg(OPT_compare))
     return handleCompareAction(Ctx);
 
+  if (Args.hasArg(OPT_merge))
+    return handleMergeAction(Ctx);
+
   return EXIT_SUCCESS;
 }



More information about the llvm-commits mailing list