[ARM] Fix crash "-target arm -mcpu=generic", without "-march="

[vsukharev]

Summary

Follow-up to http://reviews.llvm.org/rL245445
Added more tests for "-mcpu=generic", a crash fixed.

[rengolin]

Hi Vladmir,

Can you explain what the crash was?

cheers,
--renato

[vsukharev]

Yes, trying to pass new tests in test/Driver/arm-cortex-cpus.c, without code patch, it crashes

$ clang --target=arm-linux-gnueabi -mcpu=generic hello.c
/work/oss_llvm/llvm/include/llvm/ADT/StringRef.h:84: llvm::StringRef::StringRef(const char*, size_t): Assertion `(data || length == 0) && "StringRef cannot be built from a NULL argument with non-null length"' failed.
...
#9 0x2b22f25 (anonymous namespace)::._115::getSubArch() const lib/Support/TargetParser.cpp:74:0
#10 0x2b27147 llvm::ARM::getSubArch(unsigned int) lib/Support/TargetParser.cpp:309:0
#11 0x31871a1 clang::driver::tools::arm::getLLVMArchSuffixForARM(llvm::StringRef, llvm::StringRef) tools/clang/lib/Driver/Tools.cpp:6374:0
#12 0x3125da8 clang::driver::ToolChain::ComputeLLVMTriple(llvm::opt::ArgList const&, clang::driver::types::ID) const tools/clang/lib/Driver/ToolChain.cpp:321:0
#13 0x312656c clang::driver::ToolChain::ComputeEffectiveClangTriple(llvm::opt::ArgList const&, clang::driver::types::ID) const tools/clang/lib/Driver/ToolChain.cpp:353:0
#14 0x31726b1 clang::driver::tools::Clang::ConstructJob(clang::driver::Compilation&, clang::driver::JobAction const&, clang::driver::InputInfo const&, llvm::SmallVector<clang::driver::InputInfo, 4u> const&, llvm::opt::ArgList const&, char const*) const tools/clang/lib/Driver/Tools.cpp:3363:0

[rengolin]

Apart from the style nitpick, LGTM. Thanks!

lib/Driver/Tools.cpp

6139 ↗

(On Diff #35234)

Please, add curly brackets here for the else case.

cfe/trunk/lib/Driver/ToolChain.cpp

if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
       MCPU = A->getValue();
     if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
       MArch = A->getValue();
     std::string CPU =
         Triple.isOSBinFormatMachO()
             ? tools::arm::getARMCPUForMArch(MArch, Triple).str()
             : tools::arm::getARMTargetCPU(MCPU, MArch, Triple);
     StringRef Suffix =
-      tools::arm::getLLVMArchSuffixForARM(CPU,
-                                          tools::arm::getARMArch(MArch, Triple));
+      tools::arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
     bool ThumbDefault = Suffix.startswith("v6m") || Suffix.startswith("v7m") ||
       Suffix.startswith("v7em") ||
       (Suffix.startswith("v7") && getTriple().isOSBinFormatMachO());
     // FIXME: this is invalid for WindowsCE
     if (getTriple().isOSWindows())
       ThumbDefault = true;
     std::string ArchName;
     if (IsBigEndian)

cfe/trunk/lib/Driver/Tools.h

 } // end namespace amdgpu
 
 namespace arm {
 std::string getARMTargetCPU(StringRef CPU, StringRef Arch,
                             const llvm::Triple &Triple);
 const std::string getARMArch(StringRef Arch,
                              const llvm::Triple &Triple);
 StringRef getARMCPUForMArch(StringRef Arch, const llvm::Triple &Triple);
-StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch);
+StringRef getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+                                  const llvm::Triple &Triple);
 
 void appendEBLinkFlags(const llvm::opt::ArgList &Args, ArgStringList &CmdArgs,
                        const llvm::Triple &Triple);
 }
 
 namespace mips {
 typedef enum { NanLegacy = 1, Nan2008 = 2 } NanEncoding;
 

cfe/trunk/lib/Driver/Tools.cpp

if (llvm::ARM::parseArch(MArch) == llvm::ARM::AK_INVALID)
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
 // Check -mcpu=. Needs ArchName to handle -mcpu=generic.
 static void checkARMCPUName(const Driver &D, const Arg *A, const ArgList &Args,
                             llvm::StringRef CPUName, llvm::StringRef ArchName,
                             const llvm::Triple &Triple) {
   std::string CPU = arm::getARMTargetCPU(CPUName, ArchName, Triple);
-  std::string Arch = arm::getARMArch(ArchName, Triple);
-  if (arm::getLLVMArchSuffixForARM(CPU, Arch).empty())
+  if (arm::getLLVMArchSuffixForARM(CPU, ArchName, Triple).empty())
     D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 }
 
 static bool useAAPCSForMachO(const llvm::Triple &T) {
   // The backend is hardwired to assume AAPCS for M-class processors, ensure
   // the frontend matches that.
   return T.getEnvironment() == llvm::Triple::EABI ||
          T.getOS() == llvm::Triple::UnknownOS || isARMMProfile(T);
else
     MArch = Triple.getArchName();
   MArch = StringRef(MArch).lower();
 
   // Handle -march=native.
   if (MArch == "native") {
     std::string CPU = llvm::sys::getHostCPUName();
     if (CPU != "generic") {
       // Translate the native cpu into the architecture suffix for that CPU.
-      StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch);
+      StringRef Suffix = arm::getLLVMArchSuffixForARM(CPU, MArch, Triple);
       // If there is no valid architecture suffix for this CPU we don't know how
       // to handle it, so return no architecture.
       if (Suffix.empty())
         MArch = "";
       else
         MArch = std::string("arm") + Suffix.str();
     }
   }
std::string arm::getARMTargetCPU(StringRef CPU, StringRef Arch,
   }
 
   return getARMCPUForMArch(Arch, Triple);
 }
 
 /// getLLVMArchSuffixForARM - Get the LLVM arch name to use for a particular
 /// CPU  (or Arch, if CPU is generic).
 // FIXME: This is redundant with -mcpu, why does LLVM use this.
-StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch) {
-  if (CPU == "generic")
-    return llvm::ARM::getSubArch(
-        llvm::ARM::parseArch(Arch));
-
-  unsigned ArchKind = llvm::ARM::parseCPUArch(CPU);
+StringRef arm::getLLVMArchSuffixForARM(StringRef CPU, StringRef Arch,
+                                       const llvm::Triple &Triple) {
+  unsigned ArchKind;
+  Arch = tools::arm::getARMArch(Arch, Triple);
+  if (CPU == "generic") {
+    ArchKind = llvm::ARM::parseArch(Arch);
+    if (ArchKind == llvm::ARM::AK_INVALID)
+      // In case of generic Arch, i.e. "arm",
+      // extract arch from default cpu of the Triple
+      ArchKind = llvm::ARM::parseCPUArch(Triple.getARMCPUForArch(Arch));
+  } else {
+    ArchKind = llvm::ARM::parseCPUArch(CPU);
+  }
   if (ArchKind == llvm::ARM::AK_INVALID)
     return "";
   return llvm::ARM::getSubArch(ArchKind);
 }
 
 void arm::appendEBLinkFlags(const ArgList &Args, ArgStringList &CmdArgs,
                             const llvm::Triple &Triple) {
   if (Args.hasArg(options::OPT_r))

cfe/trunk/test/Driver/aarch64-cpus.c

 // Check target CPUs are correctly passed.
 
 // RUN: %clang -target aarch64 -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
+// RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
+// RUN: %clang -target arm64 -mlittle-endian -mcpu-generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 
 // ARM64-GENERIC: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64-apple-darwin -arch arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-DARWIN %s
 // ARM64-DARWIN: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cyclone"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s

cfe/trunk/test/Driver/arm-cortex-cpus.c

 // ================== Check default CPU on each major architecture
+// RUN: %clang -target arm -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC %s
+// CHECK-GENERIC: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target armeb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC %s
+// CHECK-BE-GENERIC: "-cc1"{{.*}} "-triple" "armebv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target arm -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-GENERIC-THUMB %s
+// CHECK-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbv4t-{{.*}} "-target-cpu" "generic"
+
+// RUN: %clang -target armeb -mthumb -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-GENERIC-THUMB %s
+// CHECK-BE-GENERIC-THUMB: "-cc1"{{.*}} "-triple" "thumbebv4t-{{.*}} "-target-cpu" "generic"
+
 // RUN: %clang -target armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s
 // RUN: %clang -target arm -march=armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T %s
 // CHECK-V4T: "-cc1"{{.*}} "-triple" "armv4t-{{.*}} "-target-cpu" "arm7tdmi"
 
 // RUN: %clang -target armv4t -mthumb -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T-THUMB %s
 // RUN: %clang -target arm -mthumb -march=armv4t -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V4T-THUMB %s
 // CHECK-V4T-THUMB: "-cc1"{{.*}} "-triple" "thumbv4t-{{.*}} "-target-cpu" "arm7tdmi"