Limit dyld image suffixes guessed by guessLibraryShortName()

[mtrent]

Summary

guessLibraryShortName() separates a full Mach-O dylib install name path
into a short name and a dyld image suffix. The short name is the name
of the dylib without its path or extension. The dyld image suffix is a
string used by dyld to load variants of dylibs if available at runtime;
for example, "when binding this process, load 'debug' variants of all
required dylibs." dyld knows exactly what the image suffix is, but
by convention diagnostic tools such as llvm-nm attempt to guess suffix
names by looking at the install name path.

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 previous implementation of guessLibraryShortName() did not allow
'_' characters to appear in short names. When present, the short name
would be truncated, e.g., "libcompiler_rt" => "libcompiler". This
change allows "libcompiler_rt" and "libcompiler_rt_debug" to both be
recognized as "libcompiler_rt".

rdar://47412244

[pete]

LGTM

llvm/trunk/lib/Object/MachOObjectFile.cpp

 // isFramework is set to true else it is set to false.  If the Name has a
 // suffix then Suffix is set to the substring in Name that contains the suffix
 // else it is set to a NULL StringRef.
 //
 // The Name of the dynamic library is recognized as a library name if it has
 // 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:
 //      Foo.qtx
 //
 // 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) {
   StringRef Foo, F, DotFramework, V, Dylib, Lib, Dot, Qtx;
   size_t a, b, c, d, Idx;
 
   isFramework = false;
   Suffix = StringRef();
 
   // Pull off the last component and make Foo point to it
   a = Name.rfind('/');
   if (a == Name.npos || a == 0)
     goto guess_library;
   Foo = Name.slice(a+1, Name.npos);
 
   // Look for a suffix starting with a '_'
   Idx = Foo.rfind('_');
   if (Idx != Foo.npos && Foo.size() >= 2) {
     Suffix = Foo.slice(Idx, Foo.npos);
+    if (Suffix != "_debug" && Suffix != "_profile")
+      Suffix = StringRef();
+    else
       Foo = Foo.slice(0, Idx);
   }
 
   // First look for the form Foo.framework/Foo
   b = Name.rfind('/', a);
   if (b == Name.npos)
     Idx = 0;
   else
     Idx = b+1;
guess_library:
   }
 
   b = Name.rfind('/', a);
   if (b == Name.npos)
     b = 0;
   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);
   // There are incorrect library names of the form:
   // libATS.A_profile.dylib so check for these.
   if (Lib.size() >= 3) {
     Dot = Lib.slice(Lib.size()-2, Lib.size()-1);
     if (Dot == ".")

llvm/trunk/test/Object/nm-darwin-m.test

 test1: 00000008 (__TEXT,__text) non-external [Thumb] _t
 
 # This is testing that an N_INDR symbol gets its alias name, the "(for ...)"
 test2:          (undefined) external __i
 test2:          (indirect) external _i (for __i)
 
 # 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
 # /System/Library/Frameworks/FooPathVers.framework/Versions/B/FooPathVers
 # libx.dylib
 # libxSuffix_profile.dylib
 # /usr/local/lib/libxPathSuffix_debug.dylib
 # libATS.A_profile.dylib
 # /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)
 test3:                  (undefined) external _atsVersSuffix (from libATS)
 test3:                  (undefined) external _foo (from Foo)
 test3:                  (undefined) external _fooPath (from FooPath)
 test3:                  (undefined) external _fooPathSuffix (from FooPathSuffix)
 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)
 test3:                  (undefined) external _xPathSuffix (from libxPathSuffix)
 test3:                  (undefined) external _xSuffix (from libxSuffix)
 test3:                  (undefined) external dyld_stub_binder (from libSystem)