[llvm] 83cc447 - [llvm-objcopy][NFC] Refactor CopyConfig structure - remove lazy options processing.
Alexey Lapshin via llvm-commits
llvm-commits at lists.llvm.org
Mon May 31 04:44:15 PDT 2021
Author: Alexey Lapshin
Date: 2021-05-31T14:40:27+03:00
New Revision: 83cc4478a060e795046c544d0b7618747f51f6d4
URL: https://github.com/llvm/llvm-project/commit/83cc4478a060e795046c544d0b7618747f51f6d4
DIFF: https://github.com/llvm/llvm-project/commit/83cc4478a060e795046c544d0b7618747f51f6d4.diff
LOG: [llvm-objcopy][NFC] Refactor CopyConfig structure - remove lazy options processing.
During reviewing D102277 it was decided to remove lazy options processing
from llvm-objcopy CopyConfig structure. This patch transforms processing of ELF
lazy options into the in-place processing.
Differential Revision: https://reviews.llvm.org/D103260
Added:
Modified:
llvm/tools/llvm-objcopy/CommonConfig.h
llvm/tools/llvm-objcopy/ConfigManager.cpp
llvm/tools/llvm-objcopy/ConfigManager.h
llvm/tools/llvm-objcopy/ELF/ELFConfig.h
llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
Removed:
################################################################################
diff --git a/llvm/tools/llvm-objcopy/CommonConfig.h b/llvm/tools/llvm-objcopy/CommonConfig.h
index d08765cbc5458..49a77e1815a0d 100644
--- a/llvm/tools/llvm-objcopy/CommonConfig.h
+++ b/llvm/tools/llvm-objcopy/CommonConfig.h
@@ -141,6 +141,36 @@ class NameMatcher {
bool empty() const { return PosMatchers.empty() && NegMatchers.empty(); }
};
+enum class SymbolFlag {
+ Global,
+ Local,
+ Weak,
+ Default,
+ Hidden,
+ Protected,
+ File,
+ Section,
+ Object,
+ Function,
+ IndirectFunction,
+ Debug,
+ Constructor,
+ Warning,
+ Indirect,
+ Synthetic,
+ UniqueObject,
+};
+
+// Symbol info specified by --add-symbol option. Symbol flags not supported
+// by a concrete format should be ignored.
+struct NewSymbolInfo {
+ StringRef SymbolName;
+ StringRef SectionName;
+ uint64_t Value = 0;
+ std::vector<SymbolFlag> Flags;
+ std::vector<StringRef> BeforeSyms;
+};
+
// Configuration for copying/stripping a single file.
struct CommonConfig {
// Main input/output options
@@ -200,6 +230,9 @@ struct CommonConfig {
// --change-start is used.
std::function<uint64_t(uint64_t)> EntryExpr;
+ // Symbol info specified by --add-symbol option.
+ std::vector<NewSymbolInfo> SymbolsToAdd;
+
// Boolean options
bool AllowBrokenLinks = false;
bool DeterministicArchives = true;
diff --git a/llvm/tools/llvm-objcopy/ConfigManager.cpp b/llvm/tools/llvm-objcopy/ConfigManager.cpp
index 2f0d30c78fd50..e29d07db6f3b0 100644
--- a/llvm/tools/llvm-objcopy/ConfigManager.cpp
+++ b/llvm/tools/llvm-objcopy/ConfigManager.cpp
@@ -466,8 +466,7 @@ static void printHelp(const opt::OptTable &OptTable, raw_ostream &OS,
OS << "\nPass @FILE as argument to read options from FILE.\n";
}
-static Expected<NewSymbolInfo> parseNewSymbolInfo(StringRef FlagValue,
- uint8_t DefaultVisibility) {
+static Expected<NewSymbolInfo> parseNewSymbolInfo(StringRef FlagValue) {
// Parse value given with --add-symbol option and create the
// new symbol if possible. The value format for --add-symbol is:
//
@@ -478,17 +477,7 @@ static Expected<NewSymbolInfo> parseNewSymbolInfo(StringRef FlagValue,
// <section> - optional section name. If not given ABS symbol is created
// <value> - symbol value, can be decimal or hexadecimal number prefixed
// with 0x.
- // <flags> - optional flags affecting symbol type, binding or visibility:
- // The following are currently supported:
- //
- // global, local, weak, default, hidden, file, section, object,
- // indirect-function.
- //
- // The following flags are ignored and provided for GNU
- // compatibility only:
- //
- // warning, debug, constructor, indirect, synthetic,
- // unique-object, before=<symbol>.
+ // <flags> - optional flags affecting symbol type, binding or visibility.
NewSymbolInfo SI;
StringRef Value;
std::tie(SI.SymbolName, Value) = FlagValue.split('=');
@@ -512,86 +501,73 @@ static Expected<NewSymbolInfo> parseNewSymbolInfo(StringRef FlagValue,
return createStringError(errc::invalid_argument, "bad symbol value: '%s'",
Flags[0].str().c_str());
- SI.Visibility = DefaultVisibility;
- using Functor = std::function<void(void)>;
+ using Functor = std::function<void()>;
SmallVector<StringRef, 6> UnsupportedFlags;
for (size_t I = 1, NumFlags = Flags.size(); I < NumFlags; ++I)
static_cast<Functor>(
StringSwitch<Functor>(Flags[I])
- .CaseLower("global", [&SI] { SI.Bind = ELF::STB_GLOBAL; })
- .CaseLower("local", [&SI] { SI.Bind = ELF::STB_LOCAL; })
- .CaseLower("weak", [&SI] { SI.Bind = ELF::STB_WEAK; })
- .CaseLower("default", [&SI] { SI.Visibility = ELF::STV_DEFAULT; })
- .CaseLower("hidden", [&SI] { SI.Visibility = ELF::STV_HIDDEN; })
+ .CaseLower("global",
+ [&] { SI.Flags.push_back(SymbolFlag::Global); })
+ .CaseLower("local", [&] { SI.Flags.push_back(SymbolFlag::Local); })
+ .CaseLower("weak", [&] { SI.Flags.push_back(SymbolFlag::Weak); })
+ .CaseLower("default",
+ [&] { SI.Flags.push_back(SymbolFlag::Default); })
+ .CaseLower("hidden",
+ [&] { SI.Flags.push_back(SymbolFlag::Hidden); })
.CaseLower("protected",
- [&SI] { SI.Visibility = ELF::STV_PROTECTED; })
- .CaseLower("file", [&SI] { SI.Type = ELF::STT_FILE; })
- .CaseLower("section", [&SI] { SI.Type = ELF::STT_SECTION; })
- .CaseLower("object", [&SI] { SI.Type = ELF::STT_OBJECT; })
- .CaseLower("function", [&SI] { SI.Type = ELF::STT_FUNC; })
- .CaseLower("indirect-function",
- [&SI] { SI.Type = ELF::STT_GNU_IFUNC; })
- .CaseLower("debug", [] {})
- .CaseLower("constructor", [] {})
- .CaseLower("warning", [] {})
- .CaseLower("indirect", [] {})
- .CaseLower("synthetic", [] {})
- .CaseLower("unique-object", [] {})
- .StartsWithLower("before", [] {})
+ [&] { SI.Flags.push_back(SymbolFlag::Protected); })
+ .CaseLower("file", [&] { SI.Flags.push_back(SymbolFlag::File); })
+ .CaseLower("section",
+ [&] { SI.Flags.push_back(SymbolFlag::Section); })
+ .CaseLower("object",
+ [&] { SI.Flags.push_back(SymbolFlag::Object); })
+ .CaseLower("function",
+ [&] { SI.Flags.push_back(SymbolFlag::Function); })
+ .CaseLower(
+ "indirect-function",
+ [&] { SI.Flags.push_back(SymbolFlag::IndirectFunction); })
+ .CaseLower("debug", [&] { SI.Flags.push_back(SymbolFlag::Debug); })
+ .CaseLower("constructor",
+ [&] { SI.Flags.push_back(SymbolFlag::Constructor); })
+ .CaseLower("warning",
+ [&] { SI.Flags.push_back(SymbolFlag::Warning); })
+ .CaseLower("indirect",
+ [&] { SI.Flags.push_back(SymbolFlag::Indirect); })
+ .CaseLower("synthetic",
+ [&] { SI.Flags.push_back(SymbolFlag::Synthetic); })
+ .CaseLower("unique-object",
+ [&] { SI.Flags.push_back(SymbolFlag::UniqueObject); })
+ .StartsWithLower("before=",
+ [&] {
+ StringRef SymNamePart =
+ Flags[I].split('=').second;
+
+ if (!SymNamePart.empty())
+ SI.BeforeSyms.push_back(SymNamePart);
+ })
.Default([&] { UnsupportedFlags.push_back(Flags[I]); }))();
if (!UnsupportedFlags.empty())
return createStringError(errc::invalid_argument,
"unsupported flag%s for --add-symbol: '%s'",
UnsupportedFlags.size() > 1 ? "s" : "",
join(UnsupportedFlags, "', '").c_str());
+
return SI;
}
Expected<const ELFConfig &> ConfigManager::getELFConfig() const {
- if (!ELF) {
- if (Common.StripSwiftSymbols || Common.KeepUndefined)
- return createStringError(llvm::errc::invalid_argument,
- "option not supported by llvm-objcopy for ELF");
-
- // Parse lazy options.
- ELFConfig ResConfig;
-
- if (NewSymbolVisibility) {
- const uint8_t Invalid = 0xff;
- ResConfig.NewSymbolVisibility =
- StringSwitch<uint8_t>(*NewSymbolVisibility)
- .Case("default", ELF::STV_DEFAULT)
- .Case("hidden", ELF::STV_HIDDEN)
- .Case("internal", ELF::STV_INTERNAL)
- .Case("protected", ELF::STV_PROTECTED)
- .Default(Invalid);
-
- if (ResConfig.NewSymbolVisibility == Invalid)
- return createStringError(errc::invalid_argument,
- "'%s' is not a valid symbol visibility",
- NewSymbolVisibility->str().c_str());
- }
-
- for (StringRef Arg : SymbolsToAdd) {
- Expected<NewSymbolInfo> NSI = parseNewSymbolInfo(
- Arg,
- ResConfig.NewSymbolVisibility.getValueOr((uint8_t)ELF::STV_DEFAULT));
- if (!NSI)
- return NSI.takeError();
- ResConfig.SymbolsToAdd.push_back(*NSI);
- }
-
- ELF = std::move(ResConfig);
- }
+ if (Common.StripSwiftSymbols || Common.KeepUndefined)
+ return createStringError(llvm::errc::invalid_argument,
+ "option not supported by llvm-objcopy for ELF");
- return *ELF;
+ return ELF;
}
Expected<const COFFConfig &> ConfigManager::getCOFFConfig() const {
if (Common.AllowBrokenLinks || !Common.SplitDWO.empty() ||
!Common.SymbolsPrefix.empty() || !Common.AllocSectionsPrefix.empty() ||
!Common.DumpSection.empty() || !Common.KeepSection.empty() ||
- NewSymbolVisibility || !Common.SymbolsToGlobalize.empty() ||
+ ELF.NewSymbolVisibility || !Common.SymbolsToGlobalize.empty() ||
!Common.SymbolsToKeep.empty() || !Common.SymbolsToLocalize.empty() ||
!Common.SymbolsToWeaken.empty() || !Common.SymbolsToKeepGlobal.empty() ||
!Common.SectionsToRename.empty() || !Common.SetSectionAlignment.empty() ||
@@ -599,8 +575,8 @@ Expected<const COFFConfig &> ConfigManager::getCOFFConfig() const {
Common.StripDWO || Common.StripNonAlloc || Common.StripSections ||
Common.StripSwiftSymbols || Common.KeepUndefined || Common.Weaken ||
Common.DecompressDebugSections ||
- Common.DiscardMode == DiscardType::Locals || !SymbolsToAdd.empty() ||
- Common.EntryExpr) {
+ Common.DiscardMode == DiscardType::Locals ||
+ !Common.SymbolsToAdd.empty() || Common.EntryExpr) {
return createStringError(llvm::errc::invalid_argument,
"option not supported by llvm-objcopy for COFF");
}
@@ -611,7 +587,7 @@ Expected<const COFFConfig &> ConfigManager::getCOFFConfig() const {
Expected<const MachOConfig &> ConfigManager::getMachOConfig() const {
if (Common.AllowBrokenLinks || !Common.SplitDWO.empty() ||
!Common.SymbolsPrefix.empty() || !Common.AllocSectionsPrefix.empty() ||
- !Common.KeepSection.empty() || NewSymbolVisibility ||
+ !Common.KeepSection.empty() || ELF.NewSymbolVisibility ||
!Common.SymbolsToGlobalize.empty() || !Common.SymbolsToKeep.empty() ||
!Common.SymbolsToLocalize.empty() || !Common.SymbolsToWeaken.empty() ||
!Common.SymbolsToKeepGlobal.empty() || !Common.SectionsToRename.empty() ||
@@ -621,7 +597,7 @@ Expected<const MachOConfig &> ConfigManager::getMachOConfig() const {
Common.StripAllGNU || Common.StripDWO || Common.StripNonAlloc ||
Common.StripSections || Common.Weaken || Common.DecompressDebugSections ||
Common.StripUnneeded || Common.DiscardMode == DiscardType::Locals ||
- !SymbolsToAdd.empty() || Common.EntryExpr) {
+ !Common.SymbolsToAdd.empty() || Common.EntryExpr) {
return createStringError(llvm::errc::invalid_argument,
"option not supported by llvm-objcopy for MachO");
}
@@ -633,8 +609,8 @@ Expected<const WasmConfig &> ConfigManager::getWasmConfig() const {
if (!Common.AddGnuDebugLink.empty() || Common.ExtractPartition ||
!Common.SplitDWO.empty() || !Common.SymbolsPrefix.empty() ||
!Common.AllocSectionsPrefix.empty() ||
- Common.DiscardMode != DiscardType::None || NewSymbolVisibility ||
- !SymbolsToAdd.empty() || !Common.RPathToAdd.empty() ||
+ Common.DiscardMode != DiscardType::None || ELF.NewSymbolVisibility ||
+ !Common.SymbolsToAdd.empty() || !Common.RPathToAdd.empty() ||
!Common.OnlySection.empty() || !Common.SymbolsToGlobalize.empty() ||
!Common.SymbolsToKeep.empty() || !Common.SymbolsToLocalize.empty() ||
!Common.SymbolsToRemove.empty() ||
@@ -705,6 +681,7 @@ parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
ConfigManager ConfigMgr;
CommonConfig &Config = ConfigMgr.Common;
+ ELFConfig &ELFConfig = ConfigMgr.ELF;
Config.InputFilename = Positional[0];
Config.OutputFilename = Positional[Positional.size() == 1 ? 0 : 1];
if (InputArgs.hasArg(OBJCOPY_target) &&
@@ -743,10 +720,24 @@ parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
.Case("ihex", FileFormat::IHex)
.Default(FileFormat::Unspecified);
- if (InputArgs.hasArg(OBJCOPY_new_symbol_visibility))
- ConfigMgr.NewSymbolVisibility =
+ if (InputArgs.hasArg(OBJCOPY_new_symbol_visibility)) {
+ const uint8_t Invalid = 0xff;
+ StringRef VisibilityStr =
InputArgs.getLastArgValue(OBJCOPY_new_symbol_visibility);
+ ELFConfig.NewSymbolVisibility = StringSwitch<uint8_t>(VisibilityStr)
+ .Case("default", ELF::STV_DEFAULT)
+ .Case("hidden", ELF::STV_HIDDEN)
+ .Case("internal", ELF::STV_INTERNAL)
+ .Case("protected", ELF::STV_PROTECTED)
+ .Default(Invalid);
+
+ if (ELFConfig.NewSymbolVisibility == Invalid)
+ return createStringError(errc::invalid_argument,
+ "'%s' is not a valid symbol visibility",
+ VisibilityStr.str().c_str());
+ }
+
Config.OutputFormat = StringSwitch<FileFormat>(OutputFormat)
.Case("binary", FileFormat::Binary)
.Case("ihex", FileFormat::IHex)
@@ -992,8 +983,13 @@ parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
addSymbolsFromFile(Config.SymbolsToKeep, DC.Alloc, Arg->getValue(),
SymbolMatchStyle, ErrorCallback))
return std::move(E);
- for (auto Arg : InputArgs.filtered(OBJCOPY_add_symbol))
- ConfigMgr.SymbolsToAdd.push_back(Arg->getValue());
+ for (auto *Arg : InputArgs.filtered(OBJCOPY_add_symbol)) {
+ Expected<NewSymbolInfo> SymInfo = parseNewSymbolInfo(Arg->getValue());
+ if (!SymInfo)
+ return SymInfo.takeError();
+
+ Config.SymbolsToAdd.push_back(*SymInfo);
+ }
Config.AllowBrokenLinks = InputArgs.hasArg(OBJCOPY_allow_broken_links);
diff --git a/llvm/tools/llvm-objcopy/ConfigManager.h b/llvm/tools/llvm-objcopy/ConfigManager.h
index ec1d2f33f6a0e..c0d0e8bbc7219 100644
--- a/llvm/tools/llvm-objcopy/ConfigManager.h
+++ b/llvm/tools/llvm-objcopy/ConfigManager.h
@@ -32,13 +32,9 @@ struct ConfigManager : public MultiFormatConfig {
Expected<const MachOConfig &> getMachOConfig() const override;
Expected<const WasmConfig &> getWasmConfig() const override;
- // String representation for lazy ELF options.
- std::vector<StringRef> SymbolsToAdd;
- Optional<StringRef> NewSymbolVisibility;
-
// All configs.
CommonConfig Common;
- mutable Optional<ELFConfig> ELF;
+ ELFConfig ELF;
COFFConfig COFF;
MachOConfig MachO;
WasmConfig Wasm;
diff --git a/llvm/tools/llvm-objcopy/ELF/ELFConfig.h b/llvm/tools/llvm-objcopy/ELF/ELFConfig.h
index cb170bd93a5c5..42d407da17fff 100644
--- a/llvm/tools/llvm-objcopy/ELF/ELFConfig.h
+++ b/llvm/tools/llvm-objcopy/ELF/ELFConfig.h
@@ -17,19 +17,9 @@
namespace llvm {
namespace objcopy {
-struct NewSymbolInfo {
- StringRef SymbolName;
- StringRef SectionName;
- uint64_t Value = 0;
- uint8_t Type = ELF::STT_NOTYPE;
- uint8_t Bind = ELF::STB_GLOBAL;
- uint8_t Visibility = ELF::STV_DEFAULT;
-};
-
// ELF specific configuration for copying/stripping a single file.
struct ELFConfig {
- Optional<uint8_t> NewSymbolVisibility;
- std::vector<NewSymbolInfo> SymbolsToAdd;
+ uint8_t NewSymbolVisibility = (uint8_t)ELF::STV_DEFAULT;
};
} // namespace objcopy
diff --git a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
index 806cd73dae0d0..e400882ec20d9 100644
--- a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
@@ -500,6 +500,60 @@ static Error replaceAndRemoveSections(const CommonConfig &Config, Object &Obj) {
return Obj.removeSections(Config.AllowBrokenLinks, RemovePred);
}
+// Add symbol to the Object symbol table with the specified properties.
+static void addSymbol(Object &Obj, const NewSymbolInfo &SymInfo,
+ uint8_t DefaultVisibility) {
+ SectionBase *Sec = Obj.findSection(SymInfo.SectionName);
+ uint64_t Value = Sec ? Sec->Addr + SymInfo.Value : SymInfo.Value;
+
+ uint8_t Bind = ELF::STB_GLOBAL;
+ uint8_t Type = ELF::STT_NOTYPE;
+ uint8_t Visibility = DefaultVisibility;
+
+ for (SymbolFlag FlagValue : SymInfo.Flags)
+ switch (FlagValue) {
+ case SymbolFlag::Global:
+ Bind = ELF::STB_GLOBAL;
+ break;
+ case SymbolFlag::Local:
+ Bind = ELF::STB_LOCAL;
+ break;
+ case SymbolFlag::Weak:
+ Bind = ELF::STB_WEAK;
+ break;
+ case SymbolFlag::Default:
+ Visibility = ELF::STV_DEFAULT;
+ break;
+ case SymbolFlag::Hidden:
+ Visibility = ELF::STV_HIDDEN;
+ break;
+ case SymbolFlag::Protected:
+ Visibility = ELF::STV_PROTECTED;
+ break;
+ case SymbolFlag::File:
+ Type = ELF::STT_FILE;
+ break;
+ case SymbolFlag::Section:
+ Type = ELF::STT_SECTION;
+ break;
+ case SymbolFlag::Object:
+ Type = ELF::STT_OBJECT;
+ break;
+ case SymbolFlag::Function:
+ Type = ELF::STT_FUNC;
+ break;
+ case SymbolFlag::IndirectFunction:
+ Type = ELF::STT_GNU_IFUNC;
+ break;
+ default: /* Other flag values are ignored for ELF. */
+ break;
+ };
+
+ Obj.SymbolTable->addSymbol(
+ SymInfo.SymbolName, Bind, Type, Sec, Value, Visibility,
+ Sec ? (uint16_t)SYMBOL_SIMPLE_INDEX : (uint16_t)SHN_ABS, 0);
+}
+
// This function handles the high level operations of GNU objcopy including
// handling command line options. It's important to outline certain properties
// we expect to hold of the command line operations. Any operation that "keeps"
@@ -633,17 +687,12 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig,
// If the symbol table was previously removed, we need to create a new one
// before adding new symbols.
- if (!Obj.SymbolTable && !ELFConfig.SymbolsToAdd.empty())
+ if (!Obj.SymbolTable && !Config.SymbolsToAdd.empty())
if (Error E = Obj.addNewSymbolTable())
return E;
- for (const NewSymbolInfo &SI : ELFConfig.SymbolsToAdd) {
- SectionBase *Sec = Obj.findSection(SI.SectionName);
- uint64_t Value = Sec ? Sec->Addr + SI.Value : SI.Value;
- Obj.SymbolTable->addSymbol(
- SI.SymbolName, SI.Bind, SI.Type, Sec, Value, SI.Visibility,
- Sec ? (uint16_t)SYMBOL_SIMPLE_INDEX : (uint16_t)SHN_ABS, 0);
- }
+ for (const NewSymbolInfo &SI : Config.SymbolsToAdd)
+ addSymbol(Obj, SI, ELFConfig.NewSymbolVisibility);
// --set-section-flags works with sections added by --add-section.
if (!Config.SetSectionFlags.empty()) {
@@ -688,9 +737,7 @@ Error executeObjcopyOnIHex(const CommonConfig &Config,
Error executeObjcopyOnRawBinary(const CommonConfig &Config,
const ELFConfig &ELFConfig, MemoryBuffer &In,
raw_ostream &Out) {
- uint8_t NewSymbolVisibility =
- ELFConfig.NewSymbolVisibility.getValueOr((uint8_t)ELF::STV_DEFAULT);
- BinaryReader Reader(&In, NewSymbolVisibility);
+ BinaryReader Reader(&In, ELFConfig.NewSymbolVisibility);
Expected<std::unique_ptr<Object>> Obj = Reader.create(true);
if (!Obj)
return Obj.takeError();
@@ -709,7 +756,7 @@ Error executeObjcopyOnBinary(const CommonConfig &Config,
object::ELFObjectFileBase &In, raw_ostream &Out) {
ELFReader Reader(&In, Config.ExtractPartition);
Expected<std::unique_ptr<Object>> Obj =
- Reader.create(!ELFConfig.SymbolsToAdd.empty());
+ Reader.create(!Config.SymbolsToAdd.empty());
if (!Obj)
return Obj.takeError();
// Prefer OutputArch (-O<format>) if set, otherwise infer it from the input.
More information about the llvm-commits
mailing list