[lld-macho][easy] Fix segment max protection

[int3]

Summary

We should have maxprot == initprot for all non-i386 architectures, which
is what ld64 does.

[compnerd]

lld/MachO/OutputSegment.cpp

36

It seems that it would be better to replace initProt and maxProt with just prot? Or just filling in the differences that require this would be a good idea.

[int3]

lld/MachO/OutputSegment.cpp

36

Does it make sense to fill in code that we're not going to be able to test?

[int3]

lld/MachO/OutputSegment.cpp

36

I wrote it this way so it would be easy to "fill out" the missing bits when we support i386

[compnerd]

lld/MachO/OutputSegment.cpp

36

I think that it does make sense to fill it out now even if we cannot test it. Alternatively, we could defer the setup to the point where it is needed. In the current state, it is very confusing and not very valuable.

[int3]

lld/MachO/OutputSegment.cpp

36

I think the assertion makes the intent clear -- they're two separate functions because on i386 their behaviors deviate. Moreover, making it an assertion ensures that when we do add support for i386, it'll be immediately obvious that a test needs to be written to cover this edge case.

[compnerd]

SGTM

lld/MachO/OutputSegment.cpp

static uint32_t initProt(StringRef name) {
   if (name == segment_names::pageZero)
     return 0;
   if (name == segment_names::linkEdit)
     return VM_PROT_READ;
   return VM_PROT_READ | VM_PROT_WRITE;
 }
 
 static uint32_t maxProt(StringRef name) {
-  if (name == segment_names::pageZero)
-    return 0;
-  return VM_PROT_READ | VM_PROT_WRITE | VM_PROT_EXECUTE;
+  assert(config->arch != AK_i386 &&
+         "TODO: i386 has different maxProt requirements");
+  return initProt(name);
 }
 
 size_t OutputSegment::numNonHiddenSections() const {
   size_t count = 0;
   for (const OutputSection *osec : sections) {
     count += (!osec->isHidden() ? 1 : 0);
   }
   return count;

lld/test/MachO/segments.s

 # CHECK-NEXT:   Cmd: LC_SEGMENT_64
 # CHECK-NEXT:   Name: __TEXT
 # CHECK-NEXT:   Size: 152
 # CHECK-NEXT:   vmaddr: 0x100000000
 # CHECK-NEXT:   vmsize:
 ## dyld3 assumes that the __TEXT segment starts from the file header
 # CHECK-NEXT:   fileoff: 0
 # CHECK-NEXT:   filesize:
-# CHECK-NEXT:   maxprot: rwx
+# CHECK-NEXT:   maxprot: r-x
 # CHECK-NEXT:   initprot: r-x
 # CHECK-NEXT:   nsects: 1
 # CHECK-NEXT:   flags: 0x0
 # CHECK-NEXT: }
 
 ## Check that we handle max-length names correctly.
 # CHECK:      Cmd: LC_SEGMENT_64
 # CHECK-NEXT: Name: maxlen_16ch_name
 
 ## This segment must always be present at the end of an executable, and cover
 ## its last byte.
 # CHECK:      Name: __LINKEDIT
 # CHECK-NEXT: Size:
 # CHECK-NEXT: vmaddr:
 # CHECK-NEXT: vmsize:
 # CHECK-NEXT: fileoff: [[#%u, LINKEDIT_OFF:]]
 # CHECK-NEXT: filesize: [[#%u, LINKEDIT_SIZE:]]
-# CHECK-NEXT: maxprot: rwx
+# CHECK-NEXT: maxprot: r--
 # CHECK-NEXT: initprot: r--
 # CHECK-NOT:  Cmd: LC_SEGMENT_64
 
 # CHECK-LABEL: Total file size
 # CHECK-NEXT:  [[#%u, LINKEDIT_OFF + LINKEDIT_SIZE]]
 
 .text
 .global _main
 _main:
   mov $0, %rax
   ret
 
 .section maxlen_16ch_name,foo