[lld-macho] implement options -map

[woshiccm]

Summary

Implement command-line options -map

[thakis]

Thanks for the patch!

Overall, looks pretty good.

Please clang-format the file (make the comments in the first few lines <= 80 cls first).

Please add a test.

Does ld64 print CommonSymbols, or does this run late enough that they've already been replaced by Defineds? (A good thing to include in the test.)

lld/MachO doesn't use TimeTraceScope anywhere else yet (it should!), so it's a bit weird to have it here. Maybe should add actual time trace support in a patch before this? But I suppose that part is fine as-is as well.

[woshiccm]

Fix MapFile format

[woshiccm]

Fix MapFile.cpp format

[woshiccm]

Fix Writer.cpp format

[woshiccm]

In D98323#2616678, @thakis wrote:

Thanks for the patch!

Overall, looks pretty good.

Please clang-format the file (make the comments in the first few lines <= 80 cls first).

Please add a test.

Does ld64 print CommonSymbols, or does this run late enough that they've already been replaced by Defineds? (A good thing to include in the test.)

lld/MachO doesn't use TimeTraceScope anywhere else yet (it should!), so it's a bit weird to have it here. Maybe should add actual time trace support in a patch before this? But I suppose that part is fine as-is as well.

The format fixed. Yes, CommonSymbols have already been replaced by Defineds.
Could we merge this patch first, and then I add test? I am not familiar with writing tests yet, it may take some time.

[thakis]

New code should be covered by a test added in the same change. See https://llvm.org/docs/TestingGuide.html#writing-new-regression-tests and https://llvm.org/docs/CommandGuide/FileCheck.html#tutorial -- and look at the existing tests in lld/test/MachO for many examples, of course :)

[int3]

Yeah diffs should generally come with corresponding tests. This code is fairly self-contained, so there's little chance of rebase conflicts and no rush to land :)

You can run the existing tests under lld/macho via llvm-lit -vva <filename>. -vva will let you see what commands are being run to get a sense of how things work.

lld/MachO/MapFile.cpp

10

62–71

this is fairly similar to SymtabSection::finalizeContents, with some minor differences. We should check if the logic can be factored out. I can help with that though :) could you leave a TODO for now?

66

we should check for sym != nullptr in this loop (we don't parse symbols in debug sections, so they will be null)

also JFYI, lld::macho:: won't be necessary once D98149 lands

74–75

this function isn't using parallel-for :) leave a TODO?

85

llvm/TextAPI/MachO/Architecture.h defines getArchitectureName()

[woshiccm]

Fix issues

[woshiccm]

Fix issues

[woshiccm]

Add map-file.s

[woshiccm]

In D98323#2619689, @thakis wrote:

New code should be covered by a test added in the same change. See https://llvm.org/docs/TestingGuide.html#writing-new-regression-tests and https://llvm.org/docs/CommandGuide/FileCheck.html#tutorial -- and look at the existing tests in lld/test/MachO for many examples, of course :)

I added map-file.s for test

lld/MachO/MapFile.cpp

74–75

done

85

done

[woshiccm]

In D98323#2619894, @int3 wrote:

Yeah diffs should generally come with corresponding tests. This code is fairly self-contained, so there's little chance of rebase conflicts and no rush to land :)

You can run the existing tests under lld/macho via llvm-lit -vva <filename>. -vva will let you see what commands are being run to get a sense of how things work.

I added map-file.s for test

[int3]

lld/MachO/Writer.cpp

939

I think writeMapFile() should be moved below this line -- there's no need to emit the mapfile if we can't emit the binary itself

lld/test/MachO/map-file.s

7

The test seems to be failing, I guess it should be a -map without the =?

also I think it would be nicer to have the output go to %t/map

15

we should test common symbols (.comm) as well, as @thakis mentioned in an earlier comment

29–30

We try not to match against hardcoded addresses because they're a bit of a pain to update. In this case I think it would be better to match this against llvm-objdump's output: something like

RUN: llvm-objdump --syms --section-headers %t/test-map > %t/objdump
RUN: cat %t/objdump %t/map > %t/out
RUN: FileCheck %s < %t/out

Then we can use numeric substitutions to check that the values line up.

[woshiccm]

Fix map-file.s

[int3]

almost there!

lld/test/MachO/map-file.s

7

could you move the other files under %t/ as well, i.e. %t/foo.o and %t/test.o?

29–30

this still needs to be addressed

[woshiccm]

Fix map-file.s

[int3]

lld/test/MachO/map-file.s

29–30

So right now the test is just ignoring the value, but it would be better if it checked that it matched the actual addresses in the binary using numeric substitutions. It would be something like this:

# RUN: llvm-objdump --syms --section-headers %t/test-map > %t/objdump
# RUN: cat %t/objdump %t/map > %t/out
# RUN: FileCheck %s < %t/out
...
# CHECK:      Sections:
# CHECK-NEXT: Idx  Name          Size           VMA          Type
# CHECK-NEXT:   0  __text        {{[0-9a-f]+}} [[#%x,TEXT:]] TEXT
...
# CHECK-NEXT: # Sections:
# CHECK-NEXT: # Address  Size      Segment  Section
# CHECK-NEXT: 0x[[#TEXT]]  0x00000001  __TEXT  __text

[woshiccm]

Fix map-file.s

[int3]

lld/test/MachO/map-file.s

31–33

you're re-defining the TEXT/BSS/DATA symbols here (which were already defined on lines 25-27). I know they're the same values since these symbols are at the beginnings of their sections, but I think it would be nicer to define separate names for the symbols (e.g. MAIN, NUMBER, FOO).

[woshiccm]

Fix map-file.s

[woshiccm]

Fix map-file.s

[woshiccm]

Fix map-fils.s

[woshiccm]

Fix map-file.s

[oontvoo]

lld/MachO/Driver.cpp

829

maybe check and warn if the path is empty?

[int3]

lld/MachO/Driver.cpp

829

The empty check is already done in writeMapFile(). And I think the option parser will reject something like -map without an argument, so we don't need to warn (but I haven't verified this)

