[InstallAPI] add JSON option to pass X<label> arguments (#91770)

cyndyishida · web-flow · commit c9dc52d424b7 · 2024-05-20T21:22:24.000-07:00
diff --git a/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td b/clang/include/clang/Basic/DiagnosticInstallAPIKinds.td
@@ -24,7 +24,7 @@ def err_no_matching_target : Error<"no matching target found for target variant
 def err_unsupported_vendor : Error<"vendor '%0' is not supported: '%1'">;
 def err_unsupported_environment : Error<"environment '%0' is not supported: '%1'">;
 def err_unsupported_os : Error<"os '%0' is not supported: '%1'">;
-def err_cannot_read_input_list : Error<"could not read %select{alias list|filelist}0 '%1': %2">;
+def err_cannot_read_input_list : Error<"could not read %0 input list '%1': %2">;
 def err_invalid_label: Error<"label '%0' is reserved: use a different label name for -X<label>">;
 } // end of command line category.
 
diff --git a/clang/test/InstallAPI/alias_list.test b/clang/test/InstallAPI/alias_list.test
@@ -23,7 +23,7 @@
 ; RUN: -o %t/AliasList.tbd 2>&1 | FileCheck -allow-empty %s  \
 ; RUN: --check-prefix=INVALID
 
-; INVALID: error: could not read alias list {{.*}} missing alias for: _hidden
+; INVALID: error: could not read symbol alias input list {{.*}}invalid.txt': invalid input format: missing alias for: _hidden
 
 ;--- Frameworks/AliasList.framework/Headers/AliasList.h
 // simple alias from one symbol to another.
diff --git a/clang/test/InstallAPI/exclusive-passes-2.test b/clang/test/InstallAPI/exclusive-passes-2.test
@@ -11,6 +11,15 @@
 ; RUN: -DFoo -XApple -DDarwin=1 -XElf -DNONDarwin=1 2>&1 | FileCheck -allow-empty %s 
 ; RUN: llvm-readtapi --compare %t/output.tbd %t/expected.tbd 2>&1 | FileCheck -allow-empty %s
 
+; RUN: clang-installapi -target arm64-apple-macos12 \
+; RUN: -install_name @rpath/libfoo.dylib \
+; RUN: -current_version 1 -compatibility_version 1 \
+; RUN: -I%S/Inputs/LibFoo/usr/include -dynamiclib \
+; RUN: -extra-public-header %S/Inputs/LibFoo/usr/include/foo.h \
+; RUN: -o %t/output2.tbd \
+; RUN: -DFoo -optionlist %t/options.json 2>&1 | FileCheck -allow-empty %s 
+; RUN: llvm-readtapi --compare %t/output.tbd %t/expected.tbd 2>&1 | FileCheck -allow-empty %s
+
 ; CHECK-NOT: error
 ; CHECK-NOT: warning
 
