This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] Correct sh_info for static symbol table
ClosedPublic

Authored by peter.smith on Jan 20 2017, 6:08 AM.

Details

Summary

The sh_info field of the SHT_SYMTAB section holds the index for the first non-local symbol. When there are global symbols that are output with STB_LOCAL binding due to having hidden visibility or matching the local version from a version script, the calculated value of NumLocals + 1 does not account for them. This change adds in the number of local symbols that aren't covered by NumLocals.

I think that this property is of marginal use in an Executable/Shared object but there could be an ELF processing tool that could get confused if it skips all the local symbols in the file using the sh_info field.

The test updates are where we have a global symbol output with local binding and a check of a sh_info field.

I noticed this while working on support for synthetic local symbols. It feels like a bit of a hack, but if NumLocals is interpreted as number of locals stored outside of the SymbolTableSection::Symbols in Files it makes a bit more sense.

Diff Detail

Event Timeline

peter.smith created this revision.Jan 20 2017, 6:08 AM
ruiu added inline comments.Jan 21 2017, 4:22 PM
ELF/SyntheticSections.cpp
1071–1072

Maybe we should stop using NumLocals at all here? I wonder if something like this works.

auto It = std::stable_partition(Symbols.begin(), Symbols.end(), [&](SymbolTableEntry &S) { returns true if S is STB_LOCAL; });
this->Info = It - Symbols.begin();
this->OutSec->Info = It - Symbols.begin();

Thank you for the comments. Updated to use std::stable_partition and rebased on top of D29021.

ruiu accepted this revision.Jan 23 2017, 9:52 AM

LGTM

This revision is now accepted and ready to land.Jan 23 2017, 9:52 AM
This revision was automatically updated to reflect the committed changes.