This is an archive of the discontinued LLVM Phabricator instance.

[Object/ELF] Provide symbol helpers
ClosedPublic

Authored by davide on Jun 4 2015, 5:32 PM.

Details

Summary

These were in lld. They can be reused for other tools, e.g. llvm-readobj.
If needed, we can introduce another class as COFF does (i.e. COFFSymboRef).
Note: Hexagon has a different notion of common symbol I'm not entirely sure I understand.
I wasn't able to find documentation either, so any help on this side is greatly appreciated.

Diff Detail

Event Timeline

davide updated this revision to Diff 27166.Jun 4 2015, 5:32 PM
davide retitled this revision from to [Object/ELF] Provide symbol helpers.
davide updated this object.
davide edited the test plan for this revision. (Show Details)
davide set the repository for this revision to rL LLVM.
davide added a subscriber: Unknown Object (MLST).
rafael added inline comments.Jun 5 2015, 6:20 AM
include/llvm/Object/ELF.h
1030 ↗(On Diff #27166)

You don't need the type, just she st_shndx.

1033 ↗(On Diff #27166)

This is scary and should probably not be in the common function.

These values can be reused by other architectures to mean something else, no? So they can only be checked in a hexagon only path.

1042 ↗(On Diff #27166)

You only need to check the st_shndx.

My suggestion would be to have predicates that cover all values:

SHN_UNDEF -> isUndefinedSymbol
not SHN_UNDEF and (not in SHN_LORESERVE..SHN_HIRESERVE) except for SHN_XINDEX -> isDefinedSymbol
SHN_LOPROC..SHN_HIOS -> isTargetSpecific
SHN_ABS -> isAbsSymbol
SHN_COMMON ->isAbsSymbol

rafael added inline comments.Jun 5 2015, 6:43 AM
include/llvm/Object/ELF.h
1030 ↗(On Diff #27166)

Actually, sorry, you need the type.

On a .o the symbol is required to have SHN_COMMON, but that doesn't exist on a .so.

Since we want this to work for both what you have is probably the best.

ruiu edited edge metadata.Jun 5 2015, 12:49 PM

All these member functions don't seem to actually belong to ELFFile. They
just call member functions of their arguments. Does it make more sense if
we add them to Elf_Sym?

davide updated this revision to Diff 27224.Jun 5 2015, 1:46 PM
davide edited edge metadata.
davide removed rL LLVM as the repository for this revision.

Yes, you're both right. I should have done this before.

davide updated this revision to Diff 27225.Jun 5 2015, 1:53 PM

Updated.

ruiu added a comment.Jun 5 2015, 1:56 PM

LGTM but please wait for Rafael.

davide added a comment.Jun 5 2015, 2:03 PM

2015-06-05 13:56 GMT-07:00 Rui Ueyama <ruiu@google.com>:

LGTM but please wait for Rafael.

Sure, no rush.

rafael accepted this revision.Jun 5 2015, 7:46 PM
rafael edited edge metadata.

LGTM with a fix for common symbols.

include/llvm/Object/ELFTypes.h
185

Now that I think of it, do we count common as defined? For ELF probably.

With that you can use

!isUndefiredSymbol && !(st_shndx >= SHN_LORESERVE && st_shndx < SHN_ABS);

This revision is now accepted and ready to land.Jun 5 2015, 7:46 PM
davide closed this revision.Jun 6 2015, 3:58 PM