DarwinAsmParser: Warn on multiple/mismatching version_min directives

[MatzeB]

Summary

• Add proxy function for MCAsmParser::Note() to MCAsmParserExtension
• Warn on multiple version_min directives
• Warn if .xxx_min_version does not match target triple

[steven_wu]

I am not sure if we should warn for UnknownOS. I guess there can be argument both way but we should at least have a test case for it. I am also not sure if you can run i386 asm parser when x86 backend is not built.

[MatzeB]

In D11492#211728, @steven_wu wrote:

I am not sure if we should warn for UnknownOS. I guess there can be argument both way but we should at least have a test case for it. I am also not sure if you can run i386 asm parser when x86 backend is not built.

The "Triple::OSType ExpectedOS = Triple::UnknownOS;" initialization is only there to avoid uninitialized variable warnings, in practice you will always hit one of the switch cases below it.

[MatzeB]

In D11492#211728, @steven_wu wrote:

I am also not sure if you can run i386 asm parser when x86 backend is not built.

I think you are right here. I was looking at the version-min-diagnostics.s test which made the same error when I wrote this. For this test I can switch to the nonexisting thumbv7-apple-macosx.

[steven_wu]

In D11492#211737, @MatzeB wrote:

In D11492#211728, @steven_wu wrote:

I am not sure if we should warn for UnknownOS. I guess there can be argument both way but we should at least have a test case for it. I am also not sure if you can run i386 asm parser when x86 backend is not built.

The "Triple::OSType ExpectedOS = Triple::UnknownOS;" initialization is only there to avoid uninitialized variable warnings, in practice you will always hit one of the switch cases below it.

I was actually referring to what if T.getOS() is UnknownOS. With some more thought, I realize version_min directive will not even parse with -triple armv7-apple. The only case might be interesting is what about armv7-apple-darwin?

[MatzeB]

In the meantime this functionality landed in llvm in r251569.

include/llvm/MC/MCParser/MCAsmParserExtension.h

public:
   SourceMgr &getSourceManager() { return getParser().getSourceManager(); }
   MCStreamer &getStreamer() { return getParser().getStreamer(); }
   bool Warning(SMLoc L, const Twine &Msg) {
     return getParser().Warning(L, Msg);
   }
   bool Error(SMLoc L, const Twine &Msg) {
     return getParser().Error(L, Msg);
   }
+  void Note(SMLoc L, const Twine &Msg) {
+    getParser().Note(L, Msg);
+  }
   bool TokError(const Twine &Msg) {
     return getParser().TokError(Msg);
   }
 
   const AsmToken &Lex() { return getParser().Lex(); }
 
   const AsmToken &getTok() { return getParser().getTok(); }
 
   bool HasBracketExpressions() const { return BracketExpressionsSupported; }
 
   /// @}
 };
 
 } // End llvm namespace
 
 #endif

lib/MC/MCParser/DarwinAsmParser.cpp

 //===- DarwinAsmParser.cpp - Darwin (Mach-O) Assembly Parser --------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/MC/MCParser/MCAsmParserExtension.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
MCAsmParser::ExtensionDirectiveHandler Handler = std::make_pair(
         this, HandleDirective<DarwinAsmParser, HandlerMethod>);
     getParser().addDirectiveHandler(Directive, Handler);
   }
 
   bool parseSectionSwitch(const char *Segment, const char *Section,
                           unsigned TAA = 0, unsigned ImplicitAlign = 0,
                           unsigned StubSize = 0);
 
