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.

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/ld: race enabled binaries are too large #9359

Closed
minux opened this issue Dec 17, 2014 · 4 comments
Closed

cmd/ld: race enabled binaries are too large #9359

minux opened this issue Dec 17, 2014 · 4 comments
Labels
FrozenDueToAge
Milestone
Go1.4.1

Comments

@minux
Copy link
Member

minux commented Dec 17, 2014

on linux/amd64, go test -c fmt produces a binary of 3847664 bytes, however,
go test -c -race fmt produces a binary of 14653432 bytes, almost 10MB larger!

This issue also affects the 1.4 release.

The problem is that this symbol:
0000000000c09bc0 0000000000982648 d runtime/race(.bss)
which should be put into the bss section, actually consumes space in the binary.

You can also see the problem in the program headers:

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  PHDR           0x0000000000000040 0x0000000000400040 0x0000000000400040
                 0x00000000000001f8 0x00000000000001f8  R      1000
  INTERP         0x0000000000000be4 0x0000000000400be4 0x0000000000400be4
                 0x000000000000001c 0x000000000000001c  R      1
      [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
  LOAD           0x0000000000000040 0x0000000000400040 0x0000000000400040
                 0x0000000000458630 0x0000000000458630  R E    1000
  LOAD           0x0000000000459000 0x0000000000859000 0x0000000000859000
                 0x00000000003991d8 0x00000000003991d8  R      1000
  LOAD           0x00000000007f3000 0x0000000000bf3000 0x0000000000bf3000
                 0x00000000009a2fa0 0x00000000009bffe8  RW     1000
  DYNAMIC        0x00000000007f3140 0x0000000000bf3140 0x0000000000bf3140
                 0x0000000000000140 0x0000000000000140  RW     8
  TLS            0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000010  R      8
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     8
  LOOS+5041580   0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000         8

The file size and mem size for the RW section is almost identical.

The race runtime syso file is correct, our linker somehow failed to put the bss in real bss.

   text    data     bss     dec     hex filename
 159066     484 9971272 10130822         9a9586 src/runtime/race/race_linux_amd64.syso

Should we also fix this for 1.4.1? Every race enabled binary is 10MB larger than it should be right now.
@minux
Copy link
Member Author

minux commented Dec 17, 2014

Fix is easy:

diff --git a/src/cmd/ld/ldelf.c b/src/cmd/ld/ldelf.c
index 243c8d8..4e87943 100644
--- a/src/cmd/ld/ldelf.c
+++ b/src/cmd/ld/ldelf.c
@@ -545,7 +545,10 @@ ldelf(Biobuf *f, char *pkg, int64 len, char *pn)
                        s->type = SRODATA;
                        break;
                case ElfSectFlagAlloc + ElfSectFlagWrite:
-                       s->type = SNOPTRDATA;
+                       if(sect->type == ElfSectNobits)
+                               s->type = SNOPTRBSS;
+                       else
+                               s->type = SNOPTRDATA;
                        break;
                case ElfSectFlagAlloc + ElfSectFlagExec:
                        s->type = STEXT;

Candidate for 1.4.1?

@ianlancetaylor ianlancetaylor added this to the Go1.4.1 milestone Dec 17, 2014
@minux
Copy link
Member Author

minux commented Dec 17, 2014

Turns out this bug is present ever since Go supports loading ELF files.
We didn't notice that because before 1.4, the race runtime doesn't
use that much BSS (up to 1.3.3, it only uses ~72KB of BSS).

Anyway, I believe this still is a 1.4 regression without any meaningful
workaround.

@davecheney
Copy link
Contributor

That would explain why the release $GOROOT/bin/go is double the size of a
locally compiled version

lucky(/tmp/go/bin) % ls -l go ~/go/bin/go
-rwxr-xr-x 1 dfc dfc 21435888 Dec 11 12:20 go         // from golang.org
-rwxr-xr-x 1 dfc dfc  9746184 Dec 16 21:05 /home/dfc/go/bin/go  // compiled
locally

@minux minux self-assigned this Dec 18, 2014
@minux
Copy link
Member Author

minux commented Dec 21, 2014

mailed https://golang.org/cl/1889

@minux minux closed this as completed in 1c0c611 Dec 26, 2014
minux added a commit that referenced this issue Jan 14, 2015
@minux @rsc
[release-branch.go1.4] cmd/ld: put .bss from external objects into re…
ff2ab29 
…al .bss section

Fixes #9359.

Change-Id: Iba62935b5a14de23d914f433a09a40417d7e88ed
Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/1889
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 1c0c611)
Reviewed-on: https://go-review.googlesource.com/2802
Reviewed-by: Andrew Gerrand <adg@golang.org>
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@minux @wheatman
[release-branch.go1.4] cmd/ld: put .bss from external objects into re…
6a50a64 
…al .bss section

Fixes golang#9359.

Change-Id: Iba62935b5a14de23d914f433a09a40417d7e88ed
Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/1889
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 1c0c611)
Reviewed-on: https://go-review.googlesource.com/2802
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
@minux @wheatman
[release-branch.go1.4] cmd/ld: put .bss from external objects into re…
8d7158d 
…al .bss section

Fixes golang#9359.

Change-Id: Iba62935b5a14de23d914f433a09a40417d7e88ed
Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/1889
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 1c0c611)
Reviewed-on: https://go-review.googlesource.com/2802
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@minux @wheatman
[release-branch.go1.4] cmd/ld: put .bss from external objects into re…
dbddaa8 
…al .bss section

Fixes golang#9359.

Change-Id: Iba62935b5a14de23d914f433a09a40417d7e88ed
Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/1889
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 1c0c611)
Reviewed-on: https://go-review.googlesource.com/2802
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 20, 2018
@minux @wheatman
[release-branch.go1.4] cmd/ld: put .bss from external objects into re…
464607e 
…al .bss section

Fixes golang#9359.

Change-Id: Iba62935b5a14de23d914f433a09a40417d7e88ed
Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/1889
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 1c0c611)
Reviewed-on: https://go-review.googlesource.com/2802
Reviewed-by: Andrew Gerrand <adg@golang.org>
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
@minux @wheatman
[release-branch.go1.4] cmd/ld: put .bss from external objects into re…
70d24f6 
…al .bss section

Fixes golang#9359.

Change-Id: Iba62935b5a14de23d914f433a09a40417d7e88ed
Signed-off-by: Shenghou Ma <minux@golang.org>
Reviewed-on: https://go-review.googlesource.com/1889
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit 1c0c611)
Reviewed-on: https://go-review.googlesource.com/2802
Reviewed-by: Andrew Gerrand <adg@golang.org>
@rsc rsc unassigned minux Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4.1
Development

No branches or pull requests
4 participants
@davecheney @minux @ianlancetaylor @gopherbot