[clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[plotfi]

Summary

Up until now, clang interface stubs has replaced the standard PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in conjunction with it. So what when you build your code you will get an a.out or lib.so as well as an interface stub file.

TODO: add another test or two.

[plotfi]

Adding test, and some additional features in the driver.

[plotfi]

clang/test/InterfaceStubs/driver-test2.c

7

I would use -shared here, but many of the bots don't support that. Trying to figure out what else to do.

[compnerd]

clang/lib/Driver/Driver.cpp

3493

The reference binding is odd. Why not just pass PhaseList?

3500

Do you use the PhaseList again? Why not erase_if?

3506

structured bindings ... so much.

3508

A comment explaining that we cannot inspect assembly yet and why ifs don't make sense would be nice

3515

auto would be fine too

clang/lib/Driver/ToolChains/InterfaceStubs.cpp

34

IIRC, llvm::sys::path::replace_ext or stem should do this?

45

llvm::sys::fs::replace_ext

[plotfi]

clang/lib/Driver/Driver.cpp

3500

We tried this in getCompilationPhases. It didn't work very well.

3506

I don't understand?

[compnerd]

clang/lib/Driver/Driver.cpp

3506

C++17's structured bindings would be nice:

for (const auto &[Type, Arg] : Inputs)

3536

I think that switching on the Phase will make this more clear.

[compnerd]

clang/lib/Driver/Driver.cpp

3491

Extraneous whitespace

clang/lib/Driver/ToolChains/InterfaceStubs.cpp

37

Is there a way to check if the extension is a shared library extension, or better yet, check for the output type

clang/test/InterfaceStubs/driver-test.c

4

Unnecessary whitespace change

[leonardchan]

Hi! I think this patch might be causing some test failures on our mac bots:

FAIL: Clang :: InterfaceStubs/merge-conflict-test.c (6268 of 16220)
******************** TEST 'Clang :: InterfaceStubs/merge-conflict-test.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   not /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang -fvisibility=default -o libfoo.so -emit-interface-stubs /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/driver-test.c 2>&1 | /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c:2:11: error: CHECK: expected string not found in input
// CHECK: error: Interface Stub: Type Mismatch
          ^
<stdin>:1:1: note: scanning from here
clang-10: error: cannot specify -o when generating multiple output files
^
<stdin>:1:11: note: possible intended match here
clang-10: error: cannot specify -o when generating multiple output files
          ^

--

********************
Testing:  0.. 10.. 20.. 30..
FAIL: Clang :: InterfaceStubs/object-float.c (6276 of 16220)
******************** TEST 'Clang :: InterfaceStubs/object-float.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   not /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang -fvisibility=default -o - -emit-interface-stubs /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object.c 2>&1 | /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c:2:11: error: CHECK: expected string not found in input
// CHECK: error: Interface Stub: Size Mismatch
          ^
<stdin>:1:1: note: scanning from here
clang-10: error: cannot specify -o when generating multiple output files
^
<stdin>:1:11: note: possible intended match here
clang-10: error: cannot specify -o when generating multiple output files
          ^

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 131.42s
********************
Failing Tests (2):
    Clang :: InterfaceStubs/merge-conflict-test.c
    Clang :: InterfaceStubs/object-float.c

Could you look into it? Thanks.

Bot link: https://ci.chromium.org/p/fuchsia/builders/ci/clang-mac-x64/b8896234755488932160

[plotfi]

Looking into it. This change also affected Green Dragon. Thanks for the heads up.

In D70274#1754306, @leonardchan wrote:

Hi! I think this patch might be causing some test failures on our mac bots:

FAIL: Clang :: InterfaceStubs/merge-conflict-test.c (6268 of 16220)
******************** TEST 'Clang :: InterfaceStubs/merge-conflict-test.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   not /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang -fvisibility=default -o libfoo.so -emit-interface-stubs /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/driver-test.c 2>&1 | /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c:2:11: error: CHECK: expected string not found in input
// CHECK: error: Interface Stub: Type Mismatch
          ^
<stdin>:1:1: note: scanning from here
clang-10: error: cannot specify -o when generating multiple output files
^
<stdin>:1:11: note: possible intended match here
clang-10: error: cannot specify -o when generating multiple output files
          ^

--

********************
Testing:  0.. 10.. 20.. 30..
FAIL: Clang :: InterfaceStubs/object-float.c (6276 of 16220)
******************** TEST 'Clang :: InterfaceStubs/object-float.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   not /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang -fvisibility=default -o - -emit-interface-stubs /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object.c 2>&1 | /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c:2:11: error: CHECK: expected string not found in input
// CHECK: error: Interface Stub: Size Mismatch
          ^
<stdin>:1:1: note: scanning from here
clang-10: error: cannot specify -o when generating multiple output files
^
<stdin>:1:11: note: possible intended match here
clang-10: error: cannot specify -o when generating multiple output files
          ^

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 131.42s
********************
Failing Tests (2):
    Clang :: InterfaceStubs/merge-conflict-test.c
    Clang :: InterfaceStubs/object-float.c

Could you look into it? Thanks.

Bot link: https://ci.chromium.org/p/fuchsia/builders/ci/clang-mac-x64/b8896234755488932160

[plotfi]

I have a fix. About to push.

In D70274#1754306, @leonardchan wrote:

Hi! I think this patch might be causing some test failures on our mac bots:

FAIL: Clang :: InterfaceStubs/merge-conflict-test.c (6268 of 16220)
******************** TEST 'Clang :: InterfaceStubs/merge-conflict-test.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   not /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang -fvisibility=default -o libfoo.so -emit-interface-stubs /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/driver-test.c 2>&1 | /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c:2:11: error: CHECK: expected string not found in input
// CHECK: error: Interface Stub: Type Mismatch
          ^
<stdin>:1:1: note: scanning from here
clang-10: error: cannot specify -o when generating multiple output files
^
<stdin>:1:11: note: possible intended match here
clang-10: error: cannot specify -o when generating multiple output files
          ^

--

********************
Testing:  0.. 10.. 20.. 30..
FAIL: Clang :: InterfaceStubs/object-float.c (6276 of 16220)
******************** TEST 'Clang :: InterfaceStubs/object-float.c' FAILED ********************
Script:
--
: 'RUN: at line 1';   not /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang -fvisibility=default -o - -emit-interface-stubs /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object.c 2>&1 | /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c
--
Exit Code: 1

Command Output (stderr):
--
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c:2:11: error: CHECK: expected string not found in input
// CHECK: error: Interface Stub: Size Mismatch
          ^
<stdin>:1:1: note: scanning from here
clang-10: error: cannot specify -o when generating multiple output files
^
<stdin>:1:11: note: possible intended match here
clang-10: error: cannot specify -o when generating multiple output files
          ^

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 131.42s
********************
Failing Tests (2):
    Clang :: InterfaceStubs/merge-conflict-test.c
    Clang :: InterfaceStubs/object-float.c

Could you look into it? Thanks.

Bot link: https://ci.chromium.org/p/fuchsia/builders/ci/clang-mac-x64/b8896234755488932160

clang/lib/Driver/Driver.cpp

              (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
     FinalPhase = phases::Compile;
 
-  // clang interface stubs
-  } else if ((PhaseArg = DAL.getLastArg(options::OPT_emit_interface_stubs))) {
-    FinalPhase = phases::IfsMerge;
-
   // -S only runs up to the backend.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
     FinalPhase = phases::Backend;

     Actions.push_back(
         C.MakeAction<IfsMergeJobAction>(MergerInputs, types::TY_Image));
 
+  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
+    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PhaseList;
+    if (Args.hasArg(options::OPT_c)) {
+      llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> CompilePhaseList;
+      types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
+      llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
+                    [&](phases::ID Phase) { return Phase <= phases::Compile; });
+    } else {
+      types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
+    }
+
+    ActionList MergerInputs;
+
+    for (auto &I : Inputs) {
+      types::ID InputType = I.first;
+      const Arg *InputArg = I.second;
+
+      // Currently clang and the llvm assembler do not support generating symbol
+      // stubs from assembly, so we skip the input on asm files. For ifs files
+      // we rely on the normal pipeline setup in the pipeline setup code above.
+      if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
+          InputType == types::TY_Asm)
+        continue;
+
+      Action *Current = C.MakeAction<InputAction>(*InputArg, InputType);
+
+      for (auto Phase : PhaseList) {
+        switch (Phase) {
+        default:
+          llvm_unreachable(
+              "IFS Pipeline can only consist of Compile followed by IfsMerge.");
+        case phases::Compile: {
+          // Only IfsMerge (llvm-ifs) can handle .o files by looking for ifs
+          // files where the .o file is located. The compile action can not
+          // handle this.
+          if (InputType == types::TY_Object)
+            break;
+
+          Current = C.MakeAction<CompileJobAction>(Current, types::TY_IFS_CPP);
+          break;
+        }
+        case phases::IfsMerge: {
+          assert(Phase == PhaseList.back() &&
+                 "merging must be final compilation step.");
+          MergerInputs.push_back(Current);
+          Current = nullptr;
+          break;
+        }
+        }
+      }
+
+      // If we ended with something, add to the output list.
+      if (Current)
+        Actions.push_back(Current);
+    }
+
+    // Add an interface stubs merge action if necessary.
+    if (!MergerInputs.empty())
+      Actions.push_back(
+          C.MakeAction<IfsMergeJobAction>(MergerInputs, types::TY_Image));
+  }
+
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a custom
   // Compile phase that prints out supported cpu models and quits.
   if (Arg *A = Args.getLastArg(options::OPT_print_supported_cpus)) {

       return C.MakeAction<CompileJobAction>(Input, types::TY_ModuleFile);
     if (Args.hasArg(options::OPT_verify_pch))
       return C.MakeAction<VerifyPCHJobAction>(Input, types::TY_Nothing);
-    if (Args.hasArg(options::OPT_emit_interface_stubs))
-      return C.MakeAction<CompileJobAction>(Input, types::TY_IFS_CPP);
     return C.MakeAction<CompileJobAction>(Input, types::TY_LLVM_BC);
   }
   case phases::Backend: {

   Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o);
 
   // It is an error to provide a -o option if we are making multiple output
