[llvm-nm] Fix output for Mach-O universal files wrapping bitcode objects

[alice]

Summary

Currently llvm-nm can look at a Mach-O universal binary and do one of a few generally unsurprising things (e.g. accept command line options to nm any selection of the contained architectures or all of them or if nothing is specified but the file contains a slice for the local machine’s own architecture then go with that); and obviously it can look at an LLVM bitcode file and do most of the same nm stuff other than have addresses for anything. Currently it can’t do both at once: if you’re targeting Darwin and you've configured some project to build with, say, -flto -arch x86_64 -arch i386, you'll end up with object files that are Mach-O universal binary wrappers containing LLVM bitcode for each architecture, and if you run llvm-nm on one of those, it will tend to silently exit 0. This patch fixes this, and adds two lit tests, one confirming its existing sensible output for -arch=all on Mach-O universal files that consist of native objects rather than bitcode, and one which shows that it currently fails on a universal file containing i386 bitcode and x86_64 bitcode but passes with the code changes in this patch applied.

[alice]

I’ve noticed this causing issues in e.g. some autotools/configure-based packages I’ve tried configuring to build with LTO and as universal binaries, some of them will invoke a macro that has it go "checking command to parse /.../nm output from /.../cc object… failed” and the build will indeed fail in some cases as a result.

(ALSO! Hi, this is my first attempt submitting a thing to LLVM and so I’ve tried to be painstaking about following the guidelines and policies and coding style conventions and such, but if I’ve missed anything, I’d be grateful to find out, down to and including (in all seriousness) the most minor and nitpicky of inconsequential details... I like to do things right)

[enderby]

Looks fine to me.

There is a whole bunch of better error handling that still needs to happen, I’m grinding through adding all of that, to llvm-nm and other tools. But what you have fixes the problem you point out with bitcode files in universal files. And is consistant with the code that is there today.

Kev

include/llvm/Object/MachOUniversal.h

 
 #ifndef LLVM_OBJECT_MACHOUNIVERSAL_H
 #define LLVM_OBJECT_MACHOUNIVERSAL_H
 
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Binary.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MachO.h"
 
 namespace llvm {
 class StringRef;
 
 namespace object {
public:
     std::string getArchTypeName() const {
       Triple T =
           MachOObjectFile::getArchTriple(Header.cputype, Header.cpusubtype);
       return T.getArchName();
     }
 
     ErrorOr<std::unique_ptr<MachOObjectFile>> getAsObjectFile() const;
 
+    ErrorOr<std::unique_ptr<IRObjectFile>>
+    getAsIRObjectFile(LLVMContext *Context) const;
+
     ErrorOr<std::unique_ptr<Archive>> getAsArchive() const;
   };
 
   class object_iterator {
     ObjectForArch Obj;
   public:
     object_iterator(const ObjectForArch &Obj) : Obj(Obj) {}
     const ObjectForArch *operator->() const { return &Obj; }

lib/Object/MachOUniversal.cpp

MachOUniversalBinary::ObjectForArch::getAsObjectFile() const {
 
   StringRef ParentData = Parent->getData();
   StringRef ObjectData = ParentData.substr(Header.offset, Header.size);
   StringRef ObjectName = Parent->getFileName();
   MemoryBufferRef ObjBuffer(ObjectData, ObjectName);
   return expectedToErrorOr(ObjectFile::createMachOObjectFile(ObjBuffer));
 }
 
+ErrorOr<std::unique_ptr<IRObjectFile>>
+MachOUniversalBinary::ObjectForArch::getAsIRObjectFile(LLVMContext *Context)
+const {
+  if (!Parent)
+    return object_error::parse_failed;
+  if (!Context)
+    return object_error::invalid_file_type;
+
+  StringRef ParentData = Parent->getData();
+  StringRef ObjectData = ParentData.substr(Header.offset, Header.size);
+  StringRef ObjectName = Parent->getFileName();
+  MemoryBufferRef ObjBuffer(ObjectData, ObjectName);
+
+  return IRObjectFile::create(ObjBuffer, *Context);
+}
+
 ErrorOr<std::unique_ptr<Archive>>
 MachOUniversalBinary::ObjectForArch::getAsArchive() const {
   if (!Parent)
     return object_error::parse_failed;
 
   StringRef ParentData = Parent->getData();
   StringRef ObjectData = ParentData.substr(Header.offset, Header.size);
   StringRef ObjectName = Parent->getFileName();

test/tools/llvm-nm/X86/machOUnivIR.test

+# RUN: llvm-nm -m -arch all %p/Inputs/test.macho-univ.IRobj-x86_64-i386 | FileCheck %s
+
+# CHECK: Inputs/test.macho-univ.IRobj-x86_64-i386 (for architecture x86_64):
+# CHECK: ---------------- (LTO,RODATA) external _global_const
+# CHECK: ---------------- (LTO,DATA) external _global_data
+# CHECK: ---------------- (LTO,CODE) external _global_func
+# CHECK: ---------------- (LTO,RODATA) private external _hidden_const
+# CHECK: ---------------- (LTO,DATA) private external _hidden_data
+# CHECK: ---------------- (LTO,CODE) private external _hidden_func
+# CHECK: ---------------- (LTO,RODATA) non-external _static_const
+# CHECK: ---------------- (LTO,DATA) non-external _static_data
+# CHECK: ---------------- (LTO,CODE) non-external _static_func
+
+# CHECK: Inputs/test.macho-univ.IRobj-x86_64-i386 (for architecture i386):
+# CHECK: -------- (LTO,RODATA) external _global_const
+# CHECK: -------- (LTO,DATA) external _global_data
+# CHECK: -------- (LTO,CODE) external _global_func
+# CHECK: -------- (LTO,RODATA) private external _hidden_const
+# CHECK: -------- (LTO,DATA) private external _hidden_data
+# CHECK: -------- (LTO,CODE) private external _hidden_func
+# CHECK: -------- (LTO,RODATA) non-external _static_const
+# CHECK: -------- (LTO,DATA) non-external _static_data
+# CHECK: -------- (LTO,CODE) non-external _static_func

test/tools/llvm-nm/X86/machOUnivNative.test

+# RUN: llvm-nm -m -arch all %p/Inputs/test.macho-univ.x86_64-i386 | FileCheck %s
+
+# CHECK: Inputs/test.macho-univ.x86_64-i386 (for architecture x86_64):
+# CHECK: 0000000000000046 (__TEXT,__const) external _global_const
+# CHECK: 0000000000000054 (__DATA,__data) external _global_data
+# CHECK: 0000000000000000 (__TEXT,__text) external _global_func
+# CHECK: 000000000000004a (__TEXT,__const) private external _hidden_const
+# CHECK: 0000000000000058 (__DATA,__data) private external _hidden_data
+# CHECK: 0000000000000010 (__TEXT,__text) private external _hidden_func
+# CHECK: 000000000000004e (__TEXT,__const) non-external _static_const
+# CHECK: 000000000000005c (__DATA,__data) non-external _static_data
+# CHECK: 0000000000000040 (__TEXT,__text) non-external _static_func
+
+# CHECK: Inputs/test.macho-univ.x86_64-i386 (for architecture i386):
+# CHECK: 00000035 (__TEXT,__const) external _global_const
+# CHECK: 00000044 (__DATA,__data) external _global_data
+# CHECK: 00000000 (__TEXT,__text) external _global_func
+# CHECK: 00000039 (__TEXT,__const) private external _hidden_const
+# CHECK: 00000048 (__DATA,__data) private external _hidden_data
+# CHECK: 00000010 (__TEXT,__text) private external _hidden_func
+# CHECK: 0000003d (__TEXT,__const) non-external _static_const
+# CHECK: 0000004c (__DATA,__data) non-external _static_data
+# CHECK: 00000030 (__TEXT,__text) non-external _static_func

tools/llvm-nm/llvm-nm.cpp

if (!ArchAll && ArchFlags.size() != 0) {
                              << ")";
                     }
                     outs() << ":\n";
                   }
                   dumpSymbolNamesFromObject(*O, false, ArchiveName,
                                             ArchitectureName);
                 }
               }