diff --git a/clang/test/InstallAPI/exclusive-passes-3.test b/clang/test/InstallAPI/exclusive-passes-3.test
@@ -0,0 +1,86 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t
+
+// "Apple" label has split options between the optionlist & command line. 
+; RUN: clang-installapi -target arm64-apple-macos12 \
+; RUN: -install_name @rpath/libfoo.dylib -current_version 1 \
+; RUN: -compatibility_version 1 \
+; RUN: -extra-public-header %t/usr/include/opts.h \
+; RUN: -optionlist %t/options.json -XApple -DCLI_OPT=1 \
+; RUN: -I%S/Inputs/LibFoo/usr/include \
+; RUN: -I%t/usr/include -dynamiclib -o %t/output.tbd 2>&1 | FileCheck %s -allow-empty 
+; RUN: llvm-readtapi --compare %t/output.tbd %t/expected.tbd 2>&1 | FileCheck -allow-empty %s
+
+// Validate duplicated options give same result.
+; RUN: clang-installapi -target arm64-apple-macos12 \
+; RUN: -install_name @rpath/libfoo.dylib -current_version 1 \
+; RUN: -compatibility_version 1 \
+; RUN: -extra-public-header %t/usr/include/opts.h \
+; RUN: -optionlist %t/options.json -XApple -DCLI_OPT=1 \
+; RUN: -I%S/Inputs/LibFoo/usr/include \
+; RUN: -XApple -DDarwin -XElf -DNONDarwin \
+; RUN: -I%t/usr/include -dynamiclib -o %t/output2.tbd 2>&1 | FileCheck %s -allow-empty 
+; RUN: llvm-readtapi --compare %t/output2.tbd %t/expected.tbd 2>&1 | FileCheck -allow-empty %s
+
+; CHECK-NOT: error
+; CHECK-NOT: warning
+
+;--- usr/include/opts.h
+#ifndef OPTS_H
+#define OPTS_H
+#include <macro_defs.h>
+
+#if defined(CLI_OPT) && CLI_OPT 
+  #define SUFFIX "$final"
+#else 
+  #define SUFFIX 
+#endif 
+
+
+#define __STRING(x)     #x
+#define PLATFORM_ALIAS(sym)	__asm("_" __STRING(sym) DARWIN LINUX SUFFIX)
+extern int foo() PLATFORM_ALIAS(foo);
+
+#endif 
+
+;--- expected.tbd
+{
+  "main_library": {
+    "exported_symbols": [
+      {
+        "text": {
+          "global": [
+            "_foo$darwin$final",
+            "_foo$linux",
+            "_foo"
+          ]
+        }
+      }
+    ],
+    "flags": [
+      {
+        "attributes": [
+          "not_app_extension_safe"
+        ]
+      }
+    ],
+    "install_names": [
+      {
+        "name": "@rpath/libfoo.dylib"
+      }
+    ],
+    "target_info": [
+      {
+        "min_deployment": "12",
+        "target": "arm64-macos"
+      }
+    ]
+  },
+  "tapi_tbd_version": 5
+}
+
+//--- options.json
+{
+  "Apple" : ["-DDarwin=1"],
+  "Elf" : ["-DNONDarwin=1"]
+}
diff --git a/clang/test/InstallAPI/exclusive-passes.test b/clang/test/InstallAPI/exclusive-passes.test
@@ -10,13 +10,28 @@
 ; RUN: -o %t/output.tbd -v 2>&1 | FileCheck %s --check-prefix=INSTALLAPI
 ; RUN: llvm-readtapi --compare %t/output.tbd %t/expected.tbd 2>&1 | FileCheck -allow-empty %s
 
+// Try with -optionlist.
+; RUN: clang-installapi \
+; RUN: -target arm64-apple-macos12 -install_name @rpath/libfoo.dylib \
+; RUN: -current_version 1 -compatibility_version 1 \
+; RUN: -I%S/Inputs/LibFoo/usr/include -dynamiclib \
+; RUN: -extra-public-header %S/Inputs/LibFoo/usr/include/public.h \
+; RUN: -optionlist %t/options.json -o %t/output2.tbd 2>&1 | FileCheck %s -allow-empty 
+; RUN: llvm-readtapi --compare %t/output2.tbd %t/expected.tbd 2>&1 | FileCheck -allow-empty %s
+
 ; CHECK-NOT: error
 ; CHECK-NOT: warning
 
 ; INSTALLAPI: Public Headers:
 ; INSTALLAPI: Apple Public Headers:
 ; INSTALLAPI: Elf Public Headers:
 
