[llvm] 4029f8e - Temporarily Revert "[llvm-install-name-tool] Merge install-name options" as it breaks the objcopy build.
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 6 15:40:26 PDT 2020
Author: Eric Christopher
Date: 2020-07-06T15:40:14-07:00
New Revision: 4029f8ede42f69f5fb5affb3eb008e03d448f407
URL: https://github.com/llvm/llvm-project/commit/4029f8ede42f69f5fb5affb3eb008e03d448f407
DIFF: https://github.com/llvm/llvm-project/commit/4029f8ede42f69f5fb5affb3eb008e03d448f407.diff
LOG: Temporarily Revert "[llvm-install-name-tool] Merge install-name options" as it breaks the objcopy build.
This reverts commit c143900a0851b2c7b7d52e4825c7f073b3474cf6.
Added:
Modified:
llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test
llvm/tools/llvm-objcopy/CopyConfig.cpp
llvm/tools/llvm-objcopy/CopyConfig.h
llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test b/llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test
index 7b21fdc2e03c..1435c6b744c8 100644
--- a/llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test
+++ b/llvm/test/tools/llvm-objcopy/MachO/install-name-tool-add-rpath.test
@@ -22,13 +22,6 @@
# NO-INPUT: no input file specified
-## Add same RPATH twice:
-# RUN: not llvm-install-name-tool -add_rpath @executable_X \
-# RUN: -add_rpath @executable_X %t.i386 2>&1 \
-# RUN: | FileCheck --check-prefix=DOUBLE %s
-
-# DOUBLE: duplicate load command
-
## Check that cmdsize accounts for NULL terminator.
# RUN: yaml2obj %p/Inputs/x86_64.yaml -o %t.x86_64
# RUN: llvm-install-name-tool -add_rpath abcd %t.x86_64
diff --git a/llvm/tools/llvm-objcopy/CopyConfig.cpp b/llvm/tools/llvm-objcopy/CopyConfig.cpp
index 1fde54dd290a..f93406f371d0 100644
--- a/llvm/tools/llvm-objcopy/CopyConfig.cpp
+++ b/llvm/tools/llvm-objcopy/CopyConfig.cpp
@@ -874,39 +874,42 @@ parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr) {
auto Match = [=](StringRef RPath) { return RPath == Old || RPath == New; };
// Cannot specify duplicate -rpath entries
- auto It1 = find_if(
- Config.RPathsToUpdate,
- [&Match](const DenseMap<StringRef, StringRef>::value_type &OldNew) {
- return Match(OldNew.getFirst()) || Match(OldNew.getSecond());
- });
+ auto It1 = find_if(Config.RPathsToUpdate,
+ [&Match](const std::pair<StringRef, StringRef> &OldNew) {
+ return Match(OldNew.first) || Match(OldNew.second);
+ });
if (It1 != Config.RPathsToUpdate.end())
- return createStringError(errc::invalid_argument,
- "cannot specify both -rpath " + It1->getFirst() +
- " " + It1->getSecond() + " and -rpath " +
- Old + " " + New);
+ return createStringError(
+ errc::invalid_argument,
+ "cannot specify both -rpath %s %s and -rpath %s %s",
+ It1->first.str().c_str(), It1->second.str().c_str(),
+ Old.str().c_str(), New.str().c_str());
// Cannot specify the same rpath under both -delete_rpath and -rpath
auto It2 = find_if(Config.RPathsToRemove, Match);
if (It2 != Config.RPathsToRemove.end())
- return createStringError(errc::invalid_argument,
- "cannot specify both -delete_rpath " + *It2 +
- " and -rpath " + Old + " " + New);
+ return createStringError(
+ errc::invalid_argument,
+ "cannot specify both -delete_rpath %s and -rpath %s %s",
+ It2->str().c_str(), Old.str().c_str(), New.str().c_str());
// Cannot specify the same rpath under both -add_rpath and -rpath
auto It3 = find_if(Config.RPathToAdd, Match);
if (It3 != Config.RPathToAdd.end())
- return createStringError(errc::invalid_argument,
- "cannot specify both -add_rpath " + *It3 +
- " and -rpath " + Old + " " + New);
+ return createStringError(
+ errc::invalid_argument,
+ "cannot specify both -add_rpath %s and -rpath %s %s",
+ It3->str().c_str(), Old.str().c_str(), New.str().c_str());
- Config.RPathsToUpdate.insert({Old, New});
+ Config.RPathsToUpdate.emplace_back(Old, New);
}
if (auto *Arg = InputArgs.getLastArg(INSTALL_NAME_TOOL_id))
Config.SharedLibId = Arg->getValue();
for (auto *Arg : InputArgs.filtered(INSTALL_NAME_TOOL_change)) {
- Config.InstallNamesToUpdate.insert({Arg->getValue(0), Arg->getValue(1)});
+ Config.InstallNamesToUpdate.emplace_back(Arg->getValue(0),
+ Arg->getValue(1));
}
SmallVector<StringRef, 2> Positional;
diff --git a/llvm/tools/llvm-objcopy/CopyConfig.h b/llvm/tools/llvm-objcopy/CopyConfig.h
index 1341dd674c7b..ce119dee5bff 100644
--- a/llvm/tools/llvm-objcopy/CopyConfig.h
+++ b/llvm/tools/llvm-objcopy/CopyConfig.h
@@ -178,8 +178,8 @@ struct CopyConfig {
std::vector<StringRef> DumpSection;
std::vector<StringRef> SymbolsToAdd;
std::vector<StringRef> RPathToAdd;
- DenseMap<StringRef, StringRef> RPathsToUpdate;
- DenseMap<StringRef, StringRef> InstallNamesToUpdate;
+ std::vector<std::pair<StringRef, StringRef>> RPathsToUpdate;
+ std::vector<std::pair<StringRef, StringRef>> InstallNamesToUpdate;
DenseSet<StringRef> RPathsToRemove;
// install-name-tool's id option
diff --git a/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp b/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
index 9d0c36630258..3844b6f62de6 100644
--- a/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
+++ b/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
@@ -42,6 +42,35 @@ static StringRef getPayloadString(const LoadCommand &LC) {
.rtrim('\0');
}
+static Error removeLoadCommands(const CopyConfig &Config, Object &Obj) {
+ DenseSet<StringRef> RPathsToRemove(Config.RPathsToRemove.begin(),
+ Config.RPathsToRemove.end());
+
+ LoadCommandPred RemovePred = [&RPathsToRemove](const LoadCommand &LC) {
+ if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH) {
+ StringRef RPath = getPayloadString(LC);
+ if (RPathsToRemove.count(RPath)) {
+ RPathsToRemove.erase(RPath);
+ return true;
+ }
+ }
+ return false;
+ };
+
+ if (Error E = Obj.removeLoadCommands(RemovePred))
+ return E;
+
+ // Emit an error if the Mach-O binary does not contain an rpath path name
+ // specified in -delete_rpath.
+ for (StringRef RPath : Config.RPathsToRemove) {
+ if (RPathsToRemove.count(RPath))
+ return createStringError(errc::invalid_argument,
+ "no LC_RPATH load command with path: %s",
+ RPath.str().c_str());
+ }
+ return Error::success();
+}
+
static Error removeSections(const CopyConfig &Config, Object &Obj) {
SectionPred RemovePred = [](const std::unique_ptr<Section> &) {
return false;
@@ -128,103 +157,6 @@ static LoadCommand buildRPathLoadCommand(StringRef Path) {
return LC;
}
-static Error processLoadCommands(const CopyConfig &Config, Object &Obj) {
- // Remove RPaths.
- DenseSet<StringRef> RPathsToRemove(Config.RPathsToRemove.begin(),
- Config.RPathsToRemove.end());
-
- LoadCommandPred RemovePred = [&RPathsToRemove](const LoadCommand &LC) {
- if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH) {
- StringRef RPath = getPayloadString(LC);
- if (RPathsToRemove.count(RPath)) {
- RPathsToRemove.erase(RPath);
- return true;
- }
- }
- return false;
- };
-
- if (Error E = Obj.removeLoadCommands(RemovePred))
- return E;
-
- // Emit an error if the Mach-O binary does not contain an rpath path name
- // specified in -delete_rpath.
- for (StringRef RPath : Config.RPathsToRemove) {
- if (RPathsToRemove.count(RPath))
- return createStringError(errc::invalid_argument,
- "no LC_RPATH load command with path: %s",
- RPath.str().c_str());
- }
-
- DenseSet<StringRef> RPaths;
-
- // Get all existing RPaths.
- for (LoadCommand &LC : Obj.LoadCommands) {
- if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH)
- RPaths.insert(getPayloadString(LC));
- }
-
- // Throw errors for invalid RPaths.
- for (const auto &OldNew : Config.RPathsToUpdate) {
- StringRef Old, New;
- std::tie(Old, New) = OldNew;
- if (RPaths.count(Old) == 0)
- return createStringError(errc::invalid_argument,
- "no LC_RPATH load command with path: " + Old);
- if (RPaths.count(New) != 0)
- return createStringError(errc::invalid_argument,
- "rpath " + New +
- " would create a duplicate load command");
- }
-
- // Update load commands.
- for (LoadCommand &LC : Obj.LoadCommands) {
- switch (LC.MachOLoadCommand.load_command_data.cmd) {
- case MachO::LC_ID_DYLIB:
- if (Config.SharedLibId) {
- StringRef Id = Config.SharedLibId.getValue();
- if (Id.empty())
- return createStringError(errc::invalid_argument,
- "cannot specify an empty id");
- updateLoadCommandPayloadString<MachO::dylib_command>(LC, Id);
- }
- break;
-
- case MachO::LC_RPATH: {
- StringRef RPath = getPayloadString(LC);
- StringRef NewRPath = Config.RPathsToUpdate.lookup(RPath);
- if (!NewRPath.empty())
- updateLoadCommandPayloadString<MachO::rpath_command>(LC, NewRPath);
- break;
- }
-
- // TODO: Add LC_REEXPORT_DYLIB, LC_LAZY_LOAD_DYLIB, and LC_LOAD_UPWARD_DYLIB
- // here once llvm-objcopy supports them.
- case MachO::LC_LOAD_DYLIB:
- case MachO::LC_LOAD_WEAK_DYLIB:
- StringRef InstallName = getPayloadString(LC);
- StringRef NewInstallName =
- Config.InstallNamesToUpdate.lookup(InstallName);
- if (!NewInstallName.empty())
- updateLoadCommandPayloadString<MachO::dylib_command>(LC,
- NewInstallName);
- break;
- }
- }
-
- // Add new RPaths.
- for (StringRef RPath : Config.RPathToAdd) {
- if (RPaths.count(RPath) != 0)
- return createStringError(errc::invalid_argument,
- "rpath " + RPath +
- " would create a duplicate load command");
- RPaths.insert(RPath);
- Obj.addLoadCommand(buildRPathLoadCommand(RPath));
- }
-
- return Error::success();
-}
-
static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
Object &Obj) {
for (LoadCommand &LC : Obj.LoadCommands)
@@ -341,6 +273,34 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
for (std::unique_ptr<Section> &Sec : LC.Sections)
Sec->Relocations.clear();
+ for (LoadCommand &LC : Obj.LoadCommands) {
+ switch (LC.MachOLoadCommand.load_command_data.cmd) {
+ case MachO::LC_ID_DYLIB:
+ if (Config.SharedLibId) {
+ StringRef Id = Config.SharedLibId.getValue();
+ if (Id.empty())
+ return createStringError(errc::invalid_argument,
+ "cannot specify an empty id");
+ updateLoadCommandPayloadString<MachO::dylib_command>(LC, Id);
+ }
+ break;
+
+ // TODO: Add LC_REEXPORT_DYLIB, LC_LAZY_LOAD_DYLIB, and LC_LOAD_UPWARD_DYLIB
+ // here once llvm-objcopy supports them.
+ case MachO::LC_LOAD_DYLIB:
+ case MachO::LC_LOAD_WEAK_DYLIB:
+ StringRef Old, New;
+ StringRef CurrentInstallName = getPayloadString(LC);
+ for (const auto &InstallNamePair : Config.InstallNamesToUpdate) {
+ std::tie(Old, New) = InstallNamePair;
+ if (CurrentInstallName == Old) {
+ updateLoadCommandPayloadString<MachO::dylib_command>(LC, New);
+ break;
+ }
+ }
+ }
+ }
+
for (const auto &Flag : Config.AddSection) {
std::pair<StringRef, StringRef> SecPair = Flag.split("=");
StringRef SecName = SecPair.first;
@@ -351,9 +311,45 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
return E;
}
- if (Error E = processLoadCommands(Config, Obj))
+ if (Error E = removeLoadCommands(Config, Obj))
return E;
+ StringRef Old, New;
+ for (const auto &OldNew : Config.RPathsToUpdate) {
+ std::tie(Old, New) = OldNew;
+
+ auto FindRPathLC = [&Obj](StringRef RPath) {
+ return find_if(Obj.LoadCommands, [=](const LoadCommand &LC) {
+ return LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH &&
+ getPayloadString(LC) == RPath;
+ });
+ };
+
+ auto NewIt = FindRPathLC(New);
+ if (NewIt != Obj.LoadCommands.end())
+ return createStringError(errc::invalid_argument,
+ "rpath " + New +
+ " would create a duplicate load command");
+
+ auto OldIt = FindRPathLC(Old);
+ if (OldIt == Obj.LoadCommands.end())
+ return createStringError(errc::invalid_argument,
+ "no LC_RPATH load command with path: " + Old);
+
+ updateLoadCommandPayloadString<MachO::rpath_command>(*OldIt, New);
+ }
+
+ for (StringRef RPath : Config.RPathToAdd) {
+ for (LoadCommand &LC : Obj.LoadCommands) {
+ if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH &&
+ RPath == getPayloadString(LC)) {
+ return createStringError(errc::invalid_argument,
+ "rpath " + RPath +
+ " would create a duplicate load command");
+ }
+ }
+ Obj.addLoadCommand(buildRPathLoadCommand(RPath));
+ }
return Error::success();
}
More information about the llvm-commits
mailing list