[Darwin] [Driver] Clang should invoke dsymutil for multiarch builds

[dsanders]

Summary

Much like with debug info with LTO, this case also uses temporary
object files and therefore needs to externalize it before the
temporary files are deleted

Depends on D84565

[aprantl]

clang/test/Driver/darwin-dsymutil.c

74

Is a.out the lipoed fat binary? If yes this LGTM

[beanz]

clang/test/Driver/darwin-dsymutil.c

74

Yes.

clang/lib/Driver/Driver.cpp

if (Archs.size() > 1 && !types::canLipoType(Act->getType()))
           << types::getTypeName(Act->getType());
 
     ActionList Inputs;
     for (unsigned i = 0, e = Archs.size(); i != e; ++i)
       Inputs.push_back(C.MakeAction<BindArchAction>(Act, Archs[i]));
 
     // Lipo if necessary, we do it this way because we need to set the arch flag
     // so that -Xarch_ gets overwritten.
-    if (Inputs.size() == 1 || Act->getType() == types::TY_Nothing)
+    bool LipoNeeded = Inputs.size() != 1 && Act->getType() != types::TY_Nothing;
+    if (!LipoNeeded)
       Actions.append(Inputs.begin(), Inputs.end());
     else
       Actions.push_back(C.MakeAction<LipoJobAction>(Inputs, Act->getType()));
 
     // Handle debug info queries.
     Arg *A = Args.getLastArg(options::OPT_g_Group);
     bool enablesDebugInfo = A && !A->getOption().matches(options::OPT_g0) &&
                             !A->getOption().matches(options::OPT_gstabs);
     bool isGeneratingTemporaryObject =