+;--- options.json
+{
+  "Apple" : ["-DDarwin=1"],
+  "Elf" : ["-DNONDarwin=1"]
+}
+
 ;--- expected.tbd
 {
   "main_library": {
diff --git a/clang/test/InstallAPI/invalid-exclusive-passes.test b/clang/test/InstallAPI/invalid-exclusive-passes.test
@@ -30,6 +30,39 @@
 ; RUN: -o %t/output.tbd 2>&1 | FileCheck %s --check-prefix=INVALID_PROJECT_OPT
 ; INVALID_PROJECT_OPT: error: invalid argument '-Xproject' not allowed with '-fprofile-instr-generate'
 
+// Validate arguments not allowed with -X passed via json
+; RUN: not clang-installapi -target arm64-apple-macos12 \
+; RUN: -install_name @rpath/libfoo.dylib -current_version 1 -compatibility_version 1 \
+; RUN: -optionlist %t/options.json -I/fake/path \
+; RUN: -I%t -dynamiclib -o %t/output.tbd 2>&1 | FileCheck %s --check-prefix=INVALID_JSON_OPT
+; INVALID_JSON_OPT: error: invalid argument '-XApple' not allowed with '-I/fake/path'
+
+// Validate invalid json path
+; RUN: not clang-installapi -target arm64-apple-macos12 \
+; RUN: -install_name @rpath/libfoo.dylib -current_version 1 \
+; RUN: -compatibility_version 1 -optionlist %t/invalid_loc.json \
+; RUN: -I/fake/path -I%t -dynamiclib \
+; RUN: -o %t/output.tbd %t 2>&1 | FileCheck %s --check-prefix=INVALID_JSON_LOC -DMSG=%errc_ENOENT
+; INVALID_JSON_LOC: error: cannot open file {{.*}}invalid_loc.json': [[MSG]] 
+
+// Validate invalid json format 
+; RUN: not clang-installapi -target arm64-apple-macos12 \
+; RUN: -install_name @rpath/libfoo.dylib -current_version 1 \
+; RUN: -compatibility_version 1 -optionlist %t/invalid_format.json \
+; RUN: -I/fake/path -isysroot %sysroot -I%t -dynamiclib \
+; RUN: -o %t/output.tbd %t 2>&1 | FileCheck %s --check-prefix=INVALID_JSON_FORMAT
+; INVALID_JSON_FORMAT: error: could not read option input list {{.*}}invalid_format.json': invalid input format
+
+;--- options.json
+{
+  "Apple" : ["-I/fake/path"]
+}
+
+;--- invalid_format.json
+{
+  "Apple" : {"opt"  : "-I/fake/path"}
+}
+
 ;--- inputs.json
 {
   "headers": [ ],
diff --git a/clang/tools/clang-installapi/InstallAPIOpts.td b/clang/tools/clang-installapi/InstallAPIOpts.td
@@ -99,6 +99,9 @@ def X__ : Joined<["-"], "X">,
   HelpText<"Pass <arg> to run unique clang invocation identified as <label>">, 
   MetaVarName<"<label> <arg>">;
 
+def option_list : Separate<["-"], "optionlist">, MetaVarName<"<path>">, 
+  HelpText<"Specifies the <path> to a file that contains X<label> arguments to parse.">;
+
 //
 /// Overidden clang options for different behavior.
 //
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
@@ -13,6 +13,7 @@
 #include "clang/InstallAPI/HeaderFile.h"
 #include "clang/InstallAPI/InstallAPIDiagnostic.h"
 #include "llvm/BinaryFormat/Magic.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
 #include "llvm/TargetParser/Host.h"
 #include "llvm/TextAPI/DylibReader.h"
@@ -82,6 +83,47 @@ static llvm::opt::OptTable *createDriverOptTable() {
   return new DriverOptTable();
 }
 
+/// Parse JSON input into argument list.
+///
+/* Expected input format.
+ *  { "label" : ["-ClangArg1", "-ClangArg2"] }
+ */
+///
+/// Input is interpreted as "-Xlabel ClangArg1 -XLabel ClangArg2".
+static Expected<llvm::opt::InputArgList>
+getArgListFromJSON(const StringRef Input, llvm::opt::OptTable *Table,
+                   std::vector<std::string> &Storage) {
+  using namespace json;
+  Expected<Value> ValOrErr = json::parse(Input);
+  if (!ValOrErr)
+    return ValOrErr.takeError();
+
+  const Object *Root = ValOrErr->getAsObject();
+  if (!Root)
+    return llvm::opt::InputArgList();
+
+  for (const auto &KV : *Root) {
+    const Array *ArgList = KV.getSecond().getAsArray();
+    std::string Label = "-X" + KV.getFirst().str();
+    if (!ArgList)
+      return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat);
+    for (auto Arg : *ArgList) {
+      std::optional<StringRef> ArgStr = Arg.getAsString();
+      if (!ArgStr)
+        return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat);
+      Storage.emplace_back(Label);
+      Storage.emplace_back(*ArgStr);
+    }
+  }
+
+  std::vector<const char *> CArgs(Storage.size());
+  llvm::for_each(Storage,
+                 [&CArgs](StringRef Str) { CArgs.emplace_back(Str.data()); });
+
+  unsigned MissingArgIndex, MissingArgCount;
+  return Table->ParseArgs(CArgs, MissingArgIndex, MissingArgCount);
+}
+
 bool Options::processDriverOptions(InputArgList &Args) {
   // Handle inputs.
   llvm::append_range(DriverOpts.FileLists,
@@ -348,6 +390,31 @@ bool Options::processXarchOption(InputArgList &Args, arg_iterator Curr) {
   return true;
 }
 
+bool Options::processOptionList(InputArgList &Args,
+                                llvm::opt::OptTable *Table) {
+  Arg *A = Args.getLastArg(OPT_option_list);
+  if (!A)
+    return true;
+
+  const StringRef Path = A->getValue(0);
+  auto InputOrErr = FM->getBufferForFile(Path);
+  if (auto Err = InputOrErr.getError()) {
+    Diags->Report(diag::err_cannot_open_file) << Path << Err.message();
+    return false;
+  }
+  // Backing storage referenced for argument processing.
+  std::vector<std::string> Storage;
+  auto ArgsOrErr =
+      getArgListFromJSON((*InputOrErr)->getBuffer(), Table, Storage);
+
+  if (auto Err = ArgsOrErr.takeError()) {
+    Diags->Report(diag::err_cannot_read_input_list)
+        << "option" << Path << toString(std::move(Err));
+    return false;
+  }
+  return processInstallAPIXOptions(*ArgsOrErr);
+}
+
 bool Options::processLinkerOptions(InputArgList &Args) {
   // Handle required arguments.
   if (const Arg *A = Args.getLastArg(drv::OPT_install__name))
@@ -510,6 +577,9 @@ Options::processAndFilterOutInstallAPIOptions(ArrayRef<const char *> Args) {
   if (!processInstallAPIXOptions(ParsedArgs))
     return {};
 
+  if (!processOptionList(ParsedArgs, Table.get()))
+    return {};
+
   DriverOpts.Demangle = ParsedArgs.hasArg(OPT_demangle);
 
   if (auto *A = ParsedArgs.getLastArg(OPT_filetype)) {
@@ -818,7 +888,7 @@ InstallAPIContext Options::createContext() {
     Expected<AliasMap> Result = parseAliasList(Buffer.get());
     if (!Result) {
       Diags->Report(diag::err_cannot_read_input_list)
-          << /*IsFileList=*/false << ListPath << toString(Result.takeError());
+          << "symbol alias" << ListPath << toString(Result.takeError());
       return Ctx;
     }
     Aliases.insert(Result.get().begin(), Result.get().end());
@@ -839,7 +909,7 @@ InstallAPIContext Options::createContext() {
     if (auto Err = FileListReader::loadHeaders(std::move(Buffer.get()),
                                                Ctx.InputHeaders, FM)) {
       Diags->Report(diag::err_cannot_read_input_list)
-          << /*IsFileList=*/true << ListPath << std::move(Err);
+          << "header file" << ListPath << std::move(Err);
       return Ctx;
     }
   }
diff --git a/clang/tools/clang-installapi/Options.h b/clang/tools/clang-installapi/Options.h
@@ -161,6 +161,8 @@ class Options {
   bool processXarchOption(llvm::opt::InputArgList &Args, arg_iterator Curr);
   bool processXplatformOption(llvm::opt::InputArgList &Args, arg_iterator Curr);
   bool processXprojectOption(llvm::opt::InputArgList &Args, arg_iterator Curr);
+  bool processOptionList(llvm::opt::InputArgList &Args,
+                         llvm::opt::OptTable *Table);
 
 public:
   /// The various options grouped together.
