[lld-macho] Implement options -rename_section -rename_segment

[gkm]

Summary

Implement command-line options to rename output sections & segments.

[gkm]

lld/MachO/InputFiles.cpp

109 ↗

(On Diff #326855)

FYI: This turned-up in my option-parsing test: when I omitted a sub-arg, then it swallowed -o, leaving /dev/null without a prefix, so it was interpreted as an input filename. Since its size was zero, ASAN caught the attempted read of the non-existent header.

[int3]

lld/MachO/Config.h

28–29

try to avoid std::map in general (as per the LLVM programmer's manual). In this case we could use a DenseMap of NamePair and a DenseMap of CachedHashStringRef respectively

86–93

Config should be just a plain bag of values. I think this can be a standalone function in Writer.cpp

lld/MachO/Driver.cpp

822

lld/MachO/InputFiles.cpp

109 ↗

(On Diff #326855)

Ah, nice catch. This should probably be in a separate diff with its own test. Also, I think we should make it an error if the file is shorter than sizeof(hdr->magic)... one issue though is that we currently use readFile to read non-binary files like the symbol order file, but I think we can just create a separate function for that

[gkm]

lld/MachO/Config.h

28–29

Why CachedHashStringRef ? Is that supposed to give us some code sharing, at the expense of clarity?

[gkm]

• revise according to review feedback

[int3]

lld/MachO/Config.h

28–29

It saves on recomputation of the hash. Not that it matters here since this map shouldn't part of a hot loop, but I just use it everywhere we need a map of string keys, so I don't have to think about whether performance is important in a particular case.

There's also StringMap, but that allocates and owns its own strings, which for LLD isn't really optimal since our strings are mostly interned / not deallocated till the end of the process.

lld/test/MachO/rename.s

33

[gkm]

• amend according to review feedback
• migrate some changes to lld/test/MachO/rename.s from a successor diff that really belong here

[thakis]

Looks like this breaks tests on windows: http://45.33.8.238/win/34054/step_10.txt

Please take a look, and revert for now if it takes a while to fix.

[int3]

D97610 which just landed will likely fix it

lld/MachO/Config.h

 #include <vector>
 
 namespace lld {
 namespace macho {
 
 class Symbol;
 struct SymbolPriorityEntry;
 
+using NamePair = std::pair<llvm::StringRef, llvm::StringRef>;
+using SectionRenameMap = llvm::DenseMap<NamePair, NamePair>;
+using SegmentRenameMap = llvm::DenseMap<llvm::StringRef, llvm::StringRef>;
+
 struct PlatformInfo {
   llvm::MachO::PlatformKind kind;
   llvm::VersionTuple minimum;
   llvm::VersionTuple sdk;
 };
 
 enum class NamespaceKind {
   twolevel,
UndefinedSymbolTreatment undefinedSymbolTreatment =
       UndefinedSymbolTreatment::error;
   llvm::MachO::HeaderFileType outputType;
   std::vector<llvm::StringRef> systemLibraryRoots;
   std::vector<llvm::StringRef> librarySearchPaths;
   std::vector<llvm::StringRef> frameworkSearchPaths;
   std::vector<llvm::StringRef> runtimePaths;
   std::vector<Symbol *> explicitUndefineds;
   llvm::DenseMap<llvm::StringRef, SymbolPriorityEntry> priorities;
+  SectionRenameMap sectionRenameMap;
+  SegmentRenameMap segmentRenameMap;
 };
 
 // The symbol with the highest priority should be ordered first in the output
 // section (modulo input section contiguity constraints). Using priority
 // (highest first) instead of order (lowest first) has the convenient property
 // that the default-constructed zero priority -- for symbols/sections without a
 // user-defined order -- naturally ends up putting them at the end of the
 // output.
 struct SymbolPriorityEntry {
   // The priority given to a matching symbol, regardless of which object file
   // it originated from.
   size_t anyObjectFile = 0;
   // The priority given to a matching symbol from a particular object file.
   llvm::DenseMap<llvm::StringRef, size_t> objectFiles;
 };
 
 extern Configuration *config;
 
 } // namespace macho
 } // namespace lld
 
 #endif

lld/MachO/Driver.cpp

if (const opt::Arg *arg =
           args.getLastArg(OPT_search_paths_first, OPT_search_dylibs_first))
     config->searchDylibsFirst =
         arg->getOption().getID() == OPT_search_dylibs_first;
 
   config->dylibCompatibilityVersion =
       parseDylibVersion(args, OPT_compatibility_version);
   config->dylibCurrentVersion = parseDylibVersion(args, OPT_current_version);
 
+  // Reject every special character except '.' and '$'
+  // TODO(gkm): verify that this is the proper set of invalid chars
+  StringRef invalidNameChars("!\"#%&'()*+,-/:;<=>?@[\\]^`{|}~");
+  auto validName = [invalidNameChars](StringRef s) {
+    if (s.find_first_of(invalidNameChars) != StringRef::npos)
+      error("invalid name for segment or section: " + s);
+    return s;
+  };
+  for (opt::Arg *arg : args.filtered(OPT_rename_section)) {
+    config->sectionRenameMap[{validName(arg->getValue(0)),
+                              validName(arg->getValue(1))}] = {
+        validName(arg->getValue(2)), validName(arg->getValue(3))};
+  }
+  for (opt::Arg *arg : args.filtered(OPT_rename_segment)) {
+    config->segmentRenameMap[validName(arg->getValue(0))] =
+        validName(arg->getValue(1));
+  }
+
   config->saveTemps = args.hasArg(OPT_save_temps);
 
   if (args.hasArg(OPT_v)) {
     message(getLLDVersion());
     message(StringRef("Library search paths:") +
             (config->librarySearchPaths.size()
                  ? "\n\t" + join(config->librarySearchPaths, "\n\t")
                  : ""));

lld/MachO/Options.td

 def move_to_ro_segment : MultiArg<["-"], "move_to_ro_segment", 2>,
      MetaVarName<"<segment> <path>">,
      HelpText<"Move code symbols listed in <path> to another <segment>">,
      Flags<[HelpHidden]>,
      Group<grp_rare>;
 def rename_section : MultiArg<["-"], "rename_section", 4>,
      MetaVarName<"<from_segment> <from_section> <to_segment> <to_section>">,
      HelpText<"Rename <from_segment>/<from_section> as <to_segment>/<to_section>">,
-     Flags<[HelpHidden]>,
      Group<grp_rare>;
 def rename_segment : MultiArg<["-"], "rename_segment", 2>,
      MetaVarName<"<from_segment> <to_segment>">,
      HelpText<"Rename <from_segment> as <to_segment>">,
-     Flags<[HelpHidden]>,
      Group<grp_rare>;
 def trace_symbol_layout : Flag<["-"], "trace_symbol_layout">,
      HelpText<"Show where and why symbols move, as specified by -move_to_ro_segment, -move_to_rw_segment, -rename_section, and -rename_segment">,
      Flags<[HelpHidden]>,
      Group<grp_rare>;
 def section_order : MultiArg<["-"], "section_order", 2>,
      MetaVarName<"<segment> <sections>">,
      HelpText<"With -preload, specify layout sequence of colon-separated <sections> in <segment>">,

lld/MachO/Writer.cpp

for (OutputSection *osec : seg->getSections()) {
                               return isecPriorities[a] > isecPriorities[b];
                             });
         }
       }
     }
   }
 }
 
