Temporarily Revert "[llvm-install-name-tool] Merge install-name options" as it breaks the objcopy build.

This reverts commit c143900a0851b2c7b7d52e4825c7f073b3474cf6.

4 files changed, 116 insertions, 124 deletions

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 7b21fdc..1435c6b 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 1fde54dd..f93406f 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 1341dd6..ce119de 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 9d0c366..3844b6f 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();
 }