This is an archive of the discontinued LLVM Phabricator instance.

ELF: Create synthetic sections for loadable partitions.
ClosedPublic

Authored by pcc on May 23 2019, 2:54 PM.

Details

Summary

We create several types of synthetic sections for loadable partitions, including:

  • The dynamic symbol table. This allows code outside of the loadable partitions to find entry points with dlsym.
  • Creating a dynamic symbol table also requires the creation of several other synthetic sections for the partition, such as the dynamic table and hash table sections.
  • The partition's ELF header is represented as a synthetic section in the combined output file, and will be used by llvm-objcopy to extract partitions.

Depends on D60353

Depends on D62349

Event Timeline

pcc created this revision.May 23 2019, 2:54 PM
grimar added inline comments.May 27 2019, 4:01 AM
lld/ELF/SyntheticSections.h
1128

What about having one more global pointer instead.
i.e. after creating all of the partitions do:

Partition *InM; //global, like `In`

...

InM = &Partitions[0];

And then use InM everywhere instead of mainPartition.
I wonder what others think though.

ruiu added inline comments.May 28 2019, 7:11 AM
lld/ELF/SyntheticSections.h
1128

Yeah. Looks like this function doesn't have to be a function. I thought for a while about what kind of name I'd choose, but I didn't come up with a good idea. Calling Partitions[0] as In might not be a bad idea. It's short, and we don't need to update all Ins with mainPartition() which makes this patch much easier to read.

pcc marked an inline comment as done.May 28 2019, 11:39 AM
pcc added inline comments.
lld/ELF/SyntheticSections.h
1128

It won't work to name it In because we already have a variable named In. I think that we'll always have a variable for per-file synthetic sections because some sections (such as the static symbol table) will not be duplicated between partitions.

In an earlier version of this change I had a global variable

static Partition &Main = Partitions[0];

but this won't work now that Partitions is a vector and not an array. That's why I introduced the function.

I'm fine with the idea of bringing the variable back (as a pointer rather than a reference) and setting it once all partitions are created. I'll call it Main unless there are any strong objections to that name.

pcc updated this revision to Diff 201800.May 28 2019, 5:34 PM
  • Replace mainPartition() with a global variable Main
pcc updated this revision to Diff 203070.Jun 4 2019, 9:11 PM
pcc marked an inline comment as done.
  • Write tests, fix bugs found while writing tests
pcc retitled this revision from [wip] ELF: Create synthetic sections for loadable partitions. to ELF: Create synthetic sections for loadable partitions..Jun 4 2019, 9:12 PM
pcc edited the summary of this revision. (Show Details)

Okay, I *think* I managed to write tests that cover everything that I added here, but let me know if I missed anything.

grimar added a comment.Jun 5 2019, 1:03 AM

I'd suggest adding a short description to each test case added.

lld/test/ELF/partition-synthetic-sections.s
17

Was issue about that reported? We have a good practice in LLVM tools to add an URL of the issue
for each TODO in the test. That helps to cleanup such things a lot.

23

You do not need [ 1] I think.

pcc updated this revision to Diff 203264.Jun 5 2019, 3:53 PM
pcc marked 2 inline comments as done.
  • Address review comments
pcc added inline comments.Jun 5 2019, 3:57 PM
lld/test/ELF/partition-synthetic-sections.s
17

I looked into this a bit more and filed pr42145 and linked it here. Unfortunately fixing this isn't as simple as I first thought.

23

I was using it to make sure that there were no section headers before .dynsym. But I've switched to using CHECK-NEXT here like I was doing elsewhere.

ruiu accepted this revision.Jun 7 2019, 5:02 AM

LGTM

It generally looking good, and I don't have further comments. George, are you happy with this?

This revision is now accepted and ready to land.Jun 7 2019, 5:02 AM
grimar accepted this revision.Jun 7 2019, 5:04 AM

Yep, looks fine.

This revision was automatically updated to reflect the committed changes.