[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 ↗

(On Diff #416846)

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 ↗

(On Diff #416846)

-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/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;
void Clang::DumpCompilationDatabase(Compilation &C, StringRef Filename,
   Buf += types::getTypeName(Input.getType());
   CDB << ", \"" << escape(Buf) << "\"";
   if (!D.SysRoot.empty() && !Args.hasArg(options::OPT__sysroot_EQ)) {
     Buf = "--sysroot=";
     Buf += D.SysRoot;
     CDB << ", \"" << escape(Buf) << "\"";
   }
   CDB << ", \"" << escape(Input.getFilename()) << "\"";
+  CDB << ", \"-o\", \"" << escape(Output.getFilename()) << "\"";
   for (auto &A: Args) {
     auto &O = A->getOption();
     // Skip language selection, which is positional.
     if (O.getID() == options::OPT_x)
       continue;
     // Skip writing dependency output and the compilation database itself.
     if (O.getGroup().isValid() && O.getGroup().getID() == options::OPT_M_Group)
       continue;
     if (O.getID() == options::OPT_gen_cdb_fragment_path)
       continue;
     // Skip inputs.
     if (O.getKind() == Option::InputClass)
       continue;
+    // Skip output.
+    if (O.getID() == options::OPT_o)
+      continue;
     // All other arguments are quoted and appended.
     ArgStringList ASL;
     A->render(Args, ASL);
     for (auto &it: ASL)
       CDB << ", \"" << escape(it) << "\"";
   }
   Buf = "--target=";
   Buf += Target;

clang/test/Driver/compilation_database.c

 // RUN: mkdir -p %t.workdir && cd %t.workdir
 // RUN: %clang -no-canonical-prefixes -fintegrated-as -MD -MP --sysroot=somewhere -c -x c %s -xc++ %s -Wall -MJ - 2>&1 | FileCheck %s
 // RUN: not %clang -no-canonical-prefixes -c -x c %s -MJ %s/non-existant 2>&1 | FileCheck --check-prefix=ERROR %s
 
-// CHECK: { "directory": "{{[^"]*}}workdir",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "output": "compilation_database.o", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc", "[[SRC]]", "-no-canonical-prefixes", "-fintegrated-as", "--sysroot=somewhere", "-c", "-Wall",{{.*}} "--target={{[^"]+}}"]},
-// CHECK: { "directory": "{{.*}}",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "output": "compilation_database.o", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc++", "[[SRC]]", "-no-canonical-prefixes", "-fintegrated-as", "--sysroot=somewhere", "-c", "-Wall",{{.*}} "--target={{[^"]+}}"]},
+// CHECK: { "directory": "{{[^"]*}}workdir",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "output": "compilation_database.o", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc", "[[SRC]]", "-o", "compilation_database.o", "-no-canonical-prefixes", "-fintegrated-as", "--sysroot=somewhere", "-c", "-Wall",{{.*}} "--target={{[^"]+}}"]},
+// CHECK: { "directory": "{{.*}}",  "file": "[[SRC:[^"]+[/|\\]compilation_database.c]]", "output": "compilation_database.o", "arguments": ["{{[^"]*}}clang{{[^"]*}}", "-xc++", "[[SRC]]", "-o", "compilation_database.o", "-no-canonical-prefixes", "-fintegrated-as", "--sysroot=somewhere", "-c", "-Wall",{{.*}} "--target={{[^"]+}}"]},
 // ERROR: error: compilation database '{{.*}}/non-existant' could not be opened:
 
 int main(void) {
   return 0;
 }

clang/test/Driver/compilation_database_multiarch.c

+// RUN: rm -rf %t && mkdir -p %t
+// RUN: %clang -fdriver-only -o %t/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": "[[OUTPUT_X86_64:.*]]", "arguments": [{{.*}}, "-o", "[[OUTPUT_X86_64]]", {{.*}} "--target=x86_64-apple-macosx12.0.0"]},
+// CHECK-NEXT: { "directory": "{{.*}}", "file": "{{.*}}", "output": "[[OUTPUT_ARM64:.*]]", "arguments": [{{.*}}, "-o", "[[OUTPUT_ARM64]]", {{.*}} "--target=arm64-apple-macosx12.0.0"]},