[woshiccm]

Rebase diff

lld/MachO/CMakeLists.txt

add_lld_library(lldMachO2
   ObjC.cpp
   OutputSection.cpp
   OutputSegment.cpp
   Relocations.cpp
   SymbolTable.cpp
   Symbols.cpp
   SyntheticSections.cpp
   Target.cpp
+  MapFile.cpp
   Writer.cpp
 
   LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   BinaryFormat
   BitReader
   Core
   DebugInfoDWARF

lld/MachO/Config.h

struct Configuration {
   bool adhocCodesign = false;
   bool timeTraceEnabled = false;
   uint32_t headerPad;
   uint32_t dylibCompatibilityVersion = 0;
   uint32_t dylibCurrentVersion = 0;
   uint32_t timeTraceGranularity;
   std::string progName;
   llvm::StringRef installName;
+  llvm::StringRef mapFile;
   llvm::StringRef outputFile;
   llvm::StringRef ltoObjPath;
   bool demangle = false;
   llvm::MachO::Target target;
   PlatformInfo platformInfo;
   NamespaceKind namespaceKind = NamespaceKind::twolevel;
   UndefinedSymbolTreatment undefinedSymbolTreatment =
       UndefinedSymbolTreatment::error;

lld/MachO/Driver.cpp

bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   for (const Arg *arg : args.filtered(OPT_u)) {
     config->explicitUndefineds.push_back(symtab->addUndefined(
         arg->getValue(), /*file=*/nullptr, /*isWeakRef=*/false));
   }
 
   for (const Arg *arg : args.filtered(OPT_U))
     symtab->addDynamicLookup(arg->getValue());
 
+  config->mapFile = args.getLastArgValue(OPT_map);
   config->outputFile = args.getLastArgValue(OPT_o, "a.out");
   config->headerPad = args::getHex(args, OPT_headerpad, /*Default=*/32);
   config->headerPadMaxInstallNames =
       args.hasArg(OPT_headerpad_max_install_names);
   config->printEachFile = args.hasArg(OPT_t);
   config->printWhyLoad = args.hasArg(OPT_why_load);
   config->outputType = getOutputType(args);
   if (const Arg *arg = args.getLastArg(OPT_bundle_loader)) {

lld/MachO/MapFile.h

+//===- MapFile.h ------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLD_MACHO_MAPFILE_H
+#define LLD_MACHO_MAPFILE_H
+
+namespace lld {
+namespace macho {
+void writeMapFile();
+} // namespace macho
+} // namespace lld
+
+#endif

lld/MachO/MapFile.cpp

+//===- MapFile.cpp --------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the -map option. It shows lists in order and
+// hierarchically the outputFile, arch, input files, output sections and
+// symbol:
+//
+// # Path: test
+// # Arch: x86_84
+// # Object files:
+// [  0] linker synthesized
+// [  1] a.o
+// # Sections:
+// # Address  Size      Segment  Section
+// 0x1000005C0  0x0000004C  __TEXT  __text
+// # Symbols:
+// # Address  File  Name
+// 0x1000005C0  [  1] _main
+//
+//===----------------------------------------------------------------------===//
+
+#include "MapFile.h"
+#include "Config.h"
+#include "InputFiles.h"
+#include "InputSection.h"
+#include "OutputSection.h"
+#include "OutputSegment.h"
+#include "Symbols.h"
+#include "Target.h"
+#include "llvm/Support/Parallel.h"
+
+using namespace llvm;
+using namespace llvm::sys;
+using namespace lld;
+using namespace lld::macho;
+
+using SymbolMapTy = DenseMap<const InputSection *, SmallVector<Defined *, 4>>;
+
+// Returns a map from sections to their symbols.
+static SymbolMapTy getSectionSyms(ArrayRef<Defined *> syms) {
+  SymbolMapTy ret;
+  for (Defined *dr : syms)
+    ret[dr->isec].push_back(dr);
+
+  // Sort symbols by address. We want to print out symbols in the
+  // order in the output file rather than the order they appeared
+  // in the input files.
+  for (auto &it : ret)
+    llvm::stable_sort(it.second, [](Defined *a, Defined *b) {
+      return a->getVA() < b->getVA();
+    });
+  return ret;
+}
+
+// Returns a list of all symbols that we want to print out.
+static std::vector<Defined *> getSymbols() {
+  std::vector<Defined *> v;
+  for (InputFile *file : inputFiles)
+    if (isa<ObjFile>(file))
+      for (Symbol *sym : file->symbols) {
+        if (sym == nullptr)
+          continue;
+        if (auto *d = dyn_cast<Defined>(sym))
+          if (d->isec && d->getFile() == file)
+            v.push_back(d);
+      }
+  return v;
+}
+
+// Construct a map from symbols to their stringified representations.
+// Demangling symbols (which is what toString() does) is slow, so
+// we do that in batch using parallel-for.
+static DenseMap<macho::Symbol *, std::string>
+getSymbolStrings(ArrayRef<Defined *> syms) {
+  std::vector<std::string> str(syms.size());
+  parallelForEachN(0, syms.size(), [&](size_t i) {
+    raw_string_ostream os(str[i]);
+    os << toString(*syms[i]);
+  });
+
+  DenseMap<macho::Symbol *, std::string> ret;
+  for (size_t i = 0, e = syms.size(); i < e; ++i)
+    ret[syms[i]] = std::move(str[i]);
+  return ret;
+}
+
+void macho::writeMapFile() {
+  if (config->mapFile.empty())
+    return;
+
+  // Open a map file for writing.
+  std::error_code ec;
+  raw_fd_ostream os(config->mapFile, ec, sys::fs::OF_None);
+  if (ec) {
+    error("cannot open " + config->mapFile + ": " + ec.message());
+    return;
+  }
+
+  // Dump output path
+  os << format("# Path: %s\n", config->outputFile.str().c_str());
+
+  // Dump output architecure
+  os << format("# Arch: %s\n",
+               getArchitectureName(config->target.Arch).str().c_str());
+
+  // Dump table of object files
+  os << "# Object files:\n";
+  os << format("[%3u] %s\n", 0, (const char *)"linker synthesized");
+  uint32_t fileIndex = 1;
+  DenseMap<lld::macho::InputFile *, uint32_t> readerToFileOrdinal;
+  for (InputFile *file : inputFiles) {
+    if (isa<ObjFile>(file)) {
+      os << format("[%3u] %s\n", fileIndex, file->getName().str().c_str());
+      readerToFileOrdinal[file] = fileIndex++;
+    }
+  }
+
+  // Collect symbol info that we want to print out.
+  std::vector<Defined *> syms = getSymbols();
+  SymbolMapTy sectionSyms = getSectionSyms(syms);
+  DenseMap<lld::macho::Symbol *, std::string> symStr = getSymbolStrings(syms);
+
+  // Dump table of sections
+  os << "# Sections:\n";
+  os << "# Address\tSize    \tSegment\tSection\n";
+  for (OutputSegment *seg : outputSegments)
+    for (OutputSection *osec : seg->getSections()) {
+      if (osec->isHidden())
+        continue;
+
+      os << format("0x%08llX\t0x%08llX\t%s\t%s\n", osec->addr, osec->getSize(),
+                   seg->name.str().c_str(), osec->name.str().c_str());
+    }
+
+  // Dump table of symbols
+  os << "# Symbols:\n";
+  os << "# Address\t    File  Name\n";
+  for (InputSection *isec : inputSections) {
+    for (macho::Symbol *sym : sectionSyms[isec]) {
+      os << format("0x%08llX\t[%3u] %s\n", sym->getVA(),
+                   readerToFileOrdinal[sym->getFile()], symStr[sym].c_str());
+    }
+  }
+
+  // TODO: when we implement -dead_strip, we should dump dead stripped symbols
+}

lld/MachO/Options.td

def whatsloaded : Flag<["-"], "whatsloaded">,
      Group<grp_introspect>;
 def order_file_statistics : Flag<["-"], "order_file_statistics">,
      HelpText<"Logs information about -order_file">,
      Flags<[HelpHidden]>,
      Group<grp_introspect>;
 def map : Separate<["-"], "map">,
      MetaVarName<"<path>">,
      HelpText<"Writes all symbols and their addresses to <path>">,
-     Flags<[HelpHidden]>,
      Group<grp_introspect>;
 def dependency_info : Separate<["-"], "dependency_info">,
      MetaVarName<"<path>">,
      HelpText<"Dump dependency info">,
      Flags<[HelpHidden]>,
      Group<grp_introspect>;
 def save_temps : Flag<["-"], "save-temps">,
      HelpText<"Save intermediate LTO compilation results">,

lld/MachO/Writer.cpp

 //===- Writer.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Writer.h"
 #include "Config.h"
 #include "InputFiles.h"
 #include "InputSection.h"
+#include "MapFile.h"
 #include "MergedOutputSection.h"
 #include "OutputSection.h"
 #include "OutputSegment.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
 #include "Target.h"
 #include "UnwindInfoSection.h"
if (in.stubHelper->isNeeded())
     in.stubHelper->setup();
   scanSymbols();
   createOutputSections();
   // No more sections nor segments are created beyond this point.
   sortSegmentsAndSections();
   createLoadCommands();
   finalizeAddressses();
   finalizeLinkEditSegment();
+  writeMapFile();
   openFile();
   if (errorCount())
     return;
   writeSections();
   writeUuid();
   writeCodeSignature();
 
   if (auto e = buffer->commit())
     error("failed to write to the output file: " + toString(std::move(e)));
 }
 
 void macho::writeResult() { Writer().run(); }
 
 void macho::createSyntheticSections() {
   in.header = make<MachHeaderSection>();
   in.rebase = make<RebaseSection>();
   in.binding = make<BindingSection>();

lld/test/MachO/map-file.s

+# REQUIRES: x86
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+
+# RUN: %lld -map %t/map %t/test.o %t/foo.o -o %t/test-map
+# RUN: llvm-objdump --syms --section-headers %t/test-map > %t/objdump
+# RUN: cat %t/objdump %t/map > %t/out
+# RUN: FileCheck %s < %t/out
+
+#--- foo.s
+.section __TEXT,obj
+.globl _foo
+_foo:
+
+#--- test.s
+.comm _number, 1
+.globl _main
+_main:
+  ret
+
+# CHECK:      Sections:
+# CHECK-NEXT: Idx  Name          Size           VMA           Type
+# CHECK-NEXT: 0    __text        {{[0-9a-f]+}}  [[#%x,TEXT:]] TEXT
+# CHECK-NEXT: 1    obj           {{[0-9a-f]+}}  [[#%x,DATA:]] DATA
+# CHECK-NEXT: 2    __common      {{[0-9a-f]+}}  [[#%x,BSS:]]  BSS
+
+# CHECK: SYMBOL TABLE:
+# CHECK-NEXT: [[#%x,MAIN:]]   g     F __TEXT,__text _main
+# CHECK-NEXT: [[#%x,NUMBER:]] g     O __DATA,__common _number
+# CHECK-NEXT: [[#%x,FOO:]]    g     O __TEXT,obj _foo
+
+# CHECK-NEXT: # Path: {{.*}}{{/|\\}}map-file.s.tmp/test-map
+# CHECK-NEXT: # Arch: x86_64
+# CHECK-NEXT: # Object files:
+# CHECK-NEXT: [  0] linker synthesized
+# CHECK-NEXT: [  1] {{.*}}{{/|\\}}map-file.s.tmp/test.o
+# CHECK-NEXT: [  2] {{.*}}{{/|\\}}map-file.s.tmp/foo.o
+
+# CHECK-NEXT: # Sections:
+# CHECK-NEXT: # Address    Size              Segment    Section
+# CHECK-NEXT: 0x[[#TEXT]]  0x{{[0-9a-f]+}}   __TEXT  __text
+# CHECK-NEXT: 0x[[#DATA]]  0x{{[0-9a-f]+}}   __TEXT  obj
+# CHECK-NEXT: 0x[[#BSS]]   0x{{[0-9a-f]+}}   __DATA  __common
+
+# CHECK-NEXT: # Symbols:
+# CHECK-NEXT: # Address        File  Name
+# CHECK-NEXT: 0x[[#NUMBER]]    [  1]  _number
+# CHECK-NEXT: 0x[[#MAIN]]      [  1]  _main
+# CHECK-NEXT: 0x[[#FOO]]       [  2]  _foo