This is an archive of the discontinued LLVM Phabricator instance.

[LinkerScript] Support EXCLUDE_FILE inside KEEP
ClosedPublic

Authored by davide on Jul 25 2016, 7:03 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

davide updated this revision to Diff 65461.Jul 25 2016, 7:03 PM
davide retitled this revision from to [LinkerScript] Support EXCLUDE_FILE inside KEEP.
davide updated this object.
davide added reviewers: ruiu, evgeny777.
davide added subscribers: llvm-commits, grimar.

Sorry, I updated a stale version of the tests. I'll update a new version of the tests soon.

ruiu added inline comments.Jul 25 2016, 7:33 PM
ELF/LinkerScript.cpp
82 ↗(On Diff #65461)

Maybe we should return std::vector<std::pair<StringRef, InputSectionDescription *>> instead.

99–100 ↗(On Diff #65461)

I'd pass InputSectionDescription * instead.

686 ↗(On Diff #65461)

The grammar of KEEP is

KEEP := "KEEP" "(" FILE_PATTERN ")"
FILE_PATTERN := GLOB+ "(" "EXCLUDE_FILE" "(" GLOB+ ")" GLOB+ ")"
              |  GLOB+ "(" GLOB+ ")" GLOB+ ")"

where GLOB is any glob pattern. FILE_PATTERN is the same as one of the SECTIONS subcommands.

So I think you want to define readFilePattern and use it both from this function and from readOutputSectionDescription.

694–695 ↗(On Diff #65461)
if (skip("EXCLUDE_FILE")) {
davide updated this revision to Diff 65637.Jul 26 2016, 5:22 PM

Rui's comments.

ruiu added inline comments.Jul 26 2016, 5:29 PM
ELF/LinkerScript.cpp
670 ↗(On Diff #65637)

How about changing the return type of the function to InputSectionDescription * so that you don't need to pass OutputSectionCommand * to this function?

681 ↗(On Diff #65637)

KeptSections shouldn't be updated in this function because this function should be agnostic of the context.

694 ↗(On Diff #65637)

If you change the return type, then you can write

InputSectionCommand *InCmd = readFilePattern();
Opt.KeptSections.insert(Opt.KeptSections.end(), InCmd.Patterns.begin(), InCmd.Patterns.end());
Cmd.emplace_back(InCmd);
davide added inline comments.Jul 26 2016, 5:49 PM
ELF/LinkerScript.cpp
681 ↗(On Diff #65637)

So maybe we can pass an argument std::vector<StringRef> to collect the list of tokens and use that in readKeep() ?

ruiu added inline comments.Jul 26 2016, 5:52 PM
ELF/LinkerScript.cpp
681 ↗(On Diff #65637)

File pattern consists of file names followed by section names, so std::vector<StringRef> is not enough because it is unstructured. Did you read my comment at line 694? Didn't it work?

davide updated this revision to Diff 65641.Jul 26 2016, 6:34 PM
ruiu accepted this revision.Jul 26 2016, 6:37 PM
ruiu edited edge metadata.

LGTM with a nit.

ELF/LinkerScript.cpp
694 ↗(On Diff #65641)

push_back(std::move(InCmd))

This revision is now accepted and ready to land.Jul 26 2016, 6:37 PM
This revision was automatically updated to reflect the committed changes.
grimar added inline comments.Jul 27 2016, 12:16 AM
lld/trunk/test/ELF/linkerscript/linkerscript-excludefile.s
8

excessive space at the end

14

I think you want to use CHECK-NEXT here.

32

Ditto.