[clang][driver] Fix compilation database dump with multiple architectures

[jansvoboda11]

Summary

Command lines with multiple -arch arguments expand into multiple entries in the compilation database. However, the file writes are not appending, meaning subsequent writes end up overwriting the previous ones, resulting in garbled output.

This patch fixes that by always appending to the file.

rdar://90165004

[dexonsmith]

clang/test/Driver/compilation_database_multiarch.c

3

Usually driver tests don't depend on the -cc1 working. The -cc1 depends on the backends. You'd need to check that the relevant targets have been built and linked in. It'd be better to keep this isolated to the driver?

Is there a way to get the driver to dump the compilation database only (without actually running the actions)?

E.g., will -### still emit the compilation database? Looks like not:

// If this is a dry run, do not create the compilation database file.
if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
  return;

But maybe there's some other way to avoid running the actions. Or, we can add something like -MJ-only <cdb>, which acts like -MJ <cdb> and then exits before running the actions.

6–7

I guess this is off topic, but this is the wrong output. Would be good to fix this — maybe first to avoid adding a test for the wrong behaviour. Running on macOS 12, I would expect:

% clang -arch x86_64 -arch arm64 /dev/null -x c -###

to emit

".../path/to/clang" "-cc1" "-triple" "x86_64-apple-macosx12.0.0" ...
".../path/to/clang" "-cc1" "-triple" "arm64-apple-macosx12.0.0" ...

and that's what I get from the clang that comes with Xcode. But I reproduced the incorrect output you're checking for here with the clang at llvm.org.

Is there something we forgot to upstream? @arphaman, do you know?

[dexonsmith]

clang/test/Driver/compilation_database_multiarch.c

6–7

Heh, typo in my reproducer above, which I fixed when I ran it to check but forgot to copy back into Phabricator. Correctly-spelled -### command is:

% clang -arch x86_64 -arch arm64 -x c /dev/null -c -###

[jansvoboda11]

Specify the apple-darwin target

[jansvoboda11]

clang/test/Driver/compilation_database_multiarch.c

3

If we manage to only run the driver (without invoking the -cc1s), would the question of built targets go away?

Flang has a frontend option -init-only. Maybe we could add something similar to that to Clang's driver? (-driver-only) Creating a specialized -MJ-only seems too specific IMO.

[jansvoboda11]

Avoid buggy driver behavior

[dexonsmith]

clang/test/Driver/compilation_database_multiarch.c

6–7

https://reviews.llvm.org/D122030

[dexonsmith]

In D121997#3393079, @jansvoboda11 wrote:

Avoid buggy driver behavior

I just remembered the "correct" way to specify a platform without an SDK if you're using -arch: -mmacosx-version-min=12.0.0. That's probably better for this test than using a half-ignored -target.

[dexonsmith]

clang/test/Driver/compilation_database_multiarch.c

3

Sure, -driver-only makes sense to me:

• Run driver, including side effects.
• Don't run actions.

It'd be worth testing that it works correctly with -v. My intuition is that -driver-only -v should do the same thing as -###, except that -### suppresses driver outputs that come before running the actions.

[jansvoboda11]

Introduce -driver-only to avoid running -cc1s.

[dexonsmith]

The "Fix compilation database dump" changes LGTM, assuming you split out -driver-only and land it separately ahead of the rest of this.

In D121997#3395562, @jansvoboda11 wrote:

Introduce -driver-only to avoid running -cc1s.

-driver-only looks mostly great, but should really be committed (and tested) separately.

I added a comment inline about -v / CCPrintOptions.

For the testing, I suggest:

• Add driver-only.c, where you can test a normal compilation (unrelated to -MJ).
  • If you implement -v, you can check its output.
  • Add a RUN: not %clang -driver-only in some case where it should fail.
• If you implement CCPrintOptions with -v, update clang/test/Driver/cc-print-options.c with an additional RUN line.
• Update clang/test/Driver/gen-cdb-fragment.c to use -driver-only and remove REQUIRES: x86-registered-target.

clang/lib/Driver/Driver.cpp

