[Object] Search for architecures by name in MachOUniversalBinary::getObjectForArch()

[friss]

Summary

The reason we need to search by name rather than by Triple::ArchType
is to handle x86_64h correclty. There is no different ArchType
for the x86_64h architecture (it identifies itself as x86_64), and
this is by design. The only way to get to the x86_64h slice of an
universal binary is to serch by name.

This issue led to hard to debug and transient symbolication failures
in Asan tests (it mostly works, because the files are very similar).

This also affects the Profiling infrastucture as it is the other user
of that API.

include/llvm/Object/MachOUniversal.h

public:
   uint32_t getNumberOfObjects() const { return NumberOfObjects; }
 
   // Cast methods.
   static inline bool classof(Binary const *V) {
     return V->isMachOUniversalBinary();
   }
 
   ErrorOr<std::unique_ptr<MachOObjectFile>>
-  getObjectForArch(Triple::ArchType Arch) const;
+  getObjectForArch(StringRef ArchName) const;
 };
 
 } // namespace object
 } // namespace llvm
 
 #endif

include/llvm/ProfileData/CoverageMapping.h

public:
   /// \brief Load the coverage mapping using the given readers.
   static ErrorOr<std::unique_ptr<CoverageMapping>>
   load(CoverageMappingReader &CoverageReader,
        IndexedInstrProfReader &ProfileReader);
 
   /// \brief Load the coverage mapping from the given files.
   static ErrorOr<std::unique_ptr<CoverageMapping>>
   load(StringRef ObjectFilename, StringRef ProfileFilename,
-       Triple::ArchType Arch = Triple::ArchType::UnknownArch);
+       StringRef Arch = StringRef());
 
   /// \brief The number of functions that couldn't have their profiles mapped.
   ///
   /// This is a count of functions whose profile is out of date or otherwise
   /// can't be associated with any coverage information.
   unsigned getMismatchedCount() { return MismatchedFunctionCount; }
 
   /// \brief Returns the list of files that are covered.

include/llvm/ProfileData/CoverageMappingReader.h

private:
   BinaryCoverageReader(const BinaryCoverageReader &) = delete;
   BinaryCoverageReader &operator=(const BinaryCoverageReader &) = delete;
 
   BinaryCoverageReader() : CurrentRecord(0) {}
 
 public:
   static ErrorOr<std::unique_ptr<BinaryCoverageReader>>
   create(std::unique_ptr<MemoryBuffer> &ObjectBuffer,
-         Triple::ArchType Arch = Triple::ArchType::UnknownArch);
+         StringRef Arch);
 
   std::error_code readNextRecord(CoverageMappingRecord &Record) override;
 };
 
 } // end namespace coverage
 } // end namespace llvm
 
 #endif

lib/Object/MachOUniversal.cpp

uint32_t MinSize = sizeof(MachO::fat_header) +
                      sizeof(MachO::fat_arch) * NumberOfObjects;
   if (H.magic != MachO::FAT_MAGIC || Buf.size() < MinSize) {
     ec = object_error::parse_failed;
     return;
   }
   ec = std::error_code();
 }
 
-static bool getCTMForArch(Triple::ArchType Arch, MachO::CPUType &CTM) {
-  switch (Arch) {
-    case Triple::x86:    CTM = MachO::CPU_TYPE_I386; return true;
-    case Triple::x86_64: CTM = MachO::CPU_TYPE_X86_64; return true;
-    case Triple::arm:    CTM = MachO::CPU_TYPE_ARM; return true;
-    case Triple::sparc:  CTM = MachO::CPU_TYPE_SPARC; return true;
-    case Triple::ppc:    CTM = MachO::CPU_TYPE_POWERPC; return true;
-    case Triple::ppc64:  CTM = MachO::CPU_TYPE_POWERPC64; return true;
-    default: return false;
-  }
-}
-
 ErrorOr<std::unique_ptr<MachOObjectFile>>
