[lld] f7b752d - [lld-macho] Set the SG_READ_ONLY flag on __DATA_CONST

[Daniel Bertalan via llvm-commits]

Author: Daniel Bertalan
Date: 2022-08-31T17:04:20+02:00
New Revision: f7b752d27766ecf0241edfbdc4d7142fc40e7621

URL: https://github.com/llvm/llvm-project/commit/f7b752d27766ecf0241edfbdc4d7142fc40e7621
DIFF: https://github.com/llvm/llvm-project/commit/f7b752d27766ecf0241edfbdc4d7142fc40e7621.diff

LOG: [lld-macho] Set the SG_READ_ONLY flag on __DATA_CONST

This flag instructs dyld to make the segment read-only after fixups have
been performed.

I'm not sure why this flag is needed, as on macOS 13 beta at least,
__DATA_CONST is read-only even without this flag; but ld64 sets it as
well.

Differential Revision: https://reviews.llvm.org/D133010

Added: 
    

Modified: 
    lld/MachO/OutputSegment.cpp
    lld/MachO/OutputSegment.h
    lld/MachO/Writer.cpp
    lld/test/MachO/builtin-rename.s
    llvm/include/llvm/BinaryFormat/MachO.h
    llvm/tools/llvm-objdump/MachODump.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/OutputSegment.cpp b/lld/MachO/OutputSegment.cpp
index 91770f58b805a..fa56c1314fff6 100644
--- a/lld/MachO/OutputSegment.cpp
+++ b/lld/MachO/OutputSegment.cpp
@@ -44,6 +44,12 @@ static uint32_t maxProt(StringRef name) {
   return initProt(name);
 }
 
+static uint32_t flags(StringRef name) {
+  // If we ever implement shared cache output support, SG_READ_ONLY should not
+  // be used for dylibs that can be placed in it.
+  return name == segment_names::dataConst ? SG_READ_ONLY : 0;
+}
+
 size_t OutputSegment::numNonHiddenSections() const {
   size_t count = 0;
   for (const OutputSection *osec : sections)
@@ -185,6 +191,7 @@ OutputSegment *macho::getOrCreateOutputSegment(StringRef name) {
   segRef->name = name;
   segRef->maxProt = maxProt(name);
   segRef->initProt = initProt(name);
+  segRef->flags = flags(name);
 
   outputSegments.push_back(segRef);
   return segRef;

diff  --git a/lld/MachO/OutputSegment.h b/lld/MachO/OutputSegment.h
index a1b6cef9e2c7c..7a0c4a2065a15 100644
--- a/lld/MachO/OutputSegment.h
+++ b/lld/MachO/OutputSegment.h
@@ -55,6 +55,7 @@ class OutputSegment {
   StringRef name;
   uint32_t maxProt = 0;
   uint32_t initProt = 0;
+  uint32_t flags = 0;
   uint8_t index;
 
   llvm::TinyPtrVector<Defined *> segmentStartSymbols;

diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 01054fe773b1f..89b14e15b3996 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -246,6 +246,7 @@ template <class LP> class LCSegment final : public LoadCommand {
     c->vmsize = seg->vmSize;
     c->filesize = seg->fileSize;
     c->nsects = seg->numNonHiddenSections();
+    c->flags = seg->flags;
 
     for (const OutputSection *osec : seg->getSections()) {
       if (osec->isHidden())

diff  --git a/lld/test/MachO/builtin-rename.s b/lld/test/MachO/builtin-rename.s
index dc9b7f8f75105..22b2479c02048 100644
--- a/lld/test/MachO/builtin-rename.s
+++ b/lld/test/MachO/builtin-rename.s
@@ -53,6 +53,23 @@
 # YDATA-DAG: __DATA_CONST,__objc_protolist __DATA__objc_protolist
 # YDATA-DAG: __DATA_CONST,__nl_symbol_ptr __IMPORT__pointers
 
+## Check that the SG_READ_ONLY flag is set on __DATA_CONST.
+# RUN: llvm-otool -v -l %t/ydata | \
+# RUN:     FileCheck %s --check-prefix=FLAGS
+
+# FLAGS-LABEL: Load command 2
+# FLAGS-NEXT:      cmd LC_SEGMENT_64
+# FLAGS-NEXT:  cmdsize
+# FLAGS-NEXT:  segname __DATA_CONST
+# FLAGS-NEXT:   vmaddr
+# FLAGS-NEXT:   vmsize
+# FLAGS-NEXT:  fileoff
+# FLAGS-NEXT: filesize
+# FLAGS-NEXT:  maxprot rw-
+# FLAGS-NEXT: initprot rw-
+# FLAGS-NEXT:   nsects 13
+# FLAGS-NEXT:    flags SG_READ_ONLY
+
 ## LLD doesn't support defining symbols in synthetic sections, so we test them
 ## via this slightly more awkward route.
 # RUN: llvm-readobj --section-headers %t/ydata | \

diff  --git a/llvm/include/llvm/BinaryFormat/MachO.h b/llvm/include/llvm/BinaryFormat/MachO.h
index 8627ed68bf09c..04be767250e61 100644
--- a/llvm/include/llvm/BinaryFormat/MachO.h
+++ b/llvm/include/llvm/BinaryFormat/MachO.h
@@ -107,6 +107,7 @@ enum : uint32_t {
   SG_FVMLIB = 0x2u,
   SG_NORELOC = 0x4u,
   SG_PROTECTED_VERSION_1 = 0x8u,
+  SG_READ_ONLY = 0x10u,
 
   // Constant masks for the "flags" field in llvm::MachO::section and
   // llvm::MachO::section_64

diff  --git a/llvm/tools/llvm-objdump/MachODump.cpp b/llvm/tools/llvm-objdump/MachODump.cpp
index f615fbf4d81c5..a5d611e99b38e 100644
--- a/llvm/tools/llvm-objdump/MachODump.cpp
+++ b/llvm/tools/llvm-objdump/MachODump.cpp
@@ -8858,6 +8858,12 @@ static void PrintSegmentCommand(uint32_t cmd, uint32_t cmdsize,
         outs() << " PROTECTED_VERSION_1";
         flags &= ~MachO::SG_PROTECTED_VERSION_1;
       }
+      if (flags & MachO::SG_READ_ONLY) {
+        // Apple's otool prints the SG_ prefix for this flag, but not for the
+        // others.
+        outs() << " SG_READ_ONLY";
+        flags &= ~MachO::SG_READ_ONLY;
+      }
       if (flags)
         outs() << format(" 0x%08" PRIx32, flags) << " (unknown flags)\n";
       else