[PATCH] D48406: [ELF] Make non-writable non-executable PROGBITS sections closer to .text
Rui Ueyama via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 26 21:03:12 PDT 2018
ruiu added a comment.
LGTM with these changes.
================
Comment at: lld/trunk/ELF/Writer.cpp:704
RF_EXEC = 1 << 13,
- RF_NON_TLS_BSS = 1 << 12,
- RF_NON_TLS_BSS_RO = 1 << 11,
- RF_NOT_TLS = 1 << 10,
- RF_ALLOC_FIRST = 1 << 9,
+ RF_PROGBITS_NOT_EXEC_OR_WRITE = 1 << 12,
+ RF_NON_TLS_BSS = 1 << 11,
----------------
I'd name this RF_RODATA for conciseness.
================
Comment at: lld/trunk/ELF/Writer.cpp:760-766
Rank |= RF_WRITE;
+ // Make non-executable and non-writable PROGBITS sections (e.g .rodata
+ // .eh_frame) closer to .text . They likely contain PC or GOT relative
+ // relocations and there could be relocation overflow if other huge sections
+ // (.dynstr .dynsym) were placed in between.
+ else if (Sec->Type == SHT_PROGBITS)
+ Rank |= RF_PROGBITS_NOT_EXEC_OR_WRITE;
----------------
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) {
...
}
================
Comment at: lld/trunk/ELF/Writer.cpp:762
+ // Make non-executable and non-writable PROGBITS sections (e.g .rodata
+ // .eh_frame) closer to .text . They likely contain PC or GOT relative
+ // relocations and there could be relocation overflow if other huge sections
----------------
space before `.`
Repository:
rL LLVM
https://reviews.llvm.org/D48406
More information about the llvm-commits
mailing list