driver: Don't warn about assembler flags being unused when not assembling

[thakis]

Summary

clang currently warns when passing flags for the assembler (e.g.
-Wa,-mbig-obj) to an invocation that doesn't run the assembler (e.g.
-E).

At first sight, that makes sense -- the flag really is unused. But many
other flags don't have an effect if no assembler runs (e.g.
-fno-integrated-as, -ffunction-sections, and many others), and those
currently don't warn. So this seems more like a side effect of how
CollectArgsForIntegratedAssembler() is implemented than like an
intentional feature.

Since it's a bit inconvenient when debugging builds and adding -E,
always call CollectArgsForIntegratedAssembler() to make sure assembler
args always get claimed. Currently, this affects only these flags:
-mincremental-linker-compatible, -mimplicit-it= (on ARM), -Wa, -Xassembler

It does have the side effect that assembler options now need to be valid
even if -E is passed. Previously, -Wa,-mbig-obj would error for
non-coff output only if the assembler ran, now it always errors. This
too makes assembler flags more consistent with all the other flags and
seems like a progression.

Fixes PR42066.

[rnk]

lgtm

[nathanchance]

This change breaks building the Linux kernel for arm32 (at least):

...
  YACC    scripts/dtc/dtc-parser.tab.c
  HOSTCC  scripts/dtc/yamltree.o
  HOSTCC  scripts/dtc/dtc-parser.tab.o
  HOSTCC  scripts/dtc/dtc-lexer.lex.o
  HOSTLD  scripts/dtc/dtc
  UPD     include/config/kernel.release
  UPD     include/generated/utsrelease.h
  HOSTCC  scripts/asn1_compiler
  HOSTCC  scripts/extract-cert
  HOSTCC  scripts/sortextable
  HOSTCC  scripts/kallsyms
  HOSTCC  scripts/conmakehash
  SYSNR   arch/arm/include/generated/asm/unistd-nr.h
  SYSTBL  arch/arm/include/generated/calls-oabi.S
  GEN     arch/arm/include/generated/asm/mach-types.h
  SYSTBL  arch/arm/include/generated/calls-eabi.S
  HOSTCC  scripts/mod/mk_elfconfig
  CC      scripts/mod/devicetable-offsets.s
  CC      scripts/mod/empty.o
clang-9: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,'
make[2]: *** [scripts/Makefile.build:112: scripts/mod/devicetable-offsets.s] Error 1
make[2]: *** Waiting for unfinished jobs....
clang-9: error: unsupported argument '-mno-warn-deprecated' to option 'Wa,'
make[2]: *** [scripts/Makefile.build:279: scripts/mod/empty.o] Error 1
make[1]: *** [Makefile:1118: prepare0] Error 2
make: *** [Makefile:325: __build_one_by_one] Error 2

The full command line that causes the issue is:

/home/nathan/cbl/tc-build/build/llvm/stage1/bin/clang -Wp,-MD,scripts/mod/.empty.o.d  -nostdinc -isystem /home/nathan/cbl/tc-build/build/llvm/stage1/lib/clang/9.0.0/include -I./arch/arm/include -I./arch/arm/include/generated  -I./include -I./arch/arm/include/uapi -I./arch/arm/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian -Qunused-arguments -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 --target=arm-linux-gnueabi --prefix=/home/nathan/cbl/usr/bin/ --gcc-toolchain=/home/nathan/cbl/usr -no-integrated-as -Werror=unknown-warning-option -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mfpu=vfp -funwind-tables -marm -Wa,-mno-warn-deprecated -D__LINUX_ARM_ARCH__=7 -march=armv7-a -msoft-float -Uarm -O2 -fstack-protector-strong -Wno-format-invalid-specifier -Wno-gnu -Wno-tautological-compare -mno-global-merge -fomit-frame-pointer -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-initializer-overrides -Wno-unused-value -Wno-format -Wno-sign-compare -Wno-format-zero-length    -DKBUILD_BASENAME='"empty"' -DKBUILD_MODNAME='"empty"' -c -o scripts/mod/empty.o scripts/mod/empty.c

I'm not in the right state of mind (exhausted) to debug this but I wanted to let you know in case you have any immediate ideas.

[nickdesaulniers]

This change breaks building the Linux kernel for arm32 (at least):

Follow up discussion in https://github.com/ClangBuiltLinux/linux/issues/598. I think the kernel is wrong here.

[nickdesaulniers]

ah, maybe the addition of -no-integrated-as shouldn't produce this error?

[nickdesaulniers]

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

3555

Should you be checking !TC.IsIntegratedAssemblerDefault() additionally?

[rnk]

I think the intention of this change was to ignore assembler flags in pre-processing actions without warning about them. It implements that behavior by running the code that gathers and validates assembler flags. The validation is whats emitting the problematic errors. I don't think that's desirable, so I'd recommend reverting for now.

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

3555

I don't think so. We want to ignore assembler flags in a preprocessor action even if we're not going to use the integrated assembler.

[nickdesaulniers]

Fixup: https://reviews.llvm.org/D64655

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

void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   // Push all default warning arguments that are specific to
   // the given target.  These come before user provided warning options
   // are provided.
   TC.addClangWarningOptions(CmdArgs);
 
   // Select the appropriate action.
   RewriteKind rewriteKind = RK_None;
 
+  // If CollectArgsForIntegratedAssembler() isn't called below, call it here
+  // with a dummy args list to mark assembler flags as used even when not
+  // running an assembler. Otherwise, clang would emit "argument unused"
+  // warnings for assembler flags when e.g. adding "-E" to flags while debugging
+  // something. That'd be somewhat inconvenient, and it's also inconsistent with
+  // most other flags -- we don't warn on -ffunction-sections not being used
+  // in -E mode either for example, even though it's not really used either.
+  if (!isa<AssembleJobAction>(JA)) {
+    ArgStringList DummyArgs;
+    CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D);
+  }
+
   if (isa<AnalyzeJobAction>(JA)) {
     assert(JA.getType() == types::TY_Plist && "Invalid output type.");
     CmdArgs.push_back("-analyze");
   } else if (isa<MigrateJobAction>(JA)) {
     CmdArgs.push_back("-migrate");
   } else if (isa<PreprocessJobAction>(JA)) {
     if (Output.getType() == types::TY_Dependencies)
       CmdArgs.push_back("-Eonly");

cfe/trunk/test/Driver/as-options.s

 // RUN:   -Ifoo_dir -### 2>&1 \
 // RUN:   | FileCheck %s
 
 // RUN: %clang -target armv7-linux-gnueabihf -c -integrated-as %s \
 // RUN:   -Ifoo_dir -### 2>&1 \
 // RUN:   | FileCheck %s
 
 // CHECK: "-I" "foo_dir"
+
+// Test that assembler options don't cause warnings when there's no assembler
+// stage.
+
+// RUN: %clang -mincremental-linker-compatible -E -o /dev/null %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -mimplicit-it=always -target armv7-linux-gnueabihf -E %s \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+// RUN: %clang -Wa,-mbig-obj -target i386-pc-windows -E %s \
+// RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
+// WARN-NOT: unused