1600–1607

I think if OPT_v then OPT_driver_only should call C.getJobs().Print() somehow... ideally, using the logic that's at the top of Compilation::ExecuteCommand(), also implementing the analogous CCPrintOptions, but that looks potentially awkward... you maybe want to call ExecuteJobs below with a flag that says "don't execute" or something.

Happy for those things to come in a follow-up though; just leave behind FIXME(s) if so.

1609–1611

-driver-only should return 1 in this case. Please add a test for this as well.

[jansvoboda11]

Extract -fdriver-only into separate patch, ensure output file is consistent.

[Jake-Egan]

Hi, it seems on AIX the target and arch options are ignored, so we don't get the expected output for this test. Any ideas about the course of action we should take?
https://lab.llvm.org/buildbot/#/builders/214/builds/1785/steps/6/logs/FAIL__Clang__compilation_database_multiarch_c

[jansvoboda11]

In D121997#3577690, @Jake-Egan wrote:

Hi, it seems on AIX the target and arch options are ignored, so we don't get the expected output for this test. Any ideas about the course of action we should take?
https://lab.llvm.org/buildbot/#/builders/214/builds/1785/steps/6/logs/FAIL__Clang__compilation_database_multiarch_c

Hey, I disabled the test on non-Darwin hosts in 2de36d0369a5107a3f02aceebb55326adf4ad2cb. Your build bot is back to green.

[alanphipps]

Should there be an aarch64 guard on the test? There exist downstream Arm compilers that don't support Arm64 and fail. I know other tests explicitly state "REQUIRES: aarch64-registered-target"

[jansvoboda11]

In D121997#3676972, @alanphipps wrote:

Should there be an aarch64 guard on the test? There exist downstream Arm compilers that don't support Arm64 and fail. I know other tests explicitly state "REQUIRES: aarch64-registered-target"

Could you describe your build config? I don't need to enable the Aarch64 or X86 target for this test to pass on macOS.

clang/include/clang/Driver/Options.td

HelpText<"Enable optimizations based on the strict definition of an enum's "
            "value range">,
   MarshallingInfoFlag<CodeGenOpts<"StrictEnums">>;
 defm strict_vtable_pointers : BoolFOption<"strict-vtable-pointers",
   CodeGenOpts<"StrictVTablePointers">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Enable optimizations based on the strict rules for"
             " overwriting polymorphic C++ objects">,
   NegFlag<SetFalse>>;
 def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>;
+def driver_only : Flag<["-"], "driver-only">, Flags<[NoXarchOption, CoreOption]>, HelpText<"Only run the driver.">;
 def fsyntax_only : Flag<["-"], "fsyntax-only">,
   Flags<[NoXarchOption,CoreOption,CC1Option,FC1Option]>, Group<Action_Group>;
 def ftabstop_EQ : Joined<["-"], "ftabstop=">, Group<f_Group>;
 def ftemplate_depth_EQ : Joined<["-"], "ftemplate-depth=">, Group<f_Group>;
 def ftemplate_depth_ : Joined<["-"], "ftemplate-depth-">, Group<f_Group>;
 def ftemplate_backtrace_limit_EQ : Joined<["-"], "ftemplate-backtrace-limit=">,
                                    Group<f_Group>;
 def foperator_arrow_depth_EQ : Joined<["-"], "foperator-arrow-depth=">,

clang/lib/Driver/Driver.cpp

void Driver::setUpResponseFiles(Compilation &C, Command &Cmd) {
 
   std::string TmpName = GetTemporaryPath("response", "txt");
   Cmd.setResponseFile(C.addTempFile(C.getArgs().MakeArgString(TmpName)));
 }
 
 int Driver::ExecuteCompilation(
     Compilation &C,
     SmallVectorImpl<std::pair<int, const Command *>> &FailingCommands) {
+  if (C.getArgs().hasArg(options::OPT_driver_only))
+    return 0;
+
   // Just print if -### was present.
   if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) {
     C.getJobs().Print(llvm::errs(), "\n", true);
     return 0;
   }
 
   // If there were errors building the compilation, quit now.
   if (Diags.hasErrorOccurred())
     return 1;
 
   // Set up response file names for each command, if necessary.
   for (auto &Job : C.getJobs())
     setUpResponseFiles(C, Job);
 
   C.ExecuteJobs(C.getJobs(), FailingCommands);
 
   // If the command succeeded, we are done.
