[PATCH] D33964: [LLVM][llvm-objcopy] Added basic plumbing to get things started

Tue Jul 11 11:19:02 PDT 2017

jakehehrlich added a comment.

In https://reviews.llvm.org/D33964#804772, @jhenderson wrote:

> I'm ok with that as well, but I'd like to understand what your plan is going forward to fix this, in case it has a significant impact on the design.
>
> Having a progbits section in multiple segments is absolutely fine, and I'd expect the layout to be maintained as in the diagram. The alignment of all containing segments needs to be respected when placing the first section, and then they all start at the same place (or in the case where the nested segment is not at the start, at the same relative offset). There are some weird oddities when coming to NOBITS sections, because of their file/memory footprint mismatch. The simple, but probably undesirable, option is to allocate a file footprint for any NOBITS sections that are not at the end of the top-most parent segment. I think though that actually, as is the case in the diagram here, that there is special handling for nested NOBITS sections like this in loaders (specifically, I'm thinking of the .tbss case again - the example could easily be a data segment with a nested TLS segment, with the .tdata, .tbss, and .data sections). By my understanding, as long as .tbss is at the end of the nested TLS segment, it should not have a file footprint, even though it makes it look like there'll be a mismatch in the data segment.

Well if this is in fact the intended behavior then I think that means I have to switch from "respect the memory image" to "respect the file image". If the relative offsets are used instead of relative addresses it should solve this problem with only a tiny change. If I stick to "respect the memory image" then I'd have to add all sorts of complicated book keeping to ensure that this case was handled correctly. Even if TLS isn't handled this way there might be some case like this where it is and we should respect the choice that the file made. I think this case (or even the possibility of such a case existing) convinces me that it should be "respect the file image" and not "respect the memory image".

So to handle this I propose the following changes:

1. Make the parent segment the lowest offset segment. I think I'd prefer "lowest offset segment" rather than "top segment" in order to account for strange things like overlapping segments in case those are also a thing. I'm slowly coming to realize that it's best to curtail my assumptions as much as possible.
2. Change "Offset = FirstInSeg->Offset + Section->Addr - FirstInSeg->Addr;" to "Offset = FirstInSeg->Offset + Section->OriginalOffset - FirstInSeg->OriginalOffset;" because of the TLS case you found. This way we respect the choice the input file made on this mater regardless of weather it allocates space in the file for the NOBITS section or not.
3. Add a parent segment field to segments so that nested segments (and even overlapping segments) can have offsets based on their parent segment and not the sections they contain.

Does this sound good? If so I'll go ahead and change this diff to use 1) and 2) now and I'll add 3) in another diff. Even with just 1) and 2) I think this should still work in the specific case you requested a test on but I'll wait to add that test when I add 3) I think.

================
Comment at: test/tools/llvm-objcopy/hello-world.s:2
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: llvm-objcopy %t %t2
----------------
jhenderson wrote:
> jakehehrlich wrote:
> > jhenderson wrote:
> > > I don't think this will work for many people, as LLD is not part of LLVM's main source code body, so people will be unable to run this test. That being said, I'm not sure how to work around this. Can yaml2obj help us here? (I'm not particularly familiar with that tool yet).
> > Oh man, that didn't even occur to me. As far as I am aware there is no way to produce program headers using yaml2obj. It never produces any program headers by default either. I think you're right and this should be fixed. I have no clue how to fix it other than adding support for program headers to yaml2obj. That's not something I've looked into however.
> > 
> > Perhaps there is a way to require lld for this test? That way it's just unsupported for most people?
> I'm afraid I don't know of a way. There might be, but if there is, somebody else will have to advise.
> 
> It seems to me that having program header support in yaml2obj is very desirable. I'd offer to add it, but I do not have any time to do so.
I'm adding support for program headers to yaml2obj now. Hopefully I can land that change soon.

Repository:
  rL LLVM

https://reviews.llvm.org/D33964