[clang] - Simplify tools::SplitDebugName

[grimar]

Summary

This is an updated version of the D54576, which was reverted.

Problem was that SplitDebugName calls the InputInfo::getFilename
which asserts if InputInfo given is not of type Filename:

const char *getFilename() const {
  assert(isFilename() && "Invalid accessor.");
  return Data.Filename;
}

At the same time at that point, it can be of type Nothing and
we need to use getBaseInput() it seems, like original code did.

[grimar]

With that the original issue mentioned in the following comment is fixed for me:
https://reviews.llvm.org/rL347035#299232

[dblaikie]

Cool - thanks! Any chance/way to add a test for this that'd show up sooner than the breakage caused by the previous version?

[JonasToth]

Thank you for the fix! I will try this version as soon as I am at home (~2 hours from now) and report back if the issues I had are gone.

[grimar]

In D55006#1311391, @dblaikie wrote:

Cool - thanks! Any chance/way to add a test for this that'd show up sooner than the breakage caused by the previous version?

Maybe, I am not sure. I was not able to trigger this without using the clang-tidy call yet.

[dblaikie]

In D55006#1311398, @grimar wrote:

In D55006#1311391, @dblaikie wrote:

Cool - thanks! Any chance/way to add a test for this that'd show up sooner than the breakage caused by the previous version?

Maybe, I am not sure. I was not able to trigger this without using the clang-tidy call yet.

Any chance other clang tools might reproduce this? clang-check maybe (sort of the most trivial clang tool)?

[grimar]

In D55006#1311400, @dblaikie wrote:

In D55006#1311398, @grimar wrote:

In D55006#1311391, @dblaikie wrote:

Cool - thanks! Any chance/way to add a test for this that'd show up sooner than the breakage caused by the previous version?

Maybe, I am not sure. I was not able to trigger this without using the clang-tidy call yet.

Any chance other clang tools might reproduce this? clang-check maybe (sort of the most trivial clang tool)?

I'll try to investigate this tomorrow. Never used it I think :)

[JonasToth]

The issues seems to be resolved for me as well, i will post this patch to the mailing list and ask the other guy if his build is fine, too.

[grimar]

In D55006#1312012, @JonasToth wrote:

The issues seems to be resolved for me as well, i will post this patch to the mailing list and ask the other guy if his build is fine, too.

Thanks, Jonas!

[grimar]

• Added the test case.

[JonasToth]

from my side no objections, mailing list did not react AFAIK (just in case your waiting for me until you recommit).

[grimar]

In D55006#1316985, @JonasToth wrote:

from my side no objections, mailing list did not react AFAIK (just in case your waiting for me until you recommit).

David requested the test case, I added it and now waiting for his approval :]

[dblaikie]

Looks good to me - tagging @labath @klimek here in case they have some further context on whether this is the right place for the test. But I'm fine with them providing post-commit review - please go ahead with this change as-is.

[grimar]

I am planning to revert it because of https://bugs.llvm.org/show_bug.cgi?id=41161

cfe/trunk/lib/Driver/ToolChains/Clang.cpp

#endif
   // can propagate it to the backend.
   bool SplitDWARF = (DwarfFission != DwarfFissionKind::None) &&
                     (RawTriple.isOSLinux() || RawTriple.isOSFuchsia()) &&
                     (isa<AssembleJobAction>(JA) || isa<CompileJobAction>(JA) ||
                      isa<BackendJobAction>(JA));
   const char *SplitDWARFOut;
   if (SplitDWARF) {
     CmdArgs.push_back("-split-dwarf-file");
-    SplitDWARFOut = SplitDebugName(Args, Input, Output);
+    SplitDWARFOut = SplitDebugName(Args, Output);
     CmdArgs.push_back(SplitDWARFOut);
   }
 
   // Pass the linker version in use.
   if (Arg *A = Args.getLastArg(options::OPT_mlinker_version_EQ)) {
     CmdArgs.push_back("-target-linker-version");
     CmdArgs.push_back(A->getValue());
   }
void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
   const llvm::Triple &T = getToolChain().getTriple();
   Arg *A;
   if ((getDebugFissionKind(D, Args, A) == DwarfFissionKind::Split) &&
       (T.isOSLinux() || T.isOSFuchsia())) {
     CmdArgs.push_back("-split-dwarf-file");
-    CmdArgs.push_back(SplitDebugName(Args, Input, Output));
+    CmdArgs.push_back(SplitDebugName(Args, Output));
   }
 
   assert(Input.isFilename() && "Invalid input.");
   CmdArgs.push_back(Input.getFilename());
 
   const char *Exec = getToolChain().getDriver().getClangProgramPath();
   C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
 }

