Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(204)

Issue 2587041: code review 2587041: 5l, 6l, 8l: link pclntab and symtab as ordinary rodata ... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 5 months ago by rsc
Modified:
14 years, 5 months ago
Reviewers:
ken3
CC:
ken2, golang-dev
Visibility:
Public.

Description

5l, 6l, 8l: link pclntab and symtab as ordinary rodata symbols That is, move the pc/ln table and the symbol table into the read-only data segment. This eliminates the need for a special load command to map the symbol table into memory, which makes the information available on systems that couldn't handle the magic load to 0x99000000, like NaCl and ARM QEMU and Linux without config_highmem=y. It also eliminates an #ifdef and some clumsy code to find the symbol table on Windows. The bad news is that the binary appears to be bigger than it used to be. This is not actually the case, though: the same amount of data is being mapped into memory as before, and the tables are still read-only, so they're still shared across multiple instances of the binary as they were before. The difference is just that the tables aren't squirreled away in some section that "size" doesn't know to look at. This is a checkpoint. It probably breaks Windows and breaks NaCl more than it used to be broken, but those will be fixed. The logic involving -s needs to be revisited too. Fixes issue 871.

Patch Set 1 #

Patch Set 2 : code review 2587041: 5l, 6l, 8l: link pclntab and symtab as ordinary rodata ... #

Patch Set 3 : code review 2587041: 5l, 6l, 8l: link pclntab and symtab as ordinary rodata ... #

Unified diffs Side-by-side diffs Delta from patch set Stats (+551 lines, -866 lines) Patch
M src/cmd/5l/asm.c View 8 chunks +68 lines, -51 lines 0 comments Download
M src/cmd/5l/l.h View 4 chunks +1 line, -4 lines 0 comments Download
M src/cmd/5l/obj.c View 2 chunks +2 lines, -1 line 0 comments Download
M src/cmd/5l/span.c View 3 chunks +4 lines, -2 lines 0 comments Download
R src/cmd/5l/symtab.c View 1 chunk +0 lines, -153 lines 0 comments Download
M src/cmd/6l/asm.c View 8 chunks +63 lines, -39 lines 0 comments Download
M src/cmd/6l/l.h View 3 chunks +1 line, -2 lines 0 comments Download
M src/cmd/6l/list.c View 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/6l/obj.c View 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/6l/span.c View 3 chunks +12 lines, -21 lines 0 comments Download
R src/cmd/6l/symtab.c View 1 chunk +0 lines, -235 lines 0 comments Download
M src/cmd/8l/asm.c View 7 chunks +68 lines, -53 lines 0 comments Download
M src/cmd/8l/l.h View 3 chunks +1 line, -2 lines 0 comments Download
M src/cmd/8l/list.c View 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/8l/obj.c View 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/8l/span.c View 3 chunks +12 lines, -21 lines 0 comments Download
R src/cmd/8l/symtab.c View 1 chunk +0 lines, -167 lines 0 comments Download
M src/cmd/ld/data.c View 3 chunks +7 lines, -5 lines 0 comments Download
M src/cmd/ld/lib.h View 2 chunks +6 lines, -0 lines 0 comments Download
M src/cmd/ld/lib.c View 5 chunks +29 lines, -12 lines 0 comments Download
M src/cmd/ld/macho.h View 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/ld/macho.c View 2 chunks +8 lines, -12 lines 0 comments Download
M src/cmd/ld/pe.c View 1 chunk +0 lines, -7 lines 0 comments Download
A src/cmd/ld/symtab.c View 1 chunk +220 lines, -0 lines 0 comments Download
M src/libmach/executable.c View 12 chunks +27 lines, -29 lines 0 comments Download
M src/pkg/runtime/symtab.c View 5 chunks +15 lines, -27 lines 0 comments Download
M src/pkg/runtime/windows/thread.c View 3 chunks +0 lines, -20 lines 0 comments Download

Messages

Total messages: 3
rsc
Hello ken2 (cc: golang-dev@googlegroups.com), I'd like you to review this change.
14 years, 5 months ago (2010-10-19 22:07:16 UTC) #1
rsc
*** Submitted as 52618323fda8 *** 5l, 6l, 8l: link pclntab and symtab as ordinary rodata ...
14 years, 5 months ago (2010-10-19 22:07:26 UTC) #2
ken3
14 years, 5 months ago (2010-10-20 22:35:07 UTC) #3
lgtm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b