[llvm] c143900 - [llvm-install-name-tool] Merge install-name options
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 6 15:41:55 PDT 2020
Hi Sameer,
This broke the build :) There are a lot of complaining build bots you can
take a look at.
I've temporarily reverted it here:
echristo at athyra ~/s/llvm-project> git push
To github.com:llvm/llvm-project.git
1e495e10e6c..4029f8ede42 master -> master
Thanks! and sorry for the inconvenience
-eric
On Mon, Jul 6, 2020 at 3:15 PM Sameer Arora via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
>
> Author: Sameer Arora
> Date: 2020-07-06T15:15:20-07:00
> New Revision: c143900a0851b2c7b7d52e4825c7f073b3474cf6
>
> URL:
> https://github.com/llvm/llvm-project/commit/c143900a0851b2c7b7d52e4825c7f073b3474cf6
> DIFF:
> https://github.com/llvm/llvm-project/commit/c143900a0851b2c7b7d52e4825c7f073b3474cf6.diff
>
> LOG: [llvm-install-name-tool] Merge install-name options
>
> This diff merges all options for llvm-install-name-tool under a single
> function processLoadCommands. Also adds another test case for -add_rpath
> option.
>
> Test plan: make check-all
>
> Reviewed by: jhenderson, alexshap, smeenai, Ktwu
>
> Differential Revision: https://reviews.llvm.org/D82812
>
> 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 1435c6b744c8..7b21fdc2e03c 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,6 +22,13 @@
>
> # 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 f93406f371d0..1fde54dd290a 100644
> --- a/llvm/tools/llvm-objcopy/CopyConfig.cpp
> +++ b/llvm/tools/llvm-objcopy/CopyConfig.cpp
> @@ -874,42 +874,39 @@ 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 std::pair<StringRef, StringRef>
> &OldNew) {
> - return Match(OldNew.first) ||
> Match(OldNew.second);
> - });
> + auto It1 = find_if(
> + Config.RPathsToUpdate,
> + [&Match](const DenseMap<StringRef, StringRef>::value_type
> &OldNew) {
> + return Match(OldNew.getFirst()) || Match(OldNew.getSecond());
> + });
> if (It1 != Config.RPathsToUpdate.end())
> - 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());
> + return createStringError(errc::invalid_argument,
> + "cannot specify both -rpath " +
> It1->getFirst() +
> + " " + It1->getSecond() + " and -rpath
> " +
> + Old + " " + New);
>
> // 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 %s and -rpath %s %s",
> - It2->str().c_str(), Old.str().c_str(), New.str().c_str());
> + return createStringError(errc::invalid_argument,
> + "cannot specify both -delete_rpath " +
> *It2 +
> + " and -rpath " + Old + " " + New);
>
> // 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 %s and -rpath %s %s",
> - It3->str().c_str(), Old.str().c_str(), New.str().c_str());
> + return createStringError(errc::invalid_argument,
> + "cannot specify both -add_rpath " + *It3 +
> + " and -rpath " + Old + " " + New);
>
> - Config.RPathsToUpdate.emplace_back(Old, New);
> + Config.RPathsToUpdate.insert({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.emplace_back(Arg->getValue(0),
> - Arg->getValue(1));
> + Config.InstallNamesToUpdate.insert({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 ce119dee5bff..1341dd674c7b 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;
> - std::vector<std::pair<StringRef, StringRef>> RPathsToUpdate;
> - std::vector<std::pair<StringRef, StringRef>> InstallNamesToUpdate;
> + DenseMap<StringRef, StringRef> RPathsToUpdate;
> + DenseMap<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 3844b6f62de6..9d0c36630258 100644
> --- a/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
> +++ b/llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
> @@ -42,35 +42,6 @@ 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;
> @@ -157,6 +128,103 @@ 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)
> @@ -273,34 +341,6 @@ 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;
> @@ -311,45 +351,9 @@ static Error handleArgs(const CopyConfig &Config,
> Object &Obj) {
> return E;
> }
>
> - if (Error E = removeLoadCommands(Config, Obj))
> + if (Error E = processLoadCommands(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();
> }
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200706/480600e2/attachment.html>
More information about the llvm-commits
mailing list