+            } else {
+              ErrorOr<std::unique_ptr<IRObjectFile>> IRObjOrErr =
+                I->getAsIRObjectFile(NoLLVMBitcode ? nullptr : &Context);
+              if (error(IRObjOrErr.getError(), Filename))
+                return;
+              IRObjectFile &IR = *IRObjOrErr.get();
+              if (ArchFlags.size() > 1) {
+                if (PrintFileName)
+                  ArchitectureName = I->getArchTypeName();
+                else
+                  outs() << "\n" << IR.getFileName() << " (for architecture "
+                         << I->getArchTypeName() << ")"
+                         << ":\n";
+              }
+              dumpSymbolNamesFromObject(IR, false, ArchiveName,
+                                        ArchitectureName);
             }
           }
         }
         if (!ArchFound) {
           error(ArchFlags[i],
                 "file: " + Filename + " does not contain architecture");
           return;
         }
if (!ArchAll) {
                   ArchiveName = A->getFileName();
                 else
                   outs() << "\n" << A->getFileName() << "(" << O->getFileName()
                          << ")"
                          << ":\n";
                 dumpSymbolNamesFromObject(*O, false, ArchiveName);
               }
             }
+          } else {
+            ErrorOr<std::unique_ptr<IRObjectFile>> IRObjOrErr =
+              I->getAsIRObjectFile(NoLLVMBitcode ? nullptr : &Context);
+            if (error(IRObjOrErr.getError(), Filename))
+              return;
+            IRObjectFile &IR = *IRObjOrErr.get();
+            dumpSymbolNamesFromObject(IR, false);
           }
           return;
         }
       }
     }
     // Either all architectures have been specified or none have been specified
     // and this does not contain the host architecture so dump all the slices.
     bool moreThanOneArch = UB->getNumberOfObjects() > 1;
for (MachOUniversalBinary::object_iterator I = UB->begin_objects(),
                          << ")";
               } else
                 outs() << ":" << O->getFileName();
               outs() << ":\n";
             }
             dumpSymbolNamesFromObject(*O, false, ArchiveName, ArchitectureName);
           }
         }
+      } else {
+        ErrorOr<std::unique_ptr<IRObjectFile>> IRObjOrErr =
+          I->getAsIRObjectFile(NoLLVMBitcode ? nullptr : &Context);
+        if (error(IRObjOrErr.getError(), Filename))
+          return;
+        IRObjectFile &IR = *IRObjOrErr.get();
+        if (PrintFileName) {
+          if (moreThanOneArch)
+            ArchitectureName = I->getArchTypeName();
+        } else {
+          if (moreThanOneArch)
+            outs() << "\n";
+          outs() << IR.getFileName();
+          if (moreThanOneArch)
+            outs() << " (for architecture " << I->getArchTypeName() << ")";
+          outs() << ":\n";
+        }
+        dumpSymbolNamesFromObject(IR, false, ArchiveName, ArchitectureName);
       }
     }
     return;
   }
   if (SymbolicFile *O = dyn_cast<SymbolicFile>(&Bin)) {
     if (!checkMachOAndArchFlags(O, Filename))
       return;
     dumpSymbolNamesFromObject(*O, true);