bool Driver::HandleImmediateArgs(const Compilation &C) {
 
   if (C.getArgs().hasArg(options::OPT__version)) {
     // Follow gcc behavior and use stdout for --version and stderr for -v.
     PrintVersion(C, llvm::outs());
     return false;
   }
 
   if (C.getArgs().hasArg(options::OPT_v) ||
+      C.getArgs().hasArg(options::OPT_driver_only) ||
       C.getArgs().hasArg(options::OPT__HASH_HASH_HASH) ||
       C.getArgs().hasArg(options::OPT_print_supported_cpus)) {
     PrintVersion(C, llvm::errs());
     SuppressMissingInputWarning = true;
   }
 
   if (C.getArgs().hasArg(options::OPT_v)) {
     if (!SystemConfigDir.empty())
void Driver::BuildJobs(Compilation &C) const {
   }
 
   // If the user passed -Qunused-arguments or there were errors, don't warn
   // about any unused arguments.
   if (Diags.hasErrorOccurred() ||
       C.getArgs().hasArg(options::OPT_Qunused_arguments))
     return;
 
+  // Claim -driver-only here.
+  (void)C.getArgs().hasArg(options::OPT_driver_only);
   // Claim -### here.
   (void)C.getArgs().hasArg(options::OPT__HASH_HASH_HASH);
 
   // Claim --driver-mode, --rsp-quoting, it was handled earlier.
   (void)C.getArgs().hasArg(options::OPT_driver_mode);
   (void)C.getArgs().hasArg(options::OPT_rsp_quoting);
 
   for (Arg *A : C.getArgs()) {

clang/lib/Driver/ToolChains/Clang.cpp

if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH))
     return;
 
   using llvm::yaml::escape;
   const Driver &D = getToolChain().getDriver();
 
   if (!CompilationDatabase) {
     std::error_code EC;
     auto File = std::make_unique<llvm::raw_fd_ostream>(
-        Filename, EC, llvm::sys::fs::OF_TextWithCRLF);
+        Filename, EC,
+        llvm::sys::fs::OF_TextWithCRLF | llvm::sys::fs::OF_Append);
     if (EC) {
       D.Diag(clang::diag::err_drv_compilationdatabase) << Filename
                                                        << EC.message();
       return;
     }
     CompilationDatabase = std::move(File);
   }
   auto &CDB = *CompilationDatabase;

clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);
 
   std::unique_ptr<driver::Compilation> C(TheDriver.BuildCompilation(Args));
   if (!C)
     return nullptr;
 
+  if (C->getArgs().hasArg(driver::options::OPT_driver_only))
+    return nullptr;
+
   // Just print the cc1 options if -### was present.
   if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH)) {
     C->getJobs().Print(llvm::errs(), "\n", true);
     return nullptr;
   }
 
   // We expect to get back exactly one command job, if we didn't something
   // failed. Offload compilation is an exception as it creates multiple jobs. If

clang/test/Driver/compilation_database_multiarch.c

+// RUN: rm -rf %t && mkdir -p %t
+// RUN: %clang -driver-only -o %/out %s -mtargetos=macos12 -arch arm64 -arch x86_64 -MJ %t/compilation_database.json
+// RUN: FileCheck --input-file=%t/compilation_database.json %s
+
+// CHECK:      { "directory": "{{.*}}", "file": "{{.*}}", "output": "{{.*}}", "arguments": [{{.*}} "--target=x86_64-apple-macosx12.0.0"]},
+// CHECK-NEXT: { "directory": "{{.*}}", "file": "{{.*}}", "output": "{{.*}}", "arguments": [{{.*}} "--target=arm64-apple-macosx12.0.0"]},