+  SMLoc LastVersionMinDirective;
+
 public:
   DarwinAsmParser() {}
 
   void Initialize(MCAsmParser &Parser) override {
     // Call the base implementation.
     this->MCAsmParserExtension::Initialize(Parser);
 
     addDirectiveHandler<&DarwinAsmParser::parseDirectiveDesc>(".desc");
void Initialize(MCAsmParser &Parser) override {
     addDirectiveHandler<&DarwinAsmParser::parseSectionDirectiveThreadInitFunc>(
       ".thread_init_func");
     addDirectiveHandler<&DarwinAsmParser::parseSectionDirectiveTLV>(".tlv");
 
     addDirectiveHandler<&DarwinAsmParser::parseSectionDirectiveIdent>(".ident");
     addDirectiveHandler<&DarwinAsmParser::parseVersionMin>(".ios_version_min");
     addDirectiveHandler<&DarwinAsmParser::parseVersionMin>(
       ".macosx_version_min");
+
+    LastVersionMinDirective = SMLoc();
   }
 
   bool parseDirectiveDesc(StringRef, SMLoc);
   bool parseDirectiveIndirectSymbol(StringRef, SMLoc);
   bool parseDirectiveDumpOrLoad(StringRef, SMLoc);
   bool parseDirectiveLsym(StringRef, SMLoc);
   bool parseDirectiveLinkerOption(StringRef, SMLoc);
   bool parseDirectiveSection(StringRef, SMLoc);
bool DarwinAsmParser::parseDirectiveDataRegionEnd(StringRef, SMLoc) {
   Lex();
   getStreamer().EmitDataRegion(MCDR_DataRegionEnd);
   return false;
 }
 
 /// parseVersionMin
 ///  ::= .ios_version_min major,minor[,update]
 ///  ::= .macosx_version_min major,minor[,update]
-bool DarwinAsmParser::parseVersionMin(StringRef Directive, SMLoc) {
+bool DarwinAsmParser::parseVersionMin(StringRef Directive, SMLoc Loc) {
   int64_t Major = 0, Minor = 0, Update = 0;
   int Kind = StringSwitch<int>(Directive)
     .Case(".ios_version_min", MCVM_IOSVersionMin)
     .Case(".macosx_version_min", MCVM_OSXVersionMin);
   // Get the major version number.
   if (getLexer().isNot(AsmToken::Integer))
     return TokError("invalid OS major version number");
   Major = getLexer().getTok().getIntVal();
if (getLexer().isNot(AsmToken::EndOfStatement)) {
     if (getLexer().isNot(AsmToken::Integer))
       return TokError("invalid OS update number");
     Update = getLexer().getTok().getIntVal();
   if (Update > 255 || Update < 0)
     return TokError("invalid OS update number");
     Lex();
   }
 
+  const Triple &T = getContext().getObjectFileInfo()->getTargetTriple();
+  Triple::OSType ExpectedOS = Triple::UnknownOS;
+  switch ((MCVersionMinType)Kind) {
+  case MCVM_IOSVersionMin: ExpectedOS = Triple::IOS;    break;
+  case MCVM_OSXVersionMin: ExpectedOS = Triple::MacOSX; break;
+  }
+  if (T.getOS() != ExpectedOS)
+    Warning(Loc, Directive + " should only be used for " +
+            Triple::getOSTypeName(ExpectedOS) + " targets");
+
+  if (LastVersionMinDirective.isValid()) {
+    Warning(Loc, "overriding previous version_min directive");
+    Note(LastVersionMinDirective, "previous definition is here");
+  }
+  LastVersionMinDirective = Loc;
+
   // We've parsed a correct version specifier, so send it to the streamer.
   getStreamer().EmitVersionMin((MCVersionMinType)Kind, Major, Minor, Update);
 
   return false;
 }
 
 namespace llvm {
 
 MCAsmParserExtension *createDarwinAsmParser() {
   return new DarwinAsmParser;
 }
 
 } // end llvm namespace

test/MC/MachO/ARM/version-min-diagnostics2.s

+// RUN: llvm-mc -triple thumbv7-apple-ios %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=IOS
+// RUN: llvm-mc -triple i386-apple-macosx %s 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=MACOSX
+
+.ios_version_min 1,2,3
+// MACOSX: version-min-diagnostics2.s:[[@LINE-1]]:1: warning: .ios_version_min should only be used for ios targets
+// IOS-NOT: warning: .ios_version_min should only be used for ios targets
+.macosx_version_min 4,5,6
+// IOS: version-min-diagnostics2.s:[[@LINE-1]]:1: warning: .macosx_version_min should only be used for macosx targets
+// MACOSX-NOT: warning: .macosx_version_min should only be used for macosx targets
+// CHECK: version-min-diagnostics2.s:[[@LINE-3]]:1: warning: overriding previous version_min directive
+// CHECK: version-min-diagnostics2.s:[[@LINE-7]]:1: note: previous definition is here