[lld-macho] Set __PAGEZERO size to 4GB

[int3]

Summary

That's what ld64 uses for 64-bit targets. I figured it's best
to make this change sooner rather than later since a bunch of our tests
are relying on hardcoded addresses that depend on this value.

Depends on D80169.

[smeenai]

Thanks for fixing this! It'll make it way easier to compare results with ld64.

Requesting changes cos I think we're gonna mess up entryoff as this stands, and we definitely need to add test coverage for that.

lld/MachO/Target.h

28

It's really confusing to have PageZeroSize and ImageBase be different, and for that matter, to have both of them.

With this diff, we're only using ImageBase in one place: https://github.com/llvm/llvm-project/blob/master/lld/MachO/Writer.cpp#L171. From looking into LC_MAIN, it seems like the entryoff is supposed to be an offset in the file rather than a virtual address, so computing it in terms of getVA is confusing. We should also fix that and then get rid of either ImageBase or PageZeroSize.

For that matter, is our LC_MAIN entryoff calculation gonna be correct after this? I have a hard time imagining that, and we don't have any tests for it :/

[int3]

lld/MachO/Target.h

28

oops, good catch...

[int3]

fix and add test

[int3]

make ENTRYOFF a numeric var

[smeenai]

Looks great, thanks!

lld/MachO/Symbols.h

110 ↗