-        ContainsCompileOrAssembleAction(Actions.back()) || isUsingLTO();
+        ContainsCompileOrAssembleAction(Actions.back()) || isUsingLTO() ||
+        LipoNeeded;
     if ((enablesDebugInfo || willEmitRemarks(Args)) &&
         isGeneratingTemporaryObject) {
 
       // Add a 'dsymutil' step if necessary, when debug info is enabled and we
       // are linking a temporary object. This occurs when we have a compiler
-      // or assmbler input or if LTO is enabled. We need to run 'dsymutil'
-      // ourselves in such cases because the debug info will refer to the
-      // temporary object file which will be removed at the end of the
-      // compilation process.
+      // or assembler input, if LTO is enabled, or if we're building multiple
+      // architectures. We need to run 'dsymutil' ourselves in such cases
+      // because the debug info will refer to the temporary object file which
+      // will be removed at the end of the compilation process.
       if (Act->getType() == types::TY_Image) {
         ActionList Inputs;
         Inputs.push_back(Actions.back());
         Actions.pop_back();
         Actions.push_back(
             C.MakeAction<DsymutilJobAction>(Inputs, types::TY_dSYM));
       }
 

clang/lib/Driver/ToolChains/Darwin.h

public:
 
   void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,
                     const char *LinkingOutput) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY Linker : public MachOTool {
-  bool NeedsTempPath(const InputInfoList &Inputs,
+  bool NeedsTempPath(const InputInfo &Outputs, const InputInfoList &Inputs,
                      const llvm::opt::ArgList &Args) const;
   void AddLinkArgs(Compilation &C, const llvm::opt::ArgList &Args,
-                   llvm::opt::ArgStringList &CmdArgs,
+                   llvm::opt::ArgStringList &CmdArgs, const InputInfo &Outputs,
                    const InputInfoList &Inputs, unsigned Version[5]) const;
 
 public:
   Linker(const ToolChain &TC) : MachOTool("darwin::Linker", "linker", TC) {}
 
   bool hasIntegratedCPP() const override { return false; }
   bool isLinkJob() const override { return true; }
 

clang/lib/Driver/ToolChains/Darwin.cpp

void darwin::MachOTool::AddMachOArch(const ArgList &Args,
   CmdArgs.push_back("-arch");
   CmdArgs.push_back(Args.MakeArgString(ArchName));
 
   // FIXME: Is this needed anymore?
   if (ArchName == "arm")
     CmdArgs.push_back("-force_cpusubtype_ALL");
 }
 
-bool darwin::Linker::NeedsTempPath(const InputInfoList &Inputs,
+bool darwin::Linker::NeedsTempPath(const InputInfo &Output,
+                                   const InputInfoList &Inputs,
                                    const ArgList &Args) const {
   Arg *A = Args.getLastArg(options::OPT_g_Group);
   if (A && !A->getOption().matches(options::OPT_g0) &&
       !A->getOption().matches(options::OPT_gstabs))
     return true;
 
   // We only need to generate a temp path for LTO if we aren't compiling object
   // files. When compiling source files, we run 'dsymutil' after linking. We
   // don't run 'dsymutil' when compiling object files.
   for (const auto &Input : Inputs)
     if (Input.getType() != types::TY_Object)
       return true;
 
+  // If we're going to use lipo (i.e. the linker output is a temporary file too)
+  // then we'll need to run dsymutil after linking too
+  if (Inputs.size() != 1 && Output.getType() != types::TY_Nothing)
+    return true;
+
   return false;
 }
 
 /// Pass -no_deduplicate to ld64 under certain conditions:
 ///
 /// - Either -O0 or -O1 is explicitly specified
 /// - No -O option is specified *and* this is a compile+link (implicit -O0)
 ///
static bool shouldLinkerNotDedup(bool IsLinkerOnlyAction, const ArgList &Args) {
 
   if (!IsLinkerOnlyAction) // Implicit -O0 for compile+linker only.
     return true;
   return false;
 }
 
 void darwin::Linker::AddLinkArgs(Compilation &C, const ArgList &Args,
                                  ArgStringList &CmdArgs,
+                                 const InputInfo &Output,
                                  const InputInfoList &Inputs,
                                  unsigned Version[5]) const {
   const Driver &D = getToolChain().getDriver();
   const toolchains::MachO &MachOTC = getMachOToolChain();
 
   // Newer linkers support -demangle. Pass it if supported and not disabled by
   // the user.
   if (Version[0] >= 100 && !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
     CmdArgs.push_back("-demangle");
 
   if (Args.hasArg(options::OPT_rdynamic) && Version[0] >= 137)
     CmdArgs.push_back("-export_dynamic");
 
   // If we are using App Extension restrictions, pass a flag to the linker
   // telling it that the compiled code has been audited.
   if (Args.hasFlag(options::OPT_fapplication_extension,
                    options::OPT_fno_application_extension, false))
     CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs, Args)) {
+  if (D.isUsingLTO() && Version[0] >= 116 &&
+      NeedsTempPath(Output, Inputs, Args)) {
     std::string TmpPathName;
     if (D.getLTOMode() == LTOK_Full) {
       // If we are using full LTO, then automatically create a temporary file
       // path for the linker to use, so that it's lifetime will extend past a
       // possible dsymutil step.
       TmpPathName =
           D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
     } else if (D.getLTOMode() == LTOK_Thin)
void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (Arg *A = Args.getLastArg(options::OPT_mlinker_version_EQ)) {
     if (!Driver::GetReleaseVersion(A->getValue(), Version))
       getToolChain().getDriver().Diag(diag::err_drv_invalid_version_number)
           << A->getAsString(Args);
   }
 
   // I'm not sure why this particular decomposition exists in gcc, but
   // we follow suite for ease of comparison.
-  AddLinkArgs(C, Args, CmdArgs, Inputs, Version);
+  AddLinkArgs(C, Args, CmdArgs, Output, Inputs, Version);
 
   if (willEmitRemarks(Args) &&
       checkRemarksOptions(getToolChain().getDriver(), Args,
                           getToolChain().getTriple()))
     renderRemarksOptions(Args, CmdArgs, getToolChain().getTriple(), Output, JA);
 
   // Propagate the -moutline flag to the linker in LTO.
   if (Arg *A =

clang/test/Driver/darwin-dsymutil.c

 // runs dsymutil when -flto is specified and no source is provided
 // RUN: touch %t.o
 // RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 %t.o -g -flto -### \
 // RUN:   2> %t
 // RUN: FileCheck -check-prefix=CHECK-DSYMUTIL-LTO < %t %s
 
 // CHECK-DSYMUTIL-LTO: "/usr/bin/ld" "-demangle" "-object_path_lto"
 // CHECK-DSYMUTIL-LTO: "/usr/bin/dsymutil" "-o" "a.out.dSYM" "a.out"
+
+// Check that clang provides the linker with a temporary object file path and
+// runs dsymutil when building for multiple architectures.
+// RUN: touch %t.o
+// RUN: %clang -target x86_64-apple-darwin10 -arch x86_64 -arch arm64 %t.o -g \
+// RUN:     -flto -### 2> %t
+// RUN: FileCheck -check-prefix=CHECK-DSYMUTIL-MULTIARCH < %t %s
+
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/ld" "-demangle" "-object_path_lto"
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/ld" "-demangle" "-object_path_lto"
+// CHECK-DSYMUTIL-MULTIARCH: "/usr/bin/dsymutil" "-o" "a.out.dSYM" "a.out"