cfe/trunk/lib/Driver/ToolChains/CommonArgs.h

 
 void AddHIPLinkerScript(const ToolChain &TC, Compilation &C,
                         const InputInfo &Output, const InputInfoList &Inputs,
                         const llvm::opt::ArgList &Args,
                         llvm::opt::ArgStringList &CmdArgs, const JobAction &JA,
                         const Tool &T);
 
 const char *SplitDebugName(const llvm::opt::ArgList &Args,
-                           const InputInfo &Input, const InputInfo &Output);
+                           const InputInfo &Output);
 
 void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
                     const JobAction &JA, const llvm::opt::ArgList &Args,
                     const InputInfo &Output, const char *OutFile);
 
 void AddGoldPlugin(const ToolChain &ToolChain, const llvm::opt::ArgList &Args,
                    llvm::opt::ArgStringList &CmdArgs, const InputInfo &Output,
                    const InputInfo &Input, bool IsThinLTO);

cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

 bool tools::areOptimizationsEnabled(const ArgList &Args) {
   // Find the last -O arg and see if it is non-zero.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group))
     return !A->getOption().matches(options::OPT_O0);
   // Defaults to -O0.
   return false;
 }
 
-const char *tools::SplitDebugName(const ArgList &Args, const InputInfo &Input,
+const char *tools::SplitDebugName(const ArgList &Args,
                                   const InputInfo &Output) {
+  SmallString<128> F(Output.isFilename()
+                         ? Output.getFilename()
+                         : llvm::sys::path::stem(Output.getBaseInput()));
+
   if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
     if (StringRef(A->getValue()) == "single")
-      return Args.MakeArgString(Output.getFilename());
+      return Args.MakeArgString(F);
 
-  Arg *FinalOutput = Args.getLastArg(options::OPT_o);
-  if (FinalOutput && Args.hasArg(options::OPT_c)) {
-    SmallString<128> T(FinalOutput->getValue());
-    llvm::sys::path::replace_extension(T, "dwo");
-    return Args.MakeArgString(T);
-  } else {
-    // Use the compilation dir.
-    SmallString<128> T(
-        Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
-    SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
-    llvm::sys::path::replace_extension(F, "dwo");
-    T += F;
-    return Args.MakeArgString(F);
-  }
-}
 
 void tools::SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
                            const JobAction &JA, const ArgList &Args,
                            const InputInfo &Output, const char *OutFile) {
   ArgStringList ExtractArgs;
   ExtractArgs.push_back("--extract-dwo");
 
   ArgStringList StripArgs;

cfe/trunk/lib/Driver/ToolChains/Gnu.cpp

void tools::gnutools::Assembler::ConstructJob(Compilation &C,
   C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
 
   // Handle the debug info splitting at object creation time if we're
   // creating an object.
   // TODO: Currently only works on linux with newer objcopy.
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
       getToolChain().getTriple().isOSLinux())
     SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-                   SplitDebugName(Args, Inputs[0], Output));
+                   SplitDebugName(Args, Output));
 }
 
 namespace {
 // Filter to remove Multilibs that don't exist as a suffix to Path
 class FilterNonExistent {
   StringRef Base, File;
   llvm::vfs::FileSystem &VFS;
 

cfe/trunk/lib/Driver/ToolChains/MinGW.cpp

void tools::MinGW::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   for (const auto &II : Inputs)
     CmdArgs.push_back(II.getFilename());
 
   const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath("as"));
   C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
 
   if (Args.hasArg(options::OPT_gsplit_dwarf))
     SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-                   SplitDebugName(Args, Inputs[0], Output));
+                   SplitDebugName(Args, Output));
 }
 
 void tools::MinGW::Linker::AddLibGCC(const ArgList &Args,
                                      ArgStringList &CmdArgs) const {
   if (Args.hasArg(options::OPT_mthreads))
     CmdArgs.push_back("-lmingwthrd");
   CmdArgs.push_back("-lmingw32");
 

cfe/trunk/test/Tooling/clang-check-extra-arg.cpp

 // RUN: clang-check "%s" -extra-arg=-Wunimplemented-warning -extra-arg-before=-Wunimplemented-warning-before -- -c 2>&1 | FileCheck %s
 
 // CHECK: unknown warning option '-Wunimplemented-warning-before'
 // CHECK: unknown warning option '-Wunimplemented-warning'
+
+// Check we do not crash with -extra-arg=-gsplit-dwarf (we did, under linux).
+// RUN: clang-check "%s" -extra-arg=-gsplit-dwarf -- -c
+
 void a(){}