Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/link: use __DATA_CONST on darwin? #38830

Closed
bradfitz opened this issue May 3, 2020 · 9 comments
Closed

cmd/link: use __DATA_CONST on darwin? #38830

bradfitz opened this issue May 3, 2020 · 9 comments
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Milestone
Unplanned

Comments

@bradfitz
Copy link
Contributor

bradfitz commented May 3, 2020

Recent iOS/macOS (starting at iOS 9?) support a new segment __DATA_CONST:

https://developer.apple.com/documentation/xcode_release_notes/xcode_11_release_notes?language=objc

In order to improve performance and security the static linker (ld) now moves globals that are marked as constant into a new segment: __DATA_CONST. These globals may consist of compiler generated pointers that the dynamic linker (dyld) needs to fix up during load, but are otherwise constant such as vtables and explicitly declared constant pointers. Once dyld has finished loading the image it makes __DATA_CONST readonly. This change doesn’t impact well behaved code, but may break code that depends on undefined behavior such as using a type pun to write to a pointer that’s declared as const. (50898833)

Other information online suggests that such segments don't contribute to the memory limit of processes on iOS, as they can always be paged in from disk.

Are there any Go sections that could go in that segment?

On a macOS binary now (with -ldflags=-w), I see:

sect[0] = {Name:__text Seg:__TEXT Addr:16781312 Size:1107627 Offset:4096 Align:4 Reloff:0 Nreloc:0 Flags:2147484672}
sect[1] = {Name:__rodata Seg:__TEXT Addr:17888960 Size:519427 Offset:1111744 Align:5 Reloff:0 Nreloc:0 Flags:0}
sect[2] = {Name:__symbol_stub1 Seg:__TEXT Addr:18408416 Size:510 Offset:1631200 Align:5 Reloff:0 Nreloc:0 Flags:2147484680}
sect[3] = {Name:__typelink Seg:__TEXT Addr:18408928 Size:4068 Offset:1631712 Align:5 Reloff:0 Nreloc:0 Flags:0}
sect[4] = {Name:__itablink Seg:__TEXT Addr:18413000 Size:752 Offset:1635784 Align:3 Reloff:0 Nreloc:0 Flags:0}
sect[5] = {Name:__gosymtab Seg:__TEXT Addr:18413752 Size:0 Offset:1636536 Align:0 Reloff:0 Nreloc:0 Flags:0}
sect[6] = {Name:__gopclntab Seg:__TEXT Addr:18413760 Size:666774 Offset:1636544 Align:5 Reloff:0 Nreloc:0 Flags:0}
sect[7] = {Name:__go_buildinfo Seg:__DATA Addr:19083264 Size:32 Offset:2306048 Align:4 Reloff:0 Nreloc:0 Flags:0}
sect[8] = {Name:__nl_symbol_ptr Seg:__DATA Addr:19083296 Size:696 Offset:2306080 Align:2 Reloff:0 Nreloc:0 Flags:6}
sect[9] = {Name:__noptrdata Seg:__DATA Addr:19084000 Size:74788 Offset:2306784 Align:5 Reloff:0 Nreloc:0 Flags:0}
sect[10] = {Name:__data Seg:__DATA Addr:19158816 Size:38288 Offset:2381600 Align:5 Reloff:0 Nreloc:0 Flags:0}
sect[11] = {Name:__bss Seg:__DATA Addr:19197120 Size:200864 Offset:0 Align:5 Reloff:0 Nreloc:0 Flags:1}
sect[12] = {Name:__noptrbss Seg:__DATA Addr:19397984 Size:9992 Offset:0 Align:5 Reloff:0 Nreloc:0 Flags:1}

I know relatively little about linkers but see no reference to __DATA_CONST in Go's tree, and see other projects online that have added __DATA_CONST support, so was curious if it's applicable for Go.

/cc @ianlancetaylor @cherrymui @thanm @jeremyfaller @aclements (people who seem to know linkers)
@bradfitz bradfitz added this to the Unplanned milestone May 3, 2020
@cherrymui
Copy link
Member

I think __rodata, __typelink, __itablink, and __gopclntab can go there. These are read-only data sections currently in __TEXT, or __DATA (for PIE binaries and a few configurations).

The question is, is it only supported in newer version of macOS and/or newer version of darwin linker? We probably still need to do the old way to support old platforms.

@bradfitz
Copy link
Contributor Author

bradfitz commented May 4, 2020

Sierra came out a year after iOS 9 so I assume we're good to use it unconditionally at this point.

@ianlancetaylor
Copy link
Member

At first glance the new section sounds identical to ELF's relro section, which cmd/link already supports.

@cherrymui
Copy link
Member

Yeah, I believe the linker already has the relevant bits. It's probably just a matter of where to put those sections.

@bradfitz
Copy link
Contributor Author

bradfitz commented May 5, 2020

cc @mwhudson too, as there's a TODO in the code:

        // TODO(mwhudson): It would make sense to do this more widely, but it makes                                                                                               
        // the system linker segfault on darwin.                                                                                                                                  
        addrelrosection := func(suffix string) *sym.Section {
                return addsection(ctxt.Arch, segro, suffix, 04)
        }

Is that darwin segfault referring to __DATA_CONST?

@mwhudson
Copy link
Contributor

mwhudson commented May 5, 2020

Honestly I have no idea any more. That comment is from May 2015 though, which is slightly before iOS 9 -- so I suspect it must have been something else.

@dmitshur dmitshur added OS-Darwin NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest Issues asking for a new feature that does not need a proposal. labels May 8, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/253918 mentions this issue: cmd/internal/objfile: recognize Mach-O __DATA_CONST segment as read-only data

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/253919 mentions this issue: cmd/link: put read-only data in __DATA_CONST segment

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/253921 mentions this issue: cmd/link: add a test to test RODATA is indeed read-only

gopherbot pushed a commit that referenced this issue Sep 11, 2020
@cherrymui
cmd/internal/objfile: recognize Mach-O __DATA_CONST segment as read-o…
a531bd5 
…nly data

Updates #38830.

Change-Id: I826c6b0a42bc8e48fcda556250ca4a95c73987eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/253918
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
gopherbot pushed a commit that referenced this issue Sep 11, 2020
@cherrymui
cmd/link: add a test to test RODATA is indeed read-only
1ed4f12 
Updates #38830.

Change-Id: Ie1f6ccef40a773f038aac587dfc26bf70a1a8536
Reviewed-on: https://go-review.googlesource.com/c/go/+/253921
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Darwin
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@bradfitz @mwhudson @dmitshur @ianlancetaylor @gopherbot @cherrymui