-  // files.
+  // files. There is one exception, IfsMergeJob: when generating interface stubs
+  // enabled we want to be able to generate the stub file at the same time that
+  // we generate the real library/a.out. So when a .o, .so, etc are the output,
+  // with clang interface stubs there will also be a .ifs and .ifso at the same
+  // location.
   if (FinalOutput) {
     unsigned NumOutputs = 0;
     for (const Action *A : C.getActions())
-      if (A->getType() != types::TY_Nothing)
+      if (A->getType() != types::TY_Nothing &&
+          A->getKind() != Action::IfsMergeJobClass)
         ++NumOutputs;
 
     if (NumOutputs > 1) {

clang/lib/Driver/ToolChains/InterfaceStubs.cpp

 #include "InterfaceStubs.h"
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace driver {

   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-                        ? "write-ifs"
-                        : "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
-  for (const auto &Input : Inputs)
-    CmdArgs.push_back(Input.getFilename());
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
+  // invoked then we would like to get libhello.so and libhello.ifso. If the
+  // stdout stream is given as the output file (ie `-o -`), that is the one
+  // exception where we will just append to the same filestream as the normal
+  // output.
+  SmallString<128> OutputFilename(Output.getFilename());
+  if (OutputFilename != "-") {
+    if (Args.hasArg(options::OPT_shared))
+      llvm::sys::path::replace_extension(OutputFilename,
+                                         (WriteBin ? "ifso" : "ifs"));
+    else
+      OutputFilename += ".ifso";
+  }
+
+  CmdArgs.push_back(Args.MakeArgString(OutputFilename.c_str()));
+
+  // Here we append the input files. If the input files are object files, then
+  // we look for .ifs files present in the same location as the object files.
+  for (const auto &Input : Inputs) {
+    SmallString<128> InputFilename(Input.getFilename());
+    if (Input.getType() == types::TY_Object)
+      llvm::sys::path::replace_extension(InputFilename, ".ifs");
+    CmdArgs.push_back(Args.MakeArgString(InputFilename.c_str()));
+  }
+
   C.addCommand(std::make_unique<Command>(JA, *this, Args.MakeArgString(Merger),
                                          CmdArgs, Inputs));
 }

clang/lib/Driver/Types.cpp

     llvm::copy_if(PhaseList, std::back_inserter(P),
                   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> IfsModePhaseList;
-    llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> &PL = PhaseList;
-    phases::ID LastPhase = phases::IfsMerge;
-    if (Id != types::TY_IFS) {
-      if (DAL.hasArg(options::OPT_c))
-        LastPhase = phases::Compile;
-      PL = IfsModePhaseList;
-      types::getCompilationPhases(types::TY_IFS_CPP, PL);
-    }
-    llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-      return Phase <= LastPhase;
-    });
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
            DAL.getLastArg(options::OPT_print_supported_cpus) ||

clang/test/InterfaceStubs/driver-test.c

 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1 2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }

clang/test/InterfaceStubs/driver-test2.c

+// REQUIRES: x86-registered-target
+// REQUIRES: shell
+
+// RUN: mkdir -p %t; cd %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs \
+// RUN:   %s %S/object.c %S/weak.cpp
+// RUN: %clang -emit-interface-stubs -emit-merged-ifs \
+// RUN:   %t/driver-test2.o %t/object.o %t/weak.o -S -o - 2>&1 | FileCheck %s
+
+// CHECK-DAG: data
+// CHECK-DAG: bar
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
+
+int bar(int a) { return a; }
+int main() { return 0; }

clang/test/InterfaceStubs/windows.cpp

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -S | FileCheck -check-prefix=CHECK-IFS %s
+// note: -S is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