This is an archive of the discontinued LLVM Phabricator instance.

[ELF][X86] Allow PT_LOAD to have overlapping p_offset ranges on EM_X86_64
ClosedPublic

Authored by MaskRay on Sep 12 2019, 1:37 AM.

Event Timeline

MaskRay created this revision.Sep 12 2019, 1:37 AM
Herald added a project: Restricted Project. · View Herald Transcript

This patch is the (awful) reason I posted D67325 before this one. 🤔

The dependency order D67324 (omagic & nmagic) -> D67325 (image base) -> D67482 (EM_X86_64) -> D67481 (-z separate-segments) will cause the least number of redundantly updated tests. Say, I do D67482 before D67325, the following tests will be updated redundantly.

Failing Tests (6):                                                                                              
    lld :: ELF/avoid-empty-program-headers.s                                                                    
    lld :: ELF/common-page.s                                                                                    
    lld :: ELF/gnu-ifunc-empty.s                                                                               
    lld :: ELF/image-base.s                                                                                    
    lld :: ELF/linkerscript/page-size.s              
    lld :: ELF/linkerscript/symbol-reserved.s
``` 🤔

Just a quick, no objections from me. I'll leave the decision to go ahead to people most concerned and knowledgeable about x86_64.

grimar added a comment.EditedSep 12 2019, 2:16 AM

I also think it is OK. I haven't checked all the math, seems there are problems with it in some places.
But seems we never cared too much about it anyways.

test/ELF/x86-64-tls-ie.s
33
= 2023C0
34
= 2023C8
35
= 2023C8
test/ELF/x86-64-tlsdesc-ld.s
17
= 4273
MaskRay marked 5 inline comments as done.EditedSep 15 2019, 11:48 PM

Thanks for the math suggestion. Will push shortly. We should soon get rid of the bool enabled variable in Writer<ELFT>::fixSectionAlignments().

test/ELF/x86-64-tls-ie.s
33

It should be 0x2023C8. It is another use of tls0.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 16 2019, 12:06 AM
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.