Skip to content

Commit

Permalink
[llvm-objcopy][NFC] Refactor symbol/section matching
Browse files Browse the repository at this point in the history
Summary:
The matchers for section/symbol related flags (e.g. `--keep-symbol=Name` or `--regex --keep-symbol=foo.*`) are currently just vectors that are matched linearlly. However, adding wildcard support would require negative matching too, e.g. a symbol should be removed if it matches a wildcard *but* doesn't match some other wildcard.

To make the next patch simpler, consolidate matching logic to a class defined in CopyConfig that takes care of matching.

Reviewers: jhenderson, seiya, MaskRay, espindola, alexshap

Reviewed By: jhenderson, MaskRay

Subscribers: emaste, arichardson, jakehehrlich, abrachet, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D66432

llvm-svn: 369689
  • Loading branch information
rupprecht committed Aug 22, 2019
1 parent 7a490c5 commit 6c6dd6a
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 44 deletions.
9 changes: 4 additions & 5 deletions llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
Expand Up @@ -103,8 +103,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
Obj.removeSections([&Config](const Section &Sec) {
// Contrary to --only-keep-debug, --only-section fully removes sections that
// aren't mentioned.
if (!Config.OnlySection.empty() &&
!is_contained(Config.OnlySection, Sec.Name))
if (!Config.OnlySection.empty() && !Config.OnlySection.matches(Sec.Name))
return true;

if (Config.StripDebug || Config.StripAll || Config.StripAllGNU ||
Expand All @@ -114,7 +113,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
return true;
}

if (is_contained(Config.ToRemove, Sec.Name))
if (Config.ToRemove.matches(Sec.Name))
return true;

return false;
Expand Down Expand Up @@ -148,7 +147,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
if (Config.StripAll || Config.StripAllGNU)
return true;

if (is_contained(Config.SymbolsToRemove, Sym.Name)) {
if (Config.SymbolsToRemove.matches(Sym.Name)) {
// Explicitly removing a referenced symbol is an error.
if (Sym.Referenced)
reportError(Config.OutputFilename,
Expand All @@ -167,7 +166,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
if (Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC ||
Sym.Sym.SectionNumber == 0)
if (Config.StripUnneeded ||
is_contained(Config.UnneededSymbolsToRemove, Sym.Name))
Config.UnneededSymbolsToRemove.matches(Sym.Name))
return true;

// GNU objcopy keeps referenced local symbols and external symbols
Expand Down
35 changes: 17 additions & 18 deletions llvm/tools/llvm-objcopy/CopyConfig.cpp
Expand Up @@ -342,9 +342,8 @@ getOutputTargetInfoByTargetName(StringRef TargetName) {
return {TargetInfo{Format, MI}};
}

static Error addSymbolsFromFile(std::vector<NameOrRegex> &Symbols,
BumpPtrAllocator &Alloc, StringRef Filename,
bool UseRegex) {
static Error addSymbolsFromFile(NameMatcher &Symbols, BumpPtrAllocator &Alloc,
StringRef Filename, bool UseRegex) {
StringSaver Saver(Alloc);
SmallVector<StringRef, 16> Lines;
auto BufOrErr = MemoryBuffer::getFile(Filename);
Expand All @@ -357,7 +356,7 @@ static Error addSymbolsFromFile(std::vector<NameOrRegex> &Symbols,
// it's not empty.
auto TrimmedLine = Line.split('#').first.trim();
if (!TrimmedLine.empty())
Symbols.emplace_back(Saver.save(TrimmedLine), UseRegex);
Symbols.addMatcher({Saver.save(TrimmedLine), UseRegex});
}

return Error::success();
Expand Down Expand Up @@ -613,11 +612,11 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
}

for (auto Arg : InputArgs.filtered(OBJCOPY_remove_section))
Config.ToRemove.emplace_back(Arg->getValue(), UseRegex);
Config.ToRemove.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_keep_section))
Config.KeepSection.emplace_back(Arg->getValue(), UseRegex);
Config.KeepSection.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_only_section))
Config.OnlySection.emplace_back(Arg->getValue(), UseRegex);
Config.OnlySection.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_add_section)) {
StringRef ArgValue(Arg->getValue());
if (!ArgValue.contains('='))
Expand Down Expand Up @@ -655,43 +654,43 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
if (Config.DiscardMode == DiscardType::All)
Config.StripDebug = true;
for (auto Arg : InputArgs.filtered(OBJCOPY_localize_symbol))
Config.SymbolsToLocalize.emplace_back(Arg->getValue(), UseRegex);
Config.SymbolsToLocalize.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_localize_symbols))
if (Error E = addSymbolsFromFile(Config.SymbolsToLocalize, DC.Alloc,
Arg->getValue(), UseRegex))
return std::move(E);
for (auto Arg : InputArgs.filtered(OBJCOPY_keep_global_symbol))
Config.SymbolsToKeepGlobal.emplace_back(Arg->getValue(), UseRegex);
Config.SymbolsToKeepGlobal.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_keep_global_symbols))
if (Error E = addSymbolsFromFile(Config.SymbolsToKeepGlobal, DC.Alloc,
Arg->getValue(), UseRegex))
return std::move(E);
for (auto Arg : InputArgs.filtered(OBJCOPY_globalize_symbol))
Config.SymbolsToGlobalize.emplace_back(Arg->getValue(), UseRegex);
Config.SymbolsToGlobalize.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_globalize_symbols))
if (Error E = addSymbolsFromFile(Config.SymbolsToGlobalize, DC.Alloc,
Arg->getValue(), UseRegex))
return std::move(E);
for (auto Arg : InputArgs.filtered(OBJCOPY_weaken_symbol))
Config.SymbolsToWeaken.emplace_back(Arg->getValue(), UseRegex);
Config.SymbolsToWeaken.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_weaken_symbols))
if (Error E = addSymbolsFromFile(Config.SymbolsToWeaken, DC.Alloc,
Arg->getValue(), UseRegex))
return std::move(E);
for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbol))
Config.SymbolsToRemove.emplace_back(Arg->getValue(), UseRegex);
Config.SymbolsToRemove.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbols))
if (Error E = addSymbolsFromFile(Config.SymbolsToRemove, DC.Alloc,
Arg->getValue(), UseRegex))
return std::move(E);
for (auto Arg : InputArgs.filtered(OBJCOPY_strip_unneeded_symbol))
Config.UnneededSymbolsToRemove.emplace_back(Arg->getValue(), UseRegex);
Config.UnneededSymbolsToRemove.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_strip_unneeded_symbols))
if (Error E = addSymbolsFromFile(Config.UnneededSymbolsToRemove, DC.Alloc,
Arg->getValue(), UseRegex))
return std::move(E);
for (auto Arg : InputArgs.filtered(OBJCOPY_keep_symbol))
Config.SymbolsToKeep.emplace_back(Arg->getValue(), UseRegex);
Config.SymbolsToKeep.addMatcher({Arg->getValue(), UseRegex});
for (auto Arg : InputArgs.filtered(OBJCOPY_keep_symbols))
if (Error E = addSymbolsFromFile(Config.SymbolsToKeep, DC.Alloc,
Arg->getValue(), UseRegex))
Expand Down Expand Up @@ -820,16 +819,16 @@ parseStripOptions(ArrayRef<const char *> ArgsArr,
Config.KeepFileSymbols = InputArgs.hasArg(STRIP_keep_file_symbols);

for (auto Arg : InputArgs.filtered(STRIP_keep_section))
Config.KeepSection.emplace_back(Arg->getValue(), UseRegexp);
Config.KeepSection.addMatcher({Arg->getValue(), UseRegexp});

for (auto Arg : InputArgs.filtered(STRIP_remove_section))
Config.ToRemove.emplace_back(Arg->getValue(), UseRegexp);
Config.ToRemove.addMatcher({Arg->getValue(), UseRegexp});

for (auto Arg : InputArgs.filtered(STRIP_strip_symbol))
Config.SymbolsToRemove.emplace_back(Arg->getValue(), UseRegexp);
Config.SymbolsToRemove.addMatcher({Arg->getValue(), UseRegexp});

for (auto Arg : InputArgs.filtered(STRIP_keep_symbol))
Config.SymbolsToKeep.emplace_back(Arg->getValue(), UseRegexp);
Config.SymbolsToKeep.addMatcher({Arg->getValue(), UseRegexp});

if (!InputArgs.hasArg(STRIP_no_strip_all) && !Config.StripDebug &&
!Config.StripUnneeded && Config.DiscardMode == DiscardType::None &&
Expand Down
37 changes: 27 additions & 10 deletions llvm/tools/llvm-objcopy/CopyConfig.h
Expand Up @@ -98,6 +98,19 @@ class NameOrRegex {
bool operator!=(StringRef S) const { return !operator==(S); }
};

// Matcher that checks symbol or section names against the command line flags
// provided for that option.
class NameMatcher {
std::vector<NameOrRegex> Matchers;

public:
void addMatcher(NameOrRegex Matcher) {
Matchers.push_back(std::move(Matcher));
}
bool matches(StringRef S) const { return is_contained(Matchers, S); }
bool empty() const { return Matchers.empty(); }
};

struct NewSymbolInfo {
StringRef SymbolName;
StringRef SectionName;
Expand Down Expand Up @@ -137,16 +150,20 @@ struct CopyConfig {
std::vector<StringRef> AddSection;
std::vector<StringRef> DumpSection;
std::vector<NewSymbolInfo> SymbolsToAdd;
std::vector<NameOrRegex> KeepSection;
std::vector<NameOrRegex> OnlySection;
std::vector<NameOrRegex> SymbolsToGlobalize;
std::vector<NameOrRegex> SymbolsToKeep;
std::vector<NameOrRegex> SymbolsToLocalize;
std::vector<NameOrRegex> SymbolsToRemove;
std::vector<NameOrRegex> UnneededSymbolsToRemove;
std::vector<NameOrRegex> SymbolsToWeaken;
std::vector<NameOrRegex> ToRemove;
std::vector<NameOrRegex> SymbolsToKeepGlobal;

// Section matchers
NameMatcher KeepSection;
NameMatcher OnlySection;
NameMatcher ToRemove;

// Symbol matchers
NameMatcher SymbolsToGlobalize;
NameMatcher SymbolsToKeep;
NameMatcher SymbolsToLocalize;
NameMatcher SymbolsToRemove;
NameMatcher UnneededSymbolsToRemove;
NameMatcher SymbolsToWeaken;
NameMatcher SymbolsToKeepGlobal;

// Map options
StringMap<SectionRename> SectionsToRename;
Expand Down
21 changes: 10 additions & 11 deletions llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
Expand Up @@ -356,7 +356,7 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
if (!Sym.isCommon() && Sym.getShndx() != SHN_UNDEF &&
((Config.LocalizeHidden &&
(Sym.Visibility == STV_HIDDEN || Sym.Visibility == STV_INTERNAL)) ||
is_contained(Config.SymbolsToLocalize, Sym.Name)))
Config.SymbolsToLocalize.matches(Sym.Name)))
Sym.Binding = STB_LOCAL;

// Note: these two globalize flags have very similar names but different
Expand All @@ -370,16 +370,15 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
// --keep-global-symbol. Because of that, make sure to check
// --globalize-symbol second.
if (!Config.SymbolsToKeepGlobal.empty() &&
!is_contained(Config.SymbolsToKeepGlobal, Sym.Name) &&
!Config.SymbolsToKeepGlobal.matches(Sym.Name) &&
Sym.getShndx() != SHN_UNDEF)
Sym.Binding = STB_LOCAL;

if (is_contained(Config.SymbolsToGlobalize, Sym.Name) &&
if (Config.SymbolsToGlobalize.matches(Sym.Name) &&
Sym.getShndx() != SHN_UNDEF)
Sym.Binding = STB_GLOBAL;

if (is_contained(Config.SymbolsToWeaken, Sym.Name) &&
Sym.Binding == STB_GLOBAL)
if (Config.SymbolsToWeaken.matches(Sym.Name) && Sym.Binding == STB_GLOBAL)
Sym.Binding = STB_WEAK;

if (Config.Weaken && Sym.Binding == STB_GLOBAL &&
Expand All @@ -404,7 +403,7 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
}

auto RemoveSymbolsPred = [&](const Symbol &Sym) {
if (is_contained(Config.SymbolsToKeep, Sym.Name) ||
if (Config.SymbolsToKeep.matches(Sym.Name) ||
(Config.KeepFileSymbols && Sym.Type == STT_FILE))
return false;

Expand All @@ -418,11 +417,11 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
if (Config.StripAll || Config.StripAllGNU)
return true;

if (is_contained(Config.SymbolsToRemove, Sym.Name))
if (Config.SymbolsToRemove.matches(Sym.Name))
return true;

if ((Config.StripUnneeded ||
is_contained(Config.UnneededSymbolsToRemove, Sym.Name)) &&
Config.UnneededSymbolsToRemove.matches(Sym.Name)) &&
(!Obj.isRelocatable() || isUnneededSymbol(Sym)))
return true;

Expand All @@ -443,7 +442,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
// Removes:
if (!Config.ToRemove.empty()) {
RemovePred = [&Config](const SectionBase &Sec) {
return is_contained(Config.ToRemove, Sec.Name);
return Config.ToRemove.matches(Sec.Name);
};
}

Expand Down Expand Up @@ -523,7 +522,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
if (!Config.OnlySection.empty()) {
RemovePred = [&Config, RemovePred, &Obj](const SectionBase &Sec) {
// Explicitly keep these sections regardless of previous removes.
if (is_contained(Config.OnlySection, Sec.Name))
if (Config.OnlySection.matches(Sec.Name))
return false;

// Allow all implicit removes.
Expand All @@ -545,7 +544,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
if (!Config.KeepSection.empty()) {
RemovePred = [&Config, RemovePred](const SectionBase &Sec) {
// Explicitly keep these sections regardless of previous removes.
if (is_contained(Config.KeepSection, Sec.Name))
if (Config.KeepSection.matches(Sec.Name))
return false;
// Otherwise defer to RemovePred.
return RemovePred(Sec);
Expand Down

0 comments on commit 6c6dd6a

Please sign in to comment.