MC: Don't emit min version directives when -fno-integrated-as is on

[majnemer]

Summary

The assembler included with cctools doesn't understand
.macosx_version_min. We shouldn't try to emit that directive if we
aren't going to use the integrated assembler.

This fixes PR21636.

[dblaikie]

I believe we've been trying to avoid special casing the integrated
assembler as much as possible (I think Rafael removed many instances of
this) - any other signal that would be appropriate to use?

[majnemer]

In D6371#4, @dblaikie wrote:

I believe we've been trying to avoid special casing the integrated
assembler as much as possible (I think Rafael removed many instances of
this) - any other signal that would be appropriate to use?

This patch is working around a directive which the cctools assembler does not have. Asking it to trigger on another condition would essentially be a proxy for "MC is targeting the cctools assembler" anyway.

[dblaikie]

In D6371#6, @majnemer wrote:

In D6371#4, @dblaikie wrote:

I believe we've been trying to avoid special casing the integrated
assembler as much as possible (I think Rafael removed many instances of
this) - any other signal that would be appropriate to use?

This patch is working around a directive which the cctools assembler does not have. Asking it to trigger on another condition would essentially be a proxy for "MC is targeting the cctools assembler" anyway.

Could we do this down in the MCStreamer instead?

It looks like MCAsmStreamer::EmitVersionMin should just be a no-op, maybe? (or a no-op on Darwin? Is the version directive valid anywhere else other than LLVM's integrated assembler?)

[majnemer]

In D6371#7, @dblaikie wrote:

In D6371#6, @majnemer wrote:

In D6371#4, @dblaikie wrote:

I believe we've been trying to avoid special casing the integrated
assembler as much as possible (I think Rafael removed many instances of
this) - any other signal that would be appropriate to use?

This patch is working around a directive which the cctools assembler does not have. Asking it to trigger on another condition would essentially be a proxy for "MC is targeting the cctools assembler" anyway.

Could we do this down in the MCStreamer instead?

It looks like MCAsmStreamer::EmitVersionMin should just be a no-op, maybe? (or a no-op on Darwin?

How would we be able to emit the version information for Darwin if it were a no-op? I might be missing something but it sounds like .ios_version_min and .macosx_version_min would not properly round-trip.

Is the version directive valid anywhere else other than LLVM's integrated assembler?)

AFAIK, only the LLVM integrated assembler understands .ios_version_min and .macosx_version_min. I've checked the cctools sources and the latest binutils GAS sources as well.

[grosbach]

The non integrated assembler can't handle lots of things the compiler emits on Darwin. The only realistic use of that command line option now is when processing .s files.

Sent from my iPad

[• rafael]

The cctools assembler is not supported. We should not do this.

[• jeremyhu]

FWIW, cctools' as doesn't understand .cfi_startproc et al either, so even if we did this, we'd barf over those. While this single case is quite trivial, I could see this snowballing messily elsewhere. =/

[thakis]

I'm trying to use afl-fuzz with asan on OS X. afl-fuzz tells you to set CXX to afl-clang++ and rebuild your program. afl-clang++ then calls clang++ with -no-integrated-as and -B to a path with its own as wrapper that inserts some instrumentation assembly before calling real as. This doesn't work on OS X because clang++ writes this .macosx_version_min directive that as doesn't support. If I apply this patch, everything does work. So this seems like a useful thing to me – are there any downsides to this patch?

Are there any other possible fixes for this use-case? Does llvm have an as-compatible assembler driver somewhere?

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

bool AsmPrinter::doInitialization(Module &M) {
   //
   // FIXME: If we end up with a collection of these sorts of Darwin-specific
   // or ELF-specific things, it may make sense to have a platform helper class
   // that will work with the target helper class. For now keep it here, as the
   // alternative is duplicated code in each of the target asm printers that
   // use the directive, where it would need the same conditionalization
   // anyway.
   Triple TT(getTargetTriple());
-  if (TT.isOSDarwin()) {
+  if (TT.isOSDarwin() && MAI->useIntegratedAssembler()) {
     unsigned Major, Minor, Update;
     TT.getOSVersion(Major, Minor, Update);
     // If there is a version specified, Major will be non-zero.
     if (Major)
       OutStreamer.EmitVersionMin((TT.isMacOSX() ?
                                   MCVM_OSXVersionMin : MCVM_IOSVersionMin),
                                  Major, Minor, Update);
   }

test/MC/MachO/no-integrated-as.ll

+; RUN: llc -mtriple=x86_64-apple-darwin10.9 -no-integrated-as -o - %s > %t
+; RUN: FileCheck --check-prefix=CHECK-ASM < %t %s
+
+; CHECK-ASM-NOT: .macosx_version_min
+
+; RUN: llc -mtriple=x86_64-apple-darwin10.9 -no-integrated-as -filetype=obj -o - %s | macho-dump > %t
+; RUN: FileCheck --check-prefix=CHECK-OBJ < %t %s
+
+; CHECK-OBJ-NOT: (('command', 36)