This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Linkerscript: implemented filename specification.
ClosedPublic

Authored by grimar on Jul 27 2016, 4:31 AM.

Details

Summary

Scripts can contain something like:
KEEP (*crtbegin.o(.ctors))

What means that "*crtbegin.o" is a wildcard of file to take the sections from.
This is some kind of opposite to EXCLUDE_FILE and used in FreeBSD script:
https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?revision=284870&view=markup#l122

Patch implements this.

Patch partially relies on changes from D22749 that changed the way of parsing
LS script a bit. So I would suggest to start reviewing from D22749 and
then I can rebase this one.

Diff Detail

Event Timeline

grimar updated this revision to Diff 65704.Jul 27 2016, 4:31 AM
grimar retitled this revision from to [ELF] - Linkerscript: implemented filename specification..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, davide and 2 others.
ruiu added inline comments.Jul 27 2016, 3:06 PM
ELF/LinkerScript.cpp
7–1

This name is no longer correct. Input section description is everything that is described in https://sourceware.org/binutils/docs/ld/Input-Section.html. So it includes KEEP. You got to give it a new name.

8–9

Pass InputSectionDescription instead of the two arguments.

9

This name seems odd. Everything is named readXXX where XXX is a name which the function is expected to read. This function name is not consistent.

So this function reads XXX where XXX appears

*(XXX)
KEEP(*(XXX))
EXCLUDED_FILES(foo.o, XXX)

right? Maybe I'd call it readInputFilePattern.

ELF/LinkerScript.h
2

I'd name FilePattern and rename Patterns SectionPatterns.

grimar updated this revision to Diff 65891.Jul 28 2016, 2:09 AM
  • Addressed review comments + refactored.
ELF/LinkerScript.cpp
7–1

Now it includes KEEP.

8–9

Done.

9

No. It read everything except KEEP which I thought would be reasonable to leave at higher level as everyhing in input section description can be surrounded by keep, so readInputFilePattern would not work I think.
I refactored that place.

ELF/LinkerScript.h
2

Done.

ruiu added inline comments.Jul 28 2016, 12:19 PM
ELF/LinkerScript.cpp
96

I think we used filename as one word in other places, so Filename.

109–111

Flip the condition and remove continue.

447

This looks weird. If this function reads an input section description, why do we have to pass an input section description to the function? If we can pass an input section description to the function, it implies that we have already read an input section description.

742–744

Huh, so you created an empty input section description and pass it to the function. You want to make readInputSectionDescription to return a new InputSectionDescription instead.

grimar updated this revision to Diff 66014.Jul 28 2016, 2:46 PM
  • Addressed review comments.
ELF/LinkerScript.cpp
96

Done.

109–111

Done.

447

Fixed.

742–744

Done.

ruiu accepted this revision.Jul 28 2016, 2:54 PM
ruiu edited edge metadata.

LGTM

ELF/LinkerScript.cpp
696

Use auto.

704–706
} else {
  readInpu... 
}
742–744
Cmd->Command.push_back(readInputSectionDescription());
This revision is now accepted and ready to land.Jul 28 2016, 2:54 PM
This revision was automatically updated to reflect the committed changes.