-MachOUniversalBinary::getObjectForArch(Triple::ArchType Arch) const {
-  MachO::CPUType CTM;
-  if (!getCTMForArch(Arch, CTM))
-    return object_error::arch_not_found;
+MachOUniversalBinary::getObjectForArch(StringRef ArchName) const {
   for (object_iterator I = begin_objects(), E = end_objects(); I != E; ++I) {
-    if (I->getCPUType() == static_cast<uint32_t>(CTM))
+    if (I->getArchTypeName() == ArchName)
       return I->getAsObjectFile();
   }
   return object_error::arch_not_found;
 }

lib/ProfileData/CoverageMapping.cpp

for (const auto &Record : CoverageReader) {
     Coverage->Functions.push_back(std::move(Function));
   }
 
   return std::move(Coverage);
 }
 
 ErrorOr<std::unique_ptr<CoverageMapping>>
 CoverageMapping::load(StringRef ObjectFilename, StringRef ProfileFilename,
-                      Triple::ArchType Arch) {
+                      StringRef Arch) {
   auto CounterMappingBuff = MemoryBuffer::getFileOrSTDIN(ObjectFilename);
   if (std::error_code EC = CounterMappingBuff.getError())
     return EC;
   auto CoverageReaderOrErr =
       BinaryCoverageReader::create(CounterMappingBuff.get(), Arch);
   if (std::error_code EC = CoverageReaderOrErr.getError())
     return EC;
   auto CoverageReader = std::move(CoverageReaderOrErr.get());

lib/ProfileData/CoverageMappingReader.cpp

static ErrorOr<SectionRef> lookupSection(ObjectFile &OF, StringRef Name) {
   return coveragemap_error::no_data_found;
 }
 
 static std::error_code loadBinaryFormat(MemoryBufferRef ObjectBuffer,
                                         SectionData &ProfileNames,
                                         StringRef &CoverageMapping,
                                         uint8_t &BytesInAddress,
                                         support::endianness &Endian,
-                                        Triple::ArchType Arch) {
+                                        StringRef Arch) {
   auto BinOrErr = object::createBinary(ObjectBuffer);
   if (std::error_code EC = BinOrErr.getError())
     return EC;
   auto Bin = std::move(BinOrErr.get());
   std::unique_ptr<ObjectFile> OF;
   if (auto *Universal = dyn_cast<object::MachOUniversalBinary>(Bin.get())) {
     // If we have a universal binary, try to look up the object for the
     // appropriate architecture.
     auto ObjectFileOrErr = Universal->getObjectForArch(Arch);
     if (std::error_code EC = ObjectFileOrErr.getError())
       return EC;
     OF = std::move(ObjectFileOrErr.get());
   } else if (isa<object::ObjectFile>(Bin.get())) {
     // For any other object file, upcast and take ownership.
     OF.reset(cast<object::ObjectFile>(Bin.release()));
     // If we've asked for a particular arch, make sure they match.
-    if (Arch != Triple::ArchType::UnknownArch && OF->getArch() != Arch)
+    if (!Arch.empty() && OF->getArch() != Triple(Arch).getArch())
       return object_error::arch_not_found;
   } else
     // We can only handle object files.
     return coveragemap_error::malformed;
 
   // The coverage uses native pointer sizes for the object it's written in.
   BytesInAddress = OF->getBytesInAddress();
   Endian = OF->isLittleEndian() ? support::endianness::little
static std::error_code loadBinaryFormat(MemoryBufferRef ObjectBuffer,
   if (std::error_code EC = ProfileNames.load(*NamesSection))
     return EC;
 
   return std::error_code();
 }
 
 ErrorOr<std::unique_ptr<BinaryCoverageReader>>
 BinaryCoverageReader::create(std::unique_ptr<MemoryBuffer> &ObjectBuffer,
-                             Triple::ArchType Arch) {
+                             StringRef Arch) {
   std::unique_ptr<BinaryCoverageReader> Reader(new BinaryCoverageReader());
 
   SectionData Profile;
   StringRef Coverage;
   uint8_t BytesInAddress;
   support::endianness Endian;
   std::error_code EC;
   if (ObjectBuffer->getBuffer().startswith(TestingFormatMagic))

test/tools/llvm-symbolizer/Inputs/fat.x86_64h.c

+/* Compile with:
+   $ clang -arch x86_64 -arch x86_64h fat.x86_64h.c -c
+*/
+
+#ifdef __x86_64h__
+void x86_64h_function() {}
+#else
+void x86_64_function() {}
+#endif

test/tools/llvm-symbolizer/fat.test

+RUN: echo 0 | llvm-symbolizer -obj=%p/Inputs/fat.x86_64h.o -default-arch=x86_64 | FileCheck --check-prefix=X86_64 %s
+RUN: echo 0 | llvm-symbolizer -obj=%p/Inputs/fat.x86_64h.o -default-arch=x86_64h | FileCheck --check-prefix=X86_64H %s
+
+X86_64-NOT: x86_64h_function
+X86_64: x86_64_function
+
+X86_64H-NOT: x86_64_function
+X86_64H: x86_64h_function

tools/llvm-cov/CodeCoverage.cpp

public:
   CoverageViewOptions ViewOpts;
   std::string PGOFilename;
   CoverageFiltersMatchAll Filters;
   std::vector<std::string> SourceFiles;
   std::vector<std::pair<std::string, std::unique_ptr<MemoryBuffer>>>
       LoadedSourceFiles;
   bool CompareFilenamesOnly;
   StringMap<std::string> RemappedFilenames;
-  llvm::Triple::ArchType CoverageArch;
+  std::string CoverageArch;
 };
 }
 
 void CodeCoverageTool::error(const Twine &Message, StringRef Whence) {
   errs() << "error: ";
   if (!Whence.empty())
     errs() << Whence << ": ";
   errs() << Message << "\n";
if (RegionCoverageLtFilter.getNumOccurrences() ||
         StatFilterer->push_back(llvm::make_unique<LineCoverageFilter>(
             LineCoverageFilter::LessThan, LineCoverageLtFilter));
       if (LineCoverageGtFilter.getNumOccurrences())
         StatFilterer->push_back(llvm::make_unique<LineCoverageFilter>(
             RegionCoverageFilter::GreaterThan, LineCoverageGtFilter));
       Filters.push_back(std::unique_ptr<CoverageFilter>(StatFilterer));
     }
 
-    if (Arch.empty())
-      CoverageArch = llvm::Triple::ArchType::UnknownArch;
-    else {
-      CoverageArch = Triple(Arch).getArch();
-      if (CoverageArch == llvm::Triple::ArchType::UnknownArch) {
-        errs() << "error: Unknown architecture: " << Arch << "\n";
-        return 1;
-      }
-    }
+    if (!Arch.empty() &&
+        Triple(Arch).getArch() == llvm::Triple::ArchType::UnknownArch) {
+    CoverageArch = Arch;
 
     for (const auto &File : InputSourceFiles) {
       SmallString<128> Path(File);
       if (!CompareFilenamesOnly)
         if (std::error_code EC = sys::fs::make_absolute(Path)) {
           errs() << "error: " << File << ": " << EC.message();
           return 1;
         }

tools/llvm-symbolizer/LLVMSymbolize.cpp

if (!Bin)
     return nullptr;
   ObjectFile *Res = nullptr;
   if (MachOUniversalBinary *UB = dyn_cast<MachOUniversalBinary>(Bin)) {
     const auto &I = ObjectFileForArch.find(
         std::make_pair(UB, ArchName));
     if (I != ObjectFileForArch.end())
       return I->second;
     ErrorOr<std::unique_ptr<ObjectFile>> ParsedObj =
-        UB->getObjectForArch(Triple(ArchName).getArch());
+        UB->getObjectForArch(ArchName);
     if (ParsedObj) {
       Res = ParsedObj.get().get();
       ParsedBinariesAndObjects.push_back(std::move(ParsedObj.get()));
     }
     ObjectFileForArch[std::make_pair(UB, ArchName)] = Res;
   } else if (Bin->isObject()) {
     Res = cast<ObjectFile>(Bin);
   }