[TextAPI] Track RPaths in the order its provided via command line.

RPaths are basically search paths for how to load dependent libraries.
The order they appear is the order the linker will search, we should
preserve that order in tbd files.

* Additionally add this level of detection to llvm-readtapi.

resolves: rdar://145603347

Author: cyndyishida

llvm/lib/TextAPI/InterfaceFile.cpp

@@ -59,14 +59,11 @@ void InterfaceFile::addRPath(StringRef RPath, const Target &InputTarget) {
     return;
   using RPathEntryT = const std::pair<Target, std::string>;
   RPathEntryT Entry(InputTarget, RPath);
-  auto Iter =
-      lower_bound(RPaths, Entry,
-                  [](RPathEntryT &LHS, RPathEntryT &RHS) { return LHS < RHS; });
 
-  if ((Iter != RPaths.end()) && (*Iter == Entry))
+  if (is_contained(RPaths, Entry))
     return;
 
-  RPaths.emplace(Iter, Entry);
+  RPaths.emplace_back(Entry);
 }
 
 void InterfaceFile::addTarget(const Target &Target) {

llvm/lib/TextAPI/TextStubV5.cpp

@@ -83,6 +83,33 @@ using AttrToTargets = std::map<std::string, TargetList>;
 using TargetsToSymbols =
     SmallVector<std::pair<TargetList, std::vector<JSONSymbol>>>;
 
+/// Wrapper over a vector for handling textstub attributes, mapped to target
+/// triples, that require insertion order to be intact in the resulting \c
+/// InterfaceFile.
+class InOrderAttrToTargets {
+  using EntryT = std::pair<std::string, TargetList>;
+
+public:
+  void insert(EntryT &&Entry) {
+    auto &Element = get(Entry.first);
+    Element.second = Entry.second;
+  }
+
+  const EntryT *begin() { return Container.begin(); }
+  const EntryT *end() { return Container.end(); }
+
+private:
+  EntryT &get(std::string &Key) {
+    auto *It = find_if(Container,
+                       [&Key](EntryT &Input) { return Input.first == Key; });
+    if (It != Container.end())
+      return *It;
+    Container.push_back(EntryT(Key, {}));
+    return Container.back();
+  }
+  llvm::SmallVector<EntryT> Container;
+};
+
 enum TBDKey : size_t {
   TBDVersion = 0U,
   MainLibrary,
@@ -437,14 +464,14 @@ Expected<TargetsToSymbols> getSymbolSection(const Object *File, TBDKey Key,
   return std::move(Result);
 }
 
-Expected<AttrToTargets> getLibSection(const Object *File, TBDKey Key,
-                                      TBDKey SubKey,
-                                      const TargetList &Targets) {
+template <typename ReturnT = AttrToTargets>
+Expected<ReturnT> getLibSection(const Object *File, TBDKey Key, TBDKey SubKey,
+                                const TargetList &Targets) {
   auto *Section = File->getArray(Keys[Key]);
   if (!Section)
-    return AttrToTargets();
+    return ReturnT();
 
-  AttrToTargets Result;
+  ReturnT Result;
   TargetList MappedTargets;
   for (auto Val : *Section) {
     auto *Obj = Val.getAsObject();
@@ -460,7 +487,7 @@ Expected<AttrToTargets> getLibSection(const Object *File, TBDKey Key,
     }
     auto Err =
         collectFromArray(SubKey, Obj, [&Result, &MappedTargets](StringRef Key) {
-          Result[Key.str()] = MappedTargets;
+          Result.insert({Key.str(), MappedTargets});
         });
     if (Err)
       return std::move(Err);
@@ -629,10 +656,11 @@ Expected<IFPtr> parseToInterfaceFile(const Object *File) {
     return RLOrErr.takeError();
   AttrToTargets ReexportLibs = std::move(*RLOrErr);
 
-  auto RPathsOrErr = getLibSection(File, TBDKey::RPath, TBDKey::Paths, Targets);
+  auto RPathsOrErr = getLibSection<InOrderAttrToTargets>(
+      File, TBDKey::RPath, TBDKey::Paths, Targets);
   if (!RPathsOrErr)
     return RPathsOrErr.takeError();
-  AttrToTargets RPaths = std::move(*RPathsOrErr);
+  InOrderAttrToTargets RPaths = std::move(*RPathsOrErr);
 
   auto ExportsOrErr = getSymbolSection(File, TBDKey::Exports, Targets);
   if (!ExportsOrErr)
@@ -802,6 +830,8 @@ Array serializeAttrToTargets(AggregateT &Entries, TBDKey Key) {
   return Container;
 }
 
+/// When there is no significance in order, the common case, serialize all
+/// attributes in a stable order.
 template <typename ValueT = std::string,
           typename AggregateT = std::vector<std::pair<MachO::Target, ValueT>>>
 Array serializeField(TBDKey Key, const AggregateT &Values,
@@ -834,6 +864,21 @@ Array serializeField(TBDKey Key, const std::vector<InterfaceFileRef> &Values,
   return serializeAttrToTargets(FinalEntries, Key);
 }
 
+template <
+    typename AggregateT = std::vector<std::pair<MachO::Target, std::string>>>
+Array serializeFieldInInsertionOrder(TBDKey Key, const AggregateT &Values,
+                                     const TargetList &ActiveTargets) {
+  MapVector<StringRef, std::set<MachO::Target>> Entries;
+  for (const auto &[Target, Val] : Values)
+    Entries[Val].insert(Target);
+
+  TargetsToValuesMap FinalEntries;
+  for (const auto &[Val, Targets] : Entries)
+    FinalEntries[serializeTargets(Targets, ActiveTargets)].emplace_back(
+        Val.str());
+  return serializeAttrToTargets(FinalEntries, Key);
+}
+
 struct SymbolFields {
   struct SymbolTypes {
     std::vector<StringRef> Weaks;
@@ -963,7 +1008,8 @@ Expected<Object> serializeIF(const InterfaceFile *File) {
       TBDKey::ABI, File->getSwiftABIVersion(), 0u);
   insertNonEmptyValues(Library, TBDKey::SwiftABI, std::move(SwiftABI));
 
-  Array RPaths = serializeField(TBDKey::Paths, File->rpaths(), ActiveTargets);
+  Array RPaths = serializeFieldInInsertionOrder(TBDKey::Paths, File->rpaths(),
+                                                ActiveTargets);
   insertNonEmptyValues(Library, TBDKey::RPath, std::move(RPaths));
 
   Array Umbrellas = serializeField(TBDKey::Umbrella, File->umbrellas(),

llvm/test/tools/llvm-readtapi/compare-rpath-order.test

@@ -0,0 +1,14 @@
+; RUN: rm -rf %t
+; RUN: split-file %s %t  
+; RUN: not llvm-readtapi --compare %t/rpaths_diff_order.tbd  %t/rpaths.tbd 2>&1 | FileCheck %s 
+
+; CHECK: < {{.*}}rpaths_diff_order.tbd
+; CHECK: > {{.*}}rpaths.tbd
+
+; CHECK: 'Run Path Search Paths' differ by order
+
+//--- rpaths_diff_order.tbd
+{"main_library":{"exported_symbols":[{"text":{"global":["_foo"]}}],"rpaths": [{"paths": ["/usr/lib/swift", "@loader_path/../../../"]}], "flags":[{"attributes":["not_app_extension_safe"]}],"install_names":[{"name":"@rpath/libFake.dylib"}],"target_info":[{"min_deployment":"13","target":"x86_64-macos"},{"min_deployment":"13","target":"arm64-macos"}]},"tapi_tbd_version":5}
+
+//--- rpaths.tbd
+{"main_library":{"exported_symbols":[{"text":{"global":["_foo"]}}],"rpaths": [{"paths": [ "@loader_path/../../../", "/usr/lib/swift"]}], "flags":[{"attributes":["not_app_extension_safe"]}],"install_names":[{"name":"@rpath/libFake.dylib"}],"target_info":[{"min_deployment":"13","target":"x86_64-macos"},{"min_deployment":"13","target":"arm64-macos"}]},"tapi_tbd_version":5}

llvm/tools/llvm-readtapi/DiffEngine.cpp

@@ -449,8 +449,10 @@ template <typename T> void sortTargetValues(std::vector<T> &TargValues) {
 
 template <typename T>
 void printVecVal(std::string Indent, const DiffOutput &Attr, raw_ostream &OS) {
-  if (Attr.Values.empty())
+  if (Attr.Values.empty()) {
+    OS << Indent << "'" << Attr.Name << "' differ by order\n";
     return;
+  }
 
   OS << Indent << Attr.Name << "\n";