[Tooling] Allow -flto flags and filter out -Wa, flags

[chh]

Summary

This change fixes the problem in https://bugs.llvm.org/show_bug.cgi?id=38332
by allowing driver::Action::BackendJobClass to run with the analyzer.
Otherwise, such jobs will look up the non-existing compilation database and then run without flags.
Also filter out the -Wa,* flags that could be passed to and ignored by the clang compiler.
Clang-tidy gives warnings about unused -Wa,* flags.

[ioeric]

lib/Tooling/CompilationDatabase.cpp

304

It seems to me that BackendJob will always be preceded by a CompileJob (according to https://github.com/llvm-mirror/clang/blob/master/lib/Driver/Driver.cpp#L3506), so at least of one job would be CompileJobClass here. Not sure if I understood it correctly though. Would you mind clarify?

[chh]

I tested with clang-tidy test.cpp -- -c -Iinc -flto,
and found that in Driver.cpp:getTool Inputs has one action equal to
BackendJobClass (without -flto) or CompileJobClass (with -flto).

When in CompilationDatabase:stripPositionalArgs, the Jobs has only one
AssembleJobClass (without -flto) or BackendJobClass (with -flto).
It is confusing to change the Backend/Compile to Assemble/Backend.
However, I have not seen multiple jobs yet.
Do you have a test case that could trigger multiple jobs with or without -flto?

[srhines]

lib/Tooling/CompilationDatabase.cpp

304

Is the difference here because clang driver != clang-tidy driver?

[chh]

lib/Tooling/CompilationDatabase.cpp

304

The drivers are different but share some logic. This part is not used by clang driver.
For clang-tidy and clang-check, it looks like trying to skip Link job, but to handle Assemble, Compile, and Backend jobs.

[ioeric]

In D51002#1208152, @chh wrote:

I tested with clang-tidy test.cpp -- -c -Iinc -flto,
and found that in Driver.cpp:getTool Inputs has one action equal to
BackendJobClass (without -flto) or CompileJobClass (with -flto).

When in CompilationDatabase:stripPositionalArgs, the Jobs has only one
AssembleJobClass (without -flto) or BackendJobClass (with -flto).
It is confusing to change the Backend/Compile to Assemble/Backend.
However, I have not seen multiple jobs yet.
Do you have a test case that could trigger multiple jobs with or without -flto?

Not really. I'm not familiar with backend jobs and just wanted to ask for clarification. Thanks!

lib/Tooling/CompilationDatabase.cpp

172

Do we also need to something for BackendJob here?

lib/Tooling/CompilationDatabase.cpp

void runImpl(const driver::Action *A, bool Collect) {
 
     case driver::Action::InputClass:
       if (Collect) {
         const auto *IA = cast<driver::InputAction>(A);
         Inputs.push_back(IA->getInputArg().getSpelling());
       }
       break;
 
     default:
       // Don't care about others
       break;
     }
 
     for (const driver::Action *AI : A->inputs())
       runImpl(AI, CollectChildren);
   }
 };
for (const std::string *I = Arr.begin(), *E = Arr.end(); I != E; ++I)
         return true;
     return false;
   }
 
 private:
   ArrayRef<std::string> Arr;
 };
 
+// Filter of tools unused flags such as -no-integrated-as and -Wa,*.
+// They are not used for syntax checking, and could confuse targets
+// which don't support these options.
+struct FilterUnusedFlags {
+  bool operator() (StringRef S) {
+    return (S == "-no-integrated-as") || S.startswith("-Wa,");
+  }
+};
+
 } // namespace
 
 /// Strips any positional args and possible argv[0] from a command-line
 /// provided by the user to construct a FixedCompilationDatabase.
 ///
 /// FixedCompilationDatabase requires a command line to be in this format as it
 /// constructs the command line for each file by appending the name of the file
 /// to be compiled. FixedCompilationDatabase also adds its own argv[0] to the
static bool stripPositionalArgs(std::vector<const char *> Args,
   Args.push_back("-c");
 
   // Put a dummy C++ file on to ensure there's at least one compile job for the
   // driver to construct. If the user specified some other argument that
   // prevents compilation, e.g. -E or something like -version, we may still end
   // up with no jobs but then this is the user's fault.
   Args.push_back("placeholder.cpp");
 
-  // Remove -no-integrated-as; it's not used for syntax checking,
-  // and it confuses targets which don't support this option.
-  Args.erase(std::remove_if(Args.begin(), Args.end(),
-                            MatchesAny(std::string("-no-integrated-as"))),
+  Args.erase(std::remove_if(Args.begin(), Args.end(), FilterUnusedFlags()),
              Args.end());
 
   const std::unique_ptr<driver::Compilation> Compilation(
       NewDriver->BuildCompilation(Args));
   if (!Compilation)
     return false;
 
   const driver::JobList &Jobs = Compilation->getJobs();
 
   CompileJobAnalyzer CompileAnalyzer;
 
   for (const auto &Cmd : Jobs) {
-    // Collect only for Assemble and Compile jobs. If we do all jobs we get
-    // duplicates since Link jobs point to Assemble jobs as inputs.
+    // Collect only for Assemble, Backend, and Compile jobs. If we do all jobs
+    // we get duplicates since Link jobs point to Assemble jobs as inputs.
+    // -flto* flags make the BackendJobClass, which still needs analyzer.
     if (Cmd.getSource().getKind() == driver::Action::AssembleJobClass ||
+        Cmd.getSource().getKind() == driver::Action::BackendJobClass ||
         Cmd.getSource().getKind() == driver::Action::CompileJobClass) {
       CompileAnalyzer.run(&Cmd.getSource());
     }
   }
 
   if (CompileAnalyzer.Inputs.empty()) {
     ErrorMsg = "warning: no compile jobs found\n";
     return false;

test/Tooling/clang-check-analyzer.cpp

 // RUN: clang-check -analyze "%s" -- -c 2>&1 | FileCheck %s
+// RUN: clang-check -analyze "%s" -- -c -flto -Wa,--noexecstack 2>&1 | FileCheck %s
+// RUN: clang-check -analyze "%s" -- -c -no-integrated-as -flto=thin 2>&1 | FileCheck %s
+// RUN: clang-check -analyze "%s" -- -c -flto=full 2>&1 | FileCheck %s
 
 // CHECK: Dereference of null pointer
 void a(int *x) { if(x){} *x = 47; }