This is an archive of the discontinued LLVM Phabricator instance.

Implement --just-symbols
AbandonedPublic

Authored by PkmX on Dec 15 2017, 1:07 AM.

Details

Summary

This is based on D39348, but adds a new kind of input file JustSymbolsFile which is derived from ELFFileBase. The argument is treated as a normal input file so the case where it is the sole input works and we get to reuse some of code from ELFFileBase.

Diff Detail

Event Timeline

PkmX created this revision.Dec 15 2017, 1:07 AM
ruiu added a comment.Dec 15 2017, 4:31 PM

Sorry I missed this one.

ELF/Driver.cpp
183

You can just call Files.push_back(createJustSymbolsFile(MBRef)) instead of calling addFile.

ELF/Driver.h
30 ↗(On Diff #127076)

I don't like an optional argument in general. Please avoid using it in lld.

ELF/InputFiles.h
337

Remove InputFile::.

ELF/SymbolTable.cpp
77

Obviously you need a comment just like other code blocks.

PkmX marked 3 inline comments as done.Dec 15 2017, 8:24 PM
PkmX added inline comments.
ELF/Driver.cpp
183

You are right. In retrospect I should've just call that instead of adding an extra argument to addFile.

ELF/InputFiles.h
337

The InputFile:: is required due to the rules of two-phase lookup in C++.

PkmX marked an inline comment as done.Dec 15 2017, 8:26 PM
PkmX updated this revision to Diff 127236.Dec 15 2017, 8:29 PM
PkmX updated this revision to Diff 127237.Dec 15 2017, 8:42 PM
  • Copy symbol type, size, binding, etc, like D39348.
  • Update test cases.
PkmX updated this revision to Diff 127465.Dec 18 2017, 11:13 PM

Added a check to only allow ET_EXEC and the corresponding test.

On Tue, Dec 19, 2017 at 9:54 AM, Rafael Avila de Espindola <rafael.espindola@gmail.com> wrote:

This patch allows --just-symbols to be a ET_DYN, ET_REL or ET_EXEC. Do
you need all 3? ET_REL in particular seems problematic since st_value
has a different meaning in relocatable files.

If you don't need ET_DYN or ET_REL, please reject them. If you need
them, please add a test.

ld.bfd accepts ET_EXEC and ET_REL but not ET_DYN. However I don't think this option makes much sense for ET_REL inputs so I'm restricting it to ET_EXEC for now.

PkmX updated this revision to Diff 127468.Dec 19 2017, 12:25 AM

Implement the -R option as an alias to either --rpath or --just-symbols. It is treated as latter if the argument file exists and is not a directory.

This works fine for me. Perhaps Rui should land D39348 first and then you can rebase this in terms of that to discuss the remaining features (-R alias, no input files case).

PkmX abandoned this revision.Aug 7 2018, 11:17 PM

Abandoning as this has been implemented by another commit.