[PATCH] D58426: llvm-objcopy: Change sectionWithinSegment() to use virtual addresses instead of file offsets.

[bd1976 llvm via llvm-commits]

Relevant ELF list post:
https://groups.google.com/d/msg/generic-abi/VV3kzhvrRdU/BNkq1K__CQAJ

On Tue, Mar 19, 2019 at 9:46 PM Jake Ehrlich via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> jakehehrlich added a comment.
>
> If you think about TLS as being in a sort of different address space
> (which was not a way I had thought about this before) then this is the
> function that determines if a section is in the same address area as a
> segment so its a semantically consistent idea I think. Layout should also
> always succeed because the PT_TLS will be given the respective PT_LOAD as a
> parent which will fix its offset correctly and then any sections assigned
> only to PT_TLS and not a PT_LOAD will still be laid out correctly (which
> should always be the case for any overlapping segment, we should have tests
> for this already). We are however changing what this function means even if
> in practice it's true/false in almost all of the same cases. For non-NOBITS
> `Segment.VAddr - Section.Addr == Segment.Offset - Section.OriginalOffset`
> and those sections will never fall in the range past `Segment.FileSize` so
> the predicate should return true in all the same cases for non-NOBITS. It
> would also now assign sections to the "expected" segment even if today that
> has no affect one way or the other.
>
> Things get more tricky if you change segmentOverlapsSegment to behave that
> way. In that case its critical that PT_TLS have a PT_LOAD as its parent.
> The subtle reason behind why we use VAddr in one case and Offset in another
> is going to be confusing to explain to people (as if the reason we used
> Offset and not VAddr wasn't already complicated enough)
>
> I was going to point the non-allocated section issue out as well but it
> seems you've already seen that. Since non-allocate sections don't have
> meaningful vaddrs this is tricky. Independent of this change as it exists
> right now I am 100% ok with using Offset/FileSize for most sections and
> using VAddr/MemSize for NOBITS (since in principal NOBITS should be allowed
> to have any offset). I think that would be a better heuristic for assigning
> NOBITS sections to segments. It should be the case that weather or not a
> NOBITS section is assigned to a segment, layout should not be affected
> anyhow. This seems like a less semantically consistent operation however.
> As the code exists today it assigns a size of 1 to NOBITS so they always
> wind up assigned to the wrong PT_LOAD if they wind up assigned at all. This
> only matters in so far as the offset is a bit wonky but this causes no
> general problem since the offset of a NOBITS section is meaningless (though
> an expected answer exists and we should match it).
>
> An alternative is to do this change on allocated sections (e.g.
> vaddr/memsize and account for TLS), and then to use offset/filesize on
> non-allocated sections. I think this would solve the issues being seen in
> D59293 <https://reviews.llvm.org/D59293> and would always use the
> meaningful offset unless a non-allocated NOBITS section existed...but such
> a thing should just be assumed to never exist as part of a segment I think.
>
> I still however maintain that if you're relaying on what segment a NOBITS
> section falls into using ParentSegment then you're doing something wrong.
> That said there might be an offset bug we should fix for expectation's
> sake. If we're only trying to fix the offset bug for expectations sake then
> I support using VAddr/MemSize for NOBITS only as the correct solution.
> Could you describe your use case a bit more perhaps?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D58426/new/
>
> https://reviews.llvm.org/D58426
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190320/44141ece/attachment.html>