This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Make non-writable non-executable PROGBITS sections closer to .text
ClosedPublic

Authored by MaskRay on Jun 20 2018, 4:26 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

MaskRay created this revision.Jun 20 2018, 4:26 PM
MaskRay added a comment.EditedJun 20 2018, 5:02 PM

The idea is that we should place .text in the middle and huge data sections on both sides:

The desired order is roughly:

.dynstr / .dymstr
.rodata .eh_frame .eh_frame_hdr  # they can be huge and we don't want them placed between .text and .bss
# preferably .rodata is immediately before .text 
.text
.data
.bss

Effect:

[Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
[ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
[ 1] .interp           PROGBITS        0000000000200270 000270 00001c 00   A  0   0  1
[ 2] .dynsym           DYNSYM          0000000000200290 000290 0000c0 18   A  7   1  8
[ 3] .gnu.version      VERSYM          0000000000200350 000350 000010 02   A  2   0  2
[ 4] .gnu.version_r    VERNEED         0000000000200360 000360 000020 00   A  7   1  4
[ 5] .gnu.hash         GNU_HASH        0000000000200380 000380 000020 00   A  2   0  8
[ 6] .hash             HASH            00000000002003a0 0003a0 000048 04   A  2   0  4
[ 7] .dynstr           STRTAB          00000000002003e8 0003e8 000097 00   A  0   0  1
[ 8] .rela.dyn         RELA            0000000000200480 000480 000048 18   A  2   0  8
[ 9] .rela.plt         RELA            00000000002004c8 0004c8 000030 18   A  2   0  8
[10] .note.ABI-tag     NOTE            00000000002004f8 0004f8 000020 00   A  0   0  4
[11] .note.gnu.property NOTE            0000000000200518 000518 000040 00   A  0   0  8
[12] .rodata           PROGBITS        0000000000200560 000560 000403 00 AMS  0   0 16
[13] .eh_frame_hdr     PROGBITS        0000000000200964 000964 00002c 00   A  0   0  4
[14] .eh_frame         PROGBITS        0000000000200990 000990 0000cc 00   A  0   0  8
[15] .text             PROGBITS        0000000000201000 001000 0001a2 00  AX  0   0 16
[16] .init             PROGBITS        00000000002011a4 0011a4 000017 00  AX  0   0  4
[17] .fini             PROGBITS        00000000002011bc 0011bc 000009 00  AX  0   0  4
[18] .plt              PROGBITS        00000000002011d0 0011d0 000030 00  AX  0   0 16
[19] .data             PROGBITS        0000000000202000 002000 000010 00  WA  0   0  8
[20] .tm_clone_table   PROGBITS        0000000000202010 002010 000000 00  WA  0   0  8
[21] .got.plt          PROGBITS        0000000000202010 002010 000028 00  WA  0   0  8
[22] .fini_array       FINI_ARRAY      0000000000203000 003000 000008 08  WA  0   0  8
[23] .init_array       INIT_ARRAY      0000000000203008 003008 000008 08  WA  0   0  8
[24] .dynamic          DYNAMIC         0000000000203010 003010 0001a0 10  WA  7   0  8
[25] .got              PROGBITS        00000000002031b0 0031b0 000010 00  WA  0   0  8
[26] .bss              NOBITS          0000000000204000 0031c0 0003f8 00  WA  0   0 16
[27] .comment          PROGBITS        0000000000000000 0031c0 0000aa 01  MS  0   0  1
[28] .symtab           SYMTAB          0000000000000000 003270 000378 18     30  19  8
[29] .shstrtab         STRTAB          0000000000000000 0035e8 00011e 00      0   0  1
[30] .strtab           STRTAB          0000000000000000 003706 0001fb 00      0   0  1

The Flg Type columns look better and we can remove that DYNSTR DYNSYM special case.

There are 60+ tests failing because of changed addresses and I'll fix them. Just sending it for review first so that I don't need to change the tests back and forth...

echristo accepted this revision.Jun 21 2018, 11:17 AM

OK, this ranking looks good to me. Go ahead and put your long comment with the layout into your commit message and do fix up the tests.

-eric

This revision is now accepted and ready to land.Jun 21 2018, 11:17 AM
[12] .rodata           PROGBITS        0000000000200560 000560 000403 00 AMS  0   0 16
[13] .eh_frame_hdr     PROGBITS        0000000000200964 000964 00002c 00   A  0   0  4
[14] .eh_frame         PROGBITS        0000000000200990 000990 0000cc 00   A  0   0  8
[15] .text             PROGBITS        0000000000201000 001000 0001a2 00  AX  0   0 16

I want to make .rodata and .text back-to-back, but not sure how to make such heuristic more general...

MaskRay updated this revision to Diff 152978.Jun 26 2018, 3:11 PM

Update all tests.

This revision was automatically updated to reflect the committed changes.
ruiu added a comment.Jun 26 2018, 9:03 PM

LGTM with these changes.

lld/trunk/ELF/Writer.cpp
704

I'd name this RF_RODATA for conciseness.

760–766

nit: I'd avoid writing a long comment before else if because it visually split a consecutive if ~ else if ~ else expressions. This is probably better:

if (IsWriter) {
  ...
} else if (...) {
  // your comment here
  your code here
}

That said, the best way to arrange this particular piece code is to replace else { if with else if like this

if (IsExec) {
  ...
} else if (IsWriter) {
  ...
} else if (Sec->Type == SHT_PROGBITS) {
  ...
}
762

space before .

MaskRay marked 3 inline comments as done.Jun 27 2018, 9:04 AM
MaskRay added inline comments.
lld/trunk/ELF/Writer.cpp
760–766

Done in r335743

762

I tend to avoid immediate trailing dot when the last word is an identifier (OT: when I double click it, the trailing dot will be a part of the selection) or URL (slight OT: trailing punctuation is not friendly to some terminals). That said, I don't have a strong preference here so I'll follow yours.

lld/trunk/test/ELF/basic-ppc64.s