[PATCH] D56978: Limit dyld image suffixes guessed by guessLibraryShortName()

[Michael Trent via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352104: Limit dyld image suffixes guessed by guessLibraryShortName() (authored by mtrent, committed by ).

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56978/new/

https://reviews.llvm.org/D56978

Files:
  llvm/trunk/lib/Object/MachOObjectFile.cpp
  llvm/trunk/test/Object/Inputs/darwin-m-test3.macho-x86-64
  llvm/trunk/test/Object/nm-darwin-m.test


Index: llvm/trunk/test/Object/nm-darwin-m.test
===================================================================
--- llvm/trunk/test/Object/nm-darwin-m.test
+++ llvm/trunk/test/Object/nm-darwin-m.test
@@ -19,8 +19,8 @@
 
 # This is testing is using darwin-m-test3.macho-x86-64 that is linked with
 # dylibs that have the follow set of -install_names:
-#	Foo.framework/Foo 
-#	/System/Library/Frameworks/FooPath.framework/FooPath 
+#	Foo.framework/Foo
+#	/System/Library/Frameworks/FooPath.framework/FooPath
 #	FooSuffix.framework/FooSuffix_debug
 #	/System/Library/Frameworks/FooPathSuffix.framework/FooPathSuffix_profile
 #	FooVers.framework/Versions/A/FooVers
@@ -32,8 +32,10 @@
 #	/usr/lib/libPathATS.A_profile.dylib
 #	QT.A.qtx
 #	/lib/QTPath.qtx
+#	/usr/lib/libfoo_bar.dylib
+#	/usr/lib/libfoo_bar_profile.dylib
 #	/usr/lib/libSystem.B.dylib
-# to test that MachOObjectFile::guessLibraryShortName() is correctly parsing 
+# to test that MachOObjectFile::guessLibraryShortName() is correctly parsing
 # them into their short names.
 test3: 0000000100000000 (__TEXT,__text) [referenced dynamically] external __mh_execute_header
 test3:                  (undefined) external _atsPathVersSuffix (from libPathATS)
@@ -44,7 +46,9 @@
 test3:                  (undefined) external _fooPathVers (from FooPathVers)
 test3:                  (undefined) external _fooSuffix (from FooSuffix)
 test3:                  (undefined) external _fooVers (from FooVers)
-test3: 0000000100000e60 (__TEXT,__text) external _main
+test3:                  (undefined) external _foo_bar (from libfoo_bar)
+test3:                  (undefined) external _foo_barSuffix (from libfoo_barSuffix)
+test3: 0000000100000e50 (__TEXT,__text) external _main
 test3:                  (undefined) external _qt (from QT)
 test3:                  (undefined) external _qtPath (from QTPath)
 test3:                  (undefined) external _x (from libx)
Index: llvm/trunk/lib/Object/MachOObjectFile.cpp
===================================================================
--- llvm/trunk/lib/Object/MachOObjectFile.cpp
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp
@@ -2241,9 +2241,18 @@
 // one of the two following forms:
 //      libFoo.A.dylib
 //      libFoo.dylib
+//
 // The library may have a suffix trailing the name Foo of the form:
 //      libFoo_profile.A.dylib
 //      libFoo_profile.dylib
+// These dyld image suffixes are separated from the short name by a '_'
+// character. Because the '_' character is commonly used to separate words in
+// filenames guessLibraryShortName() cannot reliably separate a dylib's short
+// name from an arbitrary image suffix; imagine if both the short name and the
+// suffix contains an '_' character! To better deal with this ambiguity,
+// guessLibraryShortName() will recognize only "_debug" and "_profile" as valid
+// Suffix values. Calling code needs to be tolerant of guessLibraryShortName()
+// guessing incorrectly.
 //
 // The Name of the dynamic library is also recognized as a library name if it
 // has the following form:
@@ -2251,7 +2260,6 @@
 //
 // If the Name of the dynamic library is none of the forms above then a NULL
 // StringRef is returned.
-//
 StringRef MachOObjectFile::guessLibraryShortName(StringRef Name,
                                                  bool &isFramework,
                                                  StringRef &Suffix) {
@@ -2271,7 +2279,10 @@
   Idx = Foo.rfind('_');
   if (Idx != Foo.npos && Foo.size() >= 2) {
     Suffix = Foo.slice(Idx, Foo.npos);
-    Foo = Foo.slice(0, Idx);
+    if (Suffix != "_debug" && Suffix != "_profile")
+      Suffix = StringRef();
+    else
+      Foo = Foo.slice(0, Idx);
   }
 
   // First look for the form Foo.framework/Foo
@@ -2332,10 +2343,14 @@
   else
     b = b+1;
   // ignore any suffix after an underbar like Foo_profile.A.dylib
-  Idx = Name.find('_', b);
+  Idx = Name.rfind('_');
   if (Idx != Name.npos && Idx != b) {
     Lib = Name.slice(b, Idx);
     Suffix = Name.slice(Idx, a);
+    if (Suffix != "_debug" && Suffix != "_profile") {
+      Suffix = StringRef();
+      Lib = Name.slice(b, a);
+    }
   }
   else
     Lib = Name.slice(b, a);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56978.183374.patch
Type: text/x-patch
Size: 4165 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190124/19c83ab9/attachment.bin>