[ISSUE-131] Library: patch possible misuse of find_library() (#132)

On Linux, the Library.load_default() function fails to properly resolve the JLink
shared library if not installed in `/opt/SEGGER/JLink`, regardless of the value
of `LD_LIBRARY_PATH`.

This is possibly caused by an improper use of the ctypes `find_library()` API, which:
- On all platforms expects as parameter the library name without any prefix like lib,
   suffix like `.so`, `.dylib` or version number
- Returns the full file path on MacOS and Windows, but only the file name (so name)
   on Linux.

Changes introduced by this patch:
- On Linux `find_library('jlinkarm')` will return the resolved soname, for e.g. 
   `libjlinkarm.so.7`, and we'll use the native `dlinfo()` API to retrieve the full file path.

pylink/library.py

@@ -17,6 +17,7 @@
 import ctypes
 import ctypes.util as ctypes_util
 import os
+import platform
 import sys
 import tempfile
 
@@ -82,11 +83,29 @@ class Library(object):
         'JLINK_SetFlashProgProgressCallback'
     ]
 
-    JLINK_SDK_NAME = 'libjlinkarm'
+    # Linux/MacOS: The JLink library name without any prefix like lib,
+    # suffix like .so, .dylib or version number.
+    JLINK_SDK_NAME = 'jlinkarm'
 
+    # Linux/MacOS: The library file name will start with 'libjlinkarm'
+    # Used by Library.find_library_{linux,darwin}()
+    JLINK_SDK_STARTS_WITH = 'libjlinkarm'
+
+    # Windows: these are suitable for both the ctypes find_library() API,
+    # and for the directory scanning done in Library.find_library_windows()
     WINDOWS_32_JLINK_SDK_NAME = 'JLinkARM'
     WINDOWS_64_JLINK_SDK_NAME = 'JLink_x64'
 
+    # Linux: Represents the dlinfo(3) associated to the jlinkarm shared library.
+    #
+    # Poor man's singleton: having this as a class member avoids to create
+    # a new JLinkarmDlInfo() for each pylink.Library instance.
+    #
+    # For e.g., the simple 'pyocd list' command creates three instances
+    # of the pylink.Library class, and it's worth not repeating the dlinfo()
+    # dance three times.
+    _dlinfo = None
+
     @classmethod
     def get_appropriate_windows_sdk_name(cls):
         """Returns the appropriate JLink SDK library name on Windows depending
@@ -157,7 +176,7 @@ def find_library_linux(cls):
           The paths to the J-Link library files in the order that they are
           found.
         """
-        dll = Library.JLINK_SDK_NAME
+        dll = Library.JLINK_SDK_STARTS_WITH
         root = os.path.join('/', 'opt', 'SEGGER')
 
         for (directory_name, subdirs, files) in os.walk(root):
@@ -206,7 +225,7 @@ def find_library_darwin(cls):
         Returns:
           The path to the J-Link library files in the order they are found.
         """
-        dll = Library.JLINK_SDK_NAME
+        dll = Library.JLINK_SDK_STARTS_WITH
         root = os.path.join('/', 'Applications', 'SEGGER')
         if not os.path.isdir(root):
             return
@@ -283,7 +302,27 @@ def load_default(self):
         Returns:
           ``True`` if the DLL was loaded, otherwise ``False``.
         """
+
+        # Request the underlying operating system, through ctypes,
+        # to resolve the J-Link DLL "the standard way" by its
+        # library name.
         path = ctypes_util.find_library(self._sdk)
+
+        # On Linux, find_library() actually returns the soname,
+        # so we've got something like path = 'libjlinkarm.so.7',
+        # and now have to retrieve the absolute file path.
+        if (path is not None) and sys.platform.startswith('linux'):
+            # For this, we'll rely on dlinfo(), which is not a POSIX API,
+            # but a GNU libc extension.
+            if platform.libc_ver()[0] == 'glibc':
+                if Library._dlinfo is None:
+                    Library._dlinfo = JLinkarmDlInfo(path)
+                path = Library._dlinfo.path
+            else:
+                # When GNU libc extensions aren't available,
+                # continue as if find_library() had failed.
+                path = None
+
         if path is None:
             # Couldn't find it the standard way.  Fallback to the non-standard
             # way of finding the J-Link library.  These methods are operating
@@ -423,3 +462,116 @@ def dll(self):
           A ``ctypes`` DLL instance if one was loaded, otherwise ``None``.
         """
         return self._lib
+
+
+class JLinkarmDlInfo:
+    """Helper to retrieve the absolute path of the JLink library (aka DLL)
+    based on its soname.
+
+    This is used on Linux, where ctypes.util.find_library() will not return
+    the library full file path, but only the file name (aka soname).
+    We'll then rely on the native dlinfo() API to retrieve the library absolute
+    file path.
+
+    For e.g.:
+    - LD_LIBRARY_PATH=/mnt/platform/segger/JLink
+    - ctypes.util.find_library('jlinkarm') -> libjlinkarm.so.7
+    - JLinkarmDlInfo('libjlinkarm.so.7').path -> /mnt/platform/segger/JLink/libjlinkarm.so.7
+
+    Note that unlike dlopen(), dlsym() and friends, dlinfo() is not a POSIX API,
+    but a GNU extension, available only on systems with the GNU libc implementation (glibc).
+
+    The dlinfo() dance implementation is adapted from @cloudflightio,
+    https://github.com/cloudflightio/python-dlinfo.
+    """
+
+    # Request to obtain a pointer to the link_map structure corresponding
+    # to a given handle (Linux).
+    # See: man dlinfo(3)
+    RTLD_DI_LINKMAP = 2
+
+    # dlinfo(3): struct link_map, where l_name will be the file path.
+    class LinkMap(ctypes.Structure):
+        """
+        Represents a C struct link_map (Linux).
+        See: man dlinfo(3)
+        """
+        _fields_ = [
+            ('l_addr', ctypes.c_void_p),
+            ('l_name', ctypes.c_char_p),
+            ('l_ld', ctypes.c_void_p),
+            ('l_next', ctypes.c_void_p),
+            ('l_prev', ctypes.c_void_p),
+        ]
+
+    def __init__(self, jlinkarm_soname):
+        """Retrieves the absolute file path of the JLink library (aka DLL)
+        corresponding to the given soname.
+
+        This runs the dlinfo() dance using the ctypes API:
+        - loads the JLink DLL shared object for given soname
+        - loads the dl library and lookup the dlinfo symbol
+        - calls dlinfo() with request RTLD_DI_LINKMAP to get the struct link_map
+          for the JLink library
+        - access the struct content to retrieve the library's absolute path
+
+        The dlinfo() dance implementation does not try to hide any OSError
+        the ctypes API may raise, since this would likely hide a host
+        system configuration issue (aka "should not happen").
+
+        Args:
+        - jlinkarm_soname: The JLink DLL soname returned by find_library(),
+          for e.g. 'libjlinkarm.so.7'.
+
+        Raises:
+          - OSError when the actual library file has been removed, is not readable,
+            is not a loadable shared object, etc.
+        """
+        # JLink DLL's absolute file path.
+        self._dll_path = None
+
+        # We're using the soname returned by a successful call to find_library(),
+        # hence we can expect LoadLibrary() to in turn successfully load the JLink DLL.
+        tmp_cdll_jlink = ctypes.cdll.LoadLibrary(jlinkarm_soname)
+
+        # dlinfo() dance to retrieve the library's absolute file path.
+        #
+        # This code path should be involved only for POSIX systems
+        # with GNU libc, where:
+        # - libdl is available (POSIX)
+        # - dlinfo() is defined (glibc)
+        #
+        # Hence bellow calls to find_library(), LoadLibrary(), and dlinfo() should succeed.
+        dl_soname = ctypes_util.find_library('dl')
+        if dl_soname is not None:
+            tmp_cdll_dl = ctypes.cdll.LoadLibrary(dl_soname)
+            dlinfo = tmp_cdll_dl.dlinfo
+            dlinfo.argtypes = ctypes.c_void_p, ctypes.c_int, ctypes.c_void_p
+            dlinfo.restype = ctypes.c_int
+
+            linkmap = ctypes.c_void_p()
+            if dlinfo(tmp_cdll_jlink._handle, JLinkarmDlInfo.RTLD_DI_LINKMAP, ctypes.byref(linkmap)) == 0:
+                linkmap = ctypes.cast(linkmap, ctypes.POINTER(JLinkarmDlInfo.LinkMap))
+                self._dll_path = linkmap.contents.l_name.decode(sys.getdefaultencoding())
+
+            # "Free" tmp dl library
+            del tmp_cdll_dl
+            tmp_cdll_dl = None
+
+        else:
+            # Should not happen.
+            pass
+
+        # "Free" tmp jlinkarm library
+        del tmp_cdll_jlink
+        tmp_cdll_jlink = None
+
+    @property
+    def path(self):
+        """Answers the JLink DLL file path.
+
+        Returns the JLink DLL's absolute path,
+        or None when the dl library is unavailable despite the system
+        presenting itself as POSIX.
+        """
+        return self._dll_path

tests/unit/test_library.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from platform import platform
 import pylink.library as library
 import pylink.util as util
 
@@ -949,6 +950,129 @@ def test_linux_empty(self, mock_os, mock_load_library, mock_find_library, mock_o
         self.assertEqual(1, mock_find_library.call_count)
         self.assertEqual(0, mock_load_library.call_count)
 
+    @mock.patch('os.name', new='posix')
+    @mock.patch('sys.platform', new='linux')
+    @mock.patch('tempfile.NamedTemporaryFile', new=mock.Mock())
+    @mock.patch('os.remove', new=mock.Mock())
+    @mock.patch('pylink.library.open')
+    @mock.patch('pylink.library.os')
+    @mock.patch('pylink.util.is_os_64bit', return_value=True)
+    @mock.patch('pylink.platform.libc_ver', return_value=('libc', '1.0'))
+    @mock.patch('ctypes.util.find_library', return_value='libjlinkarm.so.7')
+    @mock.patch('pylink.library.JLinkarmDlInfo.__init__')
+    @mock.patch('ctypes.cdll.LoadLibrary')
+    def test_linux_glibc_unavailable(self, mock_load_library, mock_dlinfo_ctr, mock_find_library,
+                                     mock_libc_ver, mock_is_os_64bit, mock_os,  mock_open):
+        """Confirms the whole JLinkarmDlInfo code path is not involved when GNU libc
+        extensions are unavailable on a Linux system, and that we'll successfully fallback
+        to the "search by file name".
+
+        Test case:
+        - initial find_library('jlinkarm') succeeds
+        - but the host system does not provide GNU libc extensions
+        - we should then skip the dlinfo() dance and proceed
+          to the "search by file name" code path, aka find_library_linux()
+        - and "successfully load" a mock library file from /opt/SEGGER/JLink
+        """
+        directories = [
+            # Library.find_library_linux() should find this.
+            '/opt/SEGGER/JLink/libjlinkarm.so.6'
+        ]
+        self.mock_directories(mock_os, directories, '/')
+
+        lib = library.Library()
+        lib.unload = mock.Mock()
+
+        mock_find_library.assert_called_once_with(library.Library.JLINK_SDK_NAME)
+        # JLinkarmDlInfo has not been instantiated.
+        self.assertEquals(0, mock_dlinfo_ctr.call_count)
+        # Fallback to "search by file name" has succeeded.
+        self.assertEquals(1, mock_load_library.call_count)
+        self.assertEqual(directories[0], lib._path)
+
+    @mock.patch('os.name', new='posix')
+    @mock.patch('sys.platform', new='linux')
+    @mock.patch('tempfile.NamedTemporaryFile', new=mock.Mock())
+    @mock.patch('os.remove', new=mock.Mock())
+    @mock.patch('pylink.library.open')
+    @mock.patch('pylink.library.os')
+    @mock.patch('pylink.util.is_os_64bit', return_value=True)
+    @mock.patch('pylink.platform.libc_ver', return_value=('glibc', '2.34'))
+    @mock.patch('ctypes.util.find_library')
+    @mock.patch('ctypes.cdll.LoadLibrary')
+    def test_linux_dl_unavailable(self, mock_load_library, mock_find_library, mock_libc_ver,
+                                  mock_is_os_64bit, mock_os,  mock_open):
+        """Confirms we successfully fallback to the "search by file name" code path when libdl is
+        unavailable despite the host system presenting itself as POSIX (GNU/Linux).
+
+        Test case:
+        - initial find_library('jlinkarm') succeeds
+        - the host system presents itself as GNU/Linux, but does not provide libdl
+        - we should then skip the dlinfo() dance and proceed
+          to the "search by file name" code path, aka find_library_linux()
+        - and "successfully load" a mock library file from /opt/SEGGER/JLink
+        """
+        mock_find_library.side_effect = [
+            # find_library('jlinkarm')
+            'libjlinkarm.so.6',
+            # find_library('dl')
+            None
+        ]
+
+        directories = [
+            '/opt/SEGGER/JLink/libjlinkarm.so.6'
+        ]
+        self.mock_directories(mock_os, directories, '/')
+
+        lib = library.Library()
+        lib.unload = mock.Mock()
+
+        mock_find_library.assert_any_call(library.Library.JLINK_SDK_NAME)
+        mock_find_library.assert_any_call('dl')
+        self.assertEquals(2, mock_find_library.call_count)
+        # Called once in JLinkarmDlInfo and once in Library.
+        self.assertEquals(2, mock_load_library.call_count)
+        # The dlinfo() dance silently failed, but will answer None resolved path.
+        self.assertIsNone(library.Library._dlinfo.path)
+        # Fallback to "search by file name" has succeeded.
+        self.assertEqual(directories[0], lib._path)
+
+    @mock.patch('os.name', new='posix')
+    @mock.patch('sys.platform', new='linux')
+    @mock.patch('pylink.platform.libc_ver', return_value=('glibc', '2.34'))
+    @mock.patch('ctypes.util.find_library')
+    @mock.patch('ctypes.cdll.LoadLibrary')
+    def test_linux_dl_oserror(self, mock_load_library, mock_find_library, mock_libc_ver):
+        """Confirms ctype API exceptions actually propagate from JLinkarmDlInfo to call site.
+
+        Test case:
+        - initial find_library('jlinkarm') succeeds
+        - the host system presents itself as GNU/Linux, and we successfully load libdl
+        - but loading libdl raises OSError
+        """
+
+        mock_find_library.side_effect = [
+            # find_library('jlinkarm')
+            'libjlinkarm.so.6',
+            # find_library('dl')
+            'libdl.so.2'
+        ]
+        mock_load_library.side_effect = [
+            # load JLink DLL
+            mock.Mock(),
+            # load libdl
+            OSError()
+        ]
+
+        with self.assertRaises(OSError):
+            lib = library.Library()
+            lib.unload = mock.Mock()
+
+        mock_find_library.assert_any_call(library.Library.JLINK_SDK_NAME)
+        mock_find_library.assert_any_call('dl')
+        self.assertEquals(2, mock_find_library.call_count)
+        self.assertEquals(2, mock_load_library.call_count)
+
 
 if __name__ == '__main__':
     unittest.main()