+static NamePair maybeRenameSection(NamePair key) {
+  auto newNames = config->sectionRenameMap.find(key);
+  if (newNames != config->sectionRenameMap.end())
+    return newNames->second;
+  auto newName = config->segmentRenameMap.find(key.first);
+  if (newName != config->segmentRenameMap.end())
+    return std::make_pair(newName->second, key.second);
+  return key;
+}
+
 void Writer::createOutputSections() {
   // First, create hidden sections
   stringTableSection = make<StringTableSection>();
   unwindInfoSection = make<UnwindInfoSection>(); // TODO(gkm): only when no -r
   symtabSection = make<SymtabSection>(*stringTableSection);
   indirectSymtabSection = make<IndirectSymtabSection>();
   if (config->outputType == MH_EXECUTE &&
       (config->arch == AK_arm64 || config->arch == AK_arm64e))
     codeSignatureSection = make<CodeSignatureSection>();
 
   switch (config->outputType) {
   case MH_EXECUTE:
     make<PageZeroSection>();
     break;
   case MH_DYLIB:
   case MH_BUNDLE:
     break;
   default:
     llvm_unreachable("unhandled output file type");
   }
 
   // Then merge input sections into output sections.
-  MapVector<std::pair<StringRef, StringRef>, MergedOutputSection *>
-      mergedOutputSections;
+  MapVector<NamePair, MergedOutputSection *> mergedOutputSections;
   for (InputSection *isec : inputSections) {
-    MergedOutputSection *&osec =
-        mergedOutputSections[{isec->segname, isec->name}];
+    NamePair names = maybeRenameSection({isec->segname, isec->name});
+    MergedOutputSection *&osec = mergedOutputSections[names];
     if (osec == nullptr)
-      osec = make<MergedOutputSection>(isec->name);
+      osec = make<MergedOutputSection>(names.second);
     osec->mergeInput(isec);
   }
 
   for (const auto &it : mergedOutputSections) {
     StringRef segname = it.first.first;
     MergedOutputSection *osec = it.second;
     if (unwindInfoSection && segname == segment_names::ld) {
       assert(osec->name == section_names::compactUnwind);

lld/test/MachO/rename.s

+# REQUIRES: x86
+# RUN: rm -fr %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+# RUN: %lld -o %t %t.o
+
+## Check option format
+# RUN: not %lld \
+# RUN:     -rename_section B@GUS_SEG b@gus_sect S/ASHY_SEG st*rry_sect \
+# RUN:     -rename_section __FROM_SECT __from_sect __TO_SECT \
+# RUN:   -o /dev/null %t.o
+# RUN: not %lld \
+# RUN:     -rename_segment H#SHY_SEG pl+ssy_sect \
+# RUN:     -rename_segment __FROM_SEG \
+# RUN:   -o /dev/null %t.o
+
+## Check that section and segment renames happen
+# RUN: %lld \
+# RUN:     -rename_section __FROM_SECT __from_sect __TO_SECT __to_sect \
+# RUN:     -rename_segment __FROM_SEG __TO_SEG \
+# RUN:   -o %t %t.o
+# RUN: llvm-objdump --macho --all-headers %t | FileCheck %s
+
+# CHECK:      {{^}}Section{{$}}
+# CHECK-NEXT: sectname __text
+# CHECK-NEXT: segname __TEXT
+# CHECK:      {{^}}Section{{$}}
+# CHECK-NOT:  sectname __from_sect
+# CHECK-NEXT: sectname __to_sect
+# CHECK-NOT:  segname __FROM_SECT
+# CHECK-NEXT: segname __TO_SECT
+# CHECK:      {{^}}Section{{$}}
+# CHECK-NEXT: sectname __from_seg
+# CHECK-NOT: segname __FROM_SEG
+# CHECK-NEXT: segname __TO_SEG
+
+.section __FROM_SECT,__from_sect
+.global _from_sect
+_from_sect:
+  .space 8
+
+.section __FROM_SEG,__from_seg
+.global _from_seg
+_from_seg:
+  .space 8
+
+.text
+.global _main
+_main:
+  ret