(On Diff #265058)

This is *definitely* follow-up territory, but we should figure out if it ever makes sense for getFileOffset to be called on a non-defined symbol, and if not, just fail loudly in that case instead of silently returning 0.

[int3]

lld/MachO/Symbols.h

110 ↗

(On Diff #265058)

oh good point. I doubt it would ever be valid; I can just insert a fatal here. Does it really need to be in a follow-up?

[smeenai]

lld/MachO/Symbols.h

110 ↗

(On Diff #265058)

It doesn't need to be a follow-up. I just didn't mean the comment to block this diff, but folding it in here is great too.

[int3]

fatal

[smeenai]

lld/MachO/Symbols.h

110 ↗

(On Diff #265058)

I'd probably use llvm_unreachable, since reaching this would be purely a logic error in the linker rather then e.g. something that could be triggered by broken or unsupported input.

[int3]

llvm_unreachable

[thakis]

Looks like this is failing on Windows: http://45.33.8.238/win/16635/step_10.txt

[int3]

Thanks for the heads up, just pushed a likely fix and will monitor http://lab.llvm.org:8011/builders/lld-x86_64-win. I'll set up a local Windows VM to catch future such issues earlier...

[vzakhari]

In D80177#2069811, @int3 wrote:

Thanks for the heads up, just pushed a likely fix and will monitor http://lab.llvm.org:8011/builders/lld-x86_64-win. I'll set up a local Windows VM to catch future such issues earlier...

@int3, can you please point out the fix you pushed? The tests are failing downstream, and I am looking for the commit that fixes it.

Moreover, in the head llvm-project there are still issues for 32-bit host builds:

enum : uint64_t {
...
  PageZeroSize = 1ull << 32, // XXX should be 4096 for 32-bit targets
...
};

PageZeroSize is a 64-bit constant, but it is being returned as size_t, which is 4 bytes for 32-bit hosts:

class PageZeroSection : public SyntheticSection {
...
  size_t getSize() const override { return PageZeroSize; }
...

Could you please fix this?

[int3]

rGd767de44bf9527cb5058f5fe16aac2f23c21977c was the referenced fix. Good point about size_t, I'll change that

[vzakhari]

In D80177#2097037, @int3 wrote:

rGd767de44bf9527cb5058f5fe16aac2f23c21977c was the referenced fix. Good point about size_t, I'll change that

Thanks. The enum change does not fix the fails on my side. I guess it has to be the size_t issue.

lld/MachO/SyntheticSections.h

 //===- SyntheticSections.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_SYNTHETIC_SECTIONS_H
 #define LLD_MACHO_SYNTHETIC_SECTIONS_H
 
+#include "Config.h"
 #include "ExportTrie.h"
 #include "InputSection.h"
 #include "OutputSection.h"
 #include "Target.h"
 
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Support/raw_ostream.h"
 

 };
 
 // A hidden section that exists solely for the purpose of creating the
 // __PAGEZERO segment, which is used to catch null pointer dereferences.
 class PageZeroSection : public SyntheticSection {
 public:
   PageZeroSection();
   bool isHidden() const override { return true; }
-  size_t getSize() const override { return ImageBase; }
+  size_t getSize() const override { return PageZeroSize; }
   uint64_t getFileSize() const override { return 0; }
   void writeTo(uint8_t *buf) const override {}
 };
 
 // This section will be populated by dyld with addresses to non-lazily-loaded
 // dylib symbols.
 class GotSection : public SyntheticSection {
 public:

lld/MachO/Target.h

 class InputSection;
 struct Reloc;
 
 enum {
   // We are currently only supporting 64-bit targets since macOS and iOS are
   // deprecating 32-bit apps.
   WordSize = 8,
   PageSize = 4096,
+  PageZeroSize = 1ull << 32, // XXX should be 4096 for 32-bit targets
   ImageBase = 4096,
   MaxAlignmentPowerOf2 = 32,
 };
 
 class TargetInfo {
 public:
   virtual ~TargetInfo() = default;
 
   virtual uint64_t getImplicitAddend(const uint8_t *loc,

lld/test/MachO/segments.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: lld -flavor darwinnew -o %t %t.o
 # RUN: llvm-readobj --macho-segment %t | FileCheck %s
 
 ## These two segments must always be present at the start of an executable.
 # CHECK-NOT:  Segment {
 # CHECK:      Segment {
 # CHECK:        Cmd: LC_SEGMENT_64
 # CHECK:        Name: __PAGEZERO
 # CHECK:        Size: 72
-# CHECK:        vmaddr:
-# CHECK:        vmsize:
+# CHECK:        vmaddr: 0x0
+# CHECK:        vmsize: 0x100000000
 # CHECK:        fileoff: 0
 # CHECK:        filesize: 0
 ## The kernel won't execute a binary with the wrong protections for __PAGEZERO.
 # CHECK:        maxprot: ---
 # CHECK:        initprot: ---
 # CHECK:        nsects: 0
 # CHECK:        flags: 0x0
 # CHECK:      }
 # CHECK:      Segment {
 # CHECK:        Cmd: LC_SEGMENT_64
 # CHECK:        Name: __TEXT
 # CHECK:        Size: 152
-# CHECK:        vmaddr:
+# CHECK:        vmaddr: 0x100000000
 # CHECK:        vmsize:
 ## dyld3 assumes that the __TEXT segment starts from the file header
 # CHECK:        fileoff: 0
 # CHECK:        filesize:
 # CHECK:        maxprot: rwx
 # CHECK:        initprot: r-x
 # CHECK:        nsects: 1
 # CHECK:        flags: 0x0

lld/test/MachO/x86-64-reloc-signed.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: lld -flavor darwinnew -o %t %t.o
 # RUN: llvm-objdump -D %t | FileCheck %s
 
 # CHECK:      <_main>:
-# CHECK-NEXT:   movl {{.*}}  # 2000 <_s>
+# CHECK-NEXT:   movl {{.*}}  # 100001000 <_s>
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movl {{.*}}  # 2002 <_s+0x2>
+# CHECK-NEXT:   movl {{.*}}  # 100001002 <_s+0x2>
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movb {{.*}}  # 2000 <_s>
+# CHECK-NEXT:   movb {{.*}}  # 100001000 <_s>
 # CHECK-NEXT:   callq {{.*}}
 # CHECK:      <__not_text>:
-# CHECK-NEXT:   movl {{.*}}  # 2005
+# CHECK-NEXT:   movl {{.*}}  # 100001005
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movl {{.*}}  # 2007
+# CHECK-NEXT:   movl {{.*}}  # 100001007
 # CHECK-NEXT:   callq {{.*}}
-# CHECK-NEXT:   movb {{.*}}  # 2005
+# CHECK-NEXT:   movb {{.*}}  # 100001005
 # CHECK-NEXT:   callq {{.*}}
 
 .section __TEXT,__text
 .globl _main
 _main:
   ## Symbol relocations
   movl $0x434241, _s(%rip)  # X86_64_RELOC_SIGNED_4
   callq _f

lld/test/MachO/x86-64-reloc-unsigned.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: lld -flavor darwinnew -o %t %t.o
 # RUN: llvm-objdump --full-contents %t | FileCheck %s
 # CHECK: Contents of section foo:
-# CHECK:  2000 08200000 00000000
+# CHECK:  100001000 08100000 01000000
 # CHECK: Contents of section bar:
-# CHECK:  2008 11311111 01000000
+# CHECK:  100001008 11211111 02000000
 
 .globl _main, _foo, _bar
 
 .section __DATA,foo
 _foo:
 .quad _bar
 
 .section __DATA,bar
 _bar:
 ## The unsigned relocation should support 64-bit addends
 .quad _foo + 0x111111111
 
 .text
 _main:
   mov $0, %rax
   ret