[llvm] [TextAPI] Track RPaths in the order its provided via command line. (PR #131665)

Cyndy Ishida via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 17 12:45:04 PDT 2025


https://github.com/cyndyishida created https://github.com/llvm/llvm-project/pull/131665

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

>From c7da2a627486a067e6c12f8e21feebd87a9c7eb1 Mon Sep 17 00:00:00 2001
From: Cyndy Ishida <cyndy_ishida at apple.com>
Date: Mon, 17 Mar 2025 12:42:49 -0700
Subject: [PATCH] [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
---
 llvm/lib/TextAPI/InterfaceFile.cpp            |  7 +-
 llvm/lib/TextAPI/TextStubV5.cpp               | 64 ++++++++++++++++---
 .../llvm-readtapi/compare-rpath-order.test    | 14 ++++
 llvm/tools/llvm-readtapi/DiffEngine.cpp       |  4 +-
 4 files changed, 74 insertions(+), 15 deletions(-)
 create mode 100644 llvm/test/tools/llvm-readtapi/compare-rpath-order.test

diff --git a/llvm/lib/TextAPI/InterfaceFile.cpp b/llvm/lib/TextAPI/InterfaceFile.cpp
index ce2feb65c9ec9..e2253723659e2 100644
--- a/llvm/lib/TextAPI/InterfaceFile.cpp
+++ b/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) {
diff --git a/llvm/lib/TextAPI/TextStubV5.cpp b/llvm/lib/TextAPI/TextStubV5.cpp
index b072c0b5d69d0..2e60b065f856c 100644
--- a/llvm/lib/TextAPI/TextStubV5.cpp
+++ b/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(),
diff --git a/llvm/test/tools/llvm-readtapi/compare-rpath-order.test b/llvm/test/tools/llvm-readtapi/compare-rpath-order.test
new file mode 100644
index 0000000000000..0514eb2cfbcd0
--- /dev/null
+++ b/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}
diff --git a/llvm/tools/llvm-readtapi/DiffEngine.cpp b/llvm/tools/llvm-readtapi/DiffEngine.cpp
index 8fc45f436109e..607be0b12c949 100644
--- a/llvm/tools/llvm-readtapi/DiffEngine.cpp
+++ b/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";
 



More information about the llvm-commits mailing list