Problems in libltdl - lt_dlerror and lt_dlcaller_get_data

[Christian Holm Christensen]

Hi, 

There's a problem in the documentation and/or the implementation of
lt_dlerror in libltdl.   In the docs, it says: 

   - Function: lt_ptr lt_dlcaller_set_data (lt_dlcaller_id KEY,
          lt_dlhandle HANDLE, lt_ptr DATA)
     Set DATA as the set of data uniquely associated with KEY and
     HANDLE for later retrieval.  This function returns the DATA
     previously associated with KEY and HANDLE if any.  A result of 0,
     may indicate that a diagnostic for the last error (if any) is
     available from `lt_dlerror()'.

     For example, to correctly remove some associated data:

              lt_ptr stale = lt_dlcaller_set_data (key, handle, 0);
              if (stale == NULL)
                {
                  char *error_msg = lt_dlerror ();
          
                  if (error_msg != NULL)
                    {
                      my_error_handler (error_msg);
                      return STATUS_FAILED;
                    }
                }
              else
                {
                  free (stale);
                }


This is wrong.  lt_dlerror _never_ returns a NULL, as is quite evident
from the code (CVS checkout as of Thu Jan 30 18:06:08 2003): 

  1   lt_dlerror ()
  2   {
  3     const char *error;
  4 
  5     LT_DLMUTEX_GETERROR (error);
  6     LT_DLMUTEX_SETERROR (0);
  7 
  8     return error ? error : LT_DLSTRERROR (UNKNOWN);
  9   }

So, on the no-error condition (lt_dllasterr == NULL), lt_dlerror
returns `unknown error'!  That is clearly wrong.  At the least, it
makes the documentation wrong! 

I think that you should change the code in line 8 to simply 

  8     return error;

There's also a problem in lt_dlcallser_get_data.  Suppose you want to
do something like 

   if (lt_dlcaller_get_data (id, handle))
     lt_dlcaller_set_data(id, handle, data); 

If you haven't set the user data yet, this code results in a
SIGSEGV. Again, the problem is obvious from the code: 

  1   lt_dlcaller_get_data  (key, handle)
  2        lt_dlcaller_id key;
  3        lt_dlhandle handle;
  4   {
  5     lt_ptr result = (lt_ptr) 0;
  6   
  7     /* This needs to be locked so that the caller data isn't updated by
  8        another thread part way through this function.  */
  9     LT_DLMUTEX_LOCK ();
  10  
  11    /* Locate the index of the element with a matching KEY.  */
  12    {
  13      int i;
  14      for (i = 0; handle->caller_data[i].key; ++i)
  15        {
  16          if (handle->caller_data[i].key == key)
  17            {
  18              result = handle->caller_data[i].data;
  19              break;
  20            }
  21        }
  22    }
  23  
  24    LT_DLMUTEX_UNLOCK ();
  25  
  26    return result;
  27  }
  
The problem is, that first off, the passed handle could be NULL, in
which case you'd try to dereference an invalid pointer in line 14.

Second off, even if handle is a valid pointer, the dereference of the
caller_data field by the array subscript will result in a SIGSEGV if
lt_dlcaller_set_data on the handle previously.  

I think you should change the function to 

  1   lt_dlcaller_get_data  (key, handle)
  2        lt_dlcaller_id key;
  3        lt_dlhandle handle;
  4   {
  5     lt_ptr result = (lt_ptr) 0;
  6   
  7     /* This needs to be locked so that the caller data isn't updated by
  8        another thread part way through this function.  */
  9     LT_DLMUTEX_LOCK ();
  10  
  11    /* Locate the index of the element with a matching KEY.  */
        if (handle && handle->caller_data)
  12    {
  13      int i;
  14      for (i = 0; handle->caller_data[i].key; ++i)
  15        {
  16          if (handle->caller_data[i].key == key)
  17            {
  18              result = handle->caller_data[i].data;
  19              break;
  20            }
  21        }
  22    }
  23  
  24    LT_DLMUTEX_UNLOCK ();
  25  
  26    return result;
  27  }

so if lt_dlcaller_set_data wasn't previously called on the handle with
that at all, it will return NULL. 

These errors crept up while I was making a set C++ libraries to
interface the libltdl library.  I want to have the classes `loader'
which represents the dynamic loader (lt_dlopen, lt_dlclose,
lt_dlsetsearchpath, ...), and a class `handle' that represents the
handles on the libraries (lt_dlsym, lt_dlinfo, ...).  To ensure that
all clients get the same `handle', I store a pointer the handle object
as the user data: 

  class loader {
  protected:
    lt_dlcaller_id _id; 
    loader() {
      _id = lt_dlcaller_register();
      ...
     }
 
  public:  
    const handle& load(const std::string& filename) const {
      lt_dlhandle lt_h;
      if (filename.empty()) lt_h = lt_dlopen(0); 
      else                  lt_h = lt_dlopenext(filename.c_str());
      if (!lt_h) throw std::runtime_error(lt_dlerror()); 
      
      handle* h;
      if (!(h = static_cast<handle*>(lt_dlcaller_get_data(_id,lt_h)))) {
        h = new handle(lt_h);
        lt_ptr old = lt_dlcaller_set_data(_id, lt_h, h);
        if (!old) {
          const char* err = lt_dlerror();
          if (err) throw std::runtime_error(err);
        }
      }
      return *h;
    }
    ...
  };

In this way, I make sure the application always get the right handle,
and that it cannot change it to something else.  The above is how I
would have coded it from the documentation, but the above errors made
me do 

    const handle& load(const std::string& filename) const {
      lt_dlhandle lt_h;
      if (filename.empty()) lt_h = lt_dlopen(0); 
      else                  lt_h = lt_dlopenext(filename.c_str());
      if (!lt_h) throw std::runtime_error(lt_dlerror()); 
      
      handle* h;
      const lt_dlnfo* i = lt_dlgetinfo(lt_h); 
      if (i->ref_count == 1 || 
          !(h = static_cast<handle*>(lt_dlcaller_get_data(_id,lt_h)))) {
        h = new handle(lt_h);
        lt_ptr old = lt_dlcaller_set_data(_id, lt_h, h);
      }
      return *h;
    }

That is, I had to get the information on the handle (which I would
have preferred to postpone until it was needed), and there's no error
checking on call to lt_dlcaller_set_data.  

I'll post my C++ libraries from my web-page tomorrow CET (don't have them
right here I'm afraid).  You'll find them at 

  http://cholm.home.cern.ch/cholm/misc/#ltmm

Fell free to comment on it. 

Yours, 

 ___  |  Christian Holm Christensen 
  |_| |  -------------------------------------------------------------
    | |  Address: Sankt Hansgade 23, 1. th.  Phone:  (+45) 35 35 96 91
     _|           DK-2200 Copenhagen N       Cell:   (+45) 24 61 85 91
    _|            Denmark                    Office: (+45) 353  25 305
 ____|   Email:   address@hidden               Web:    www.nbi.dk/~cholm
 | |