Luabind 0.9.1 cast graph cache bug

From: Peter Colberg <[email protected]>
Date: Sat, 30 Jul 2011 12:33:42 -0400

This patch fixes a bug in the cast graph cache in Luabind 0.9.1. The
problem did not manifest itself in earlier versions, but only became
apparent with Luabind commit 8e9bb9c7, which fixed a different cast
cache bug.

When a cast from a Lua value to a C++ object fails due to mismatching
types, e.g. upon function overload resolution, or explicit object_cast,
Luabind stores a bogus entry in the cast graph cache. A second cast to
the same C++ type will then erroneously yield a successful cast despite
the mismatching types. This results in e.g. ambiguous overload errors
for overloaded functions despite non-ambiguity, or segmentation faults
due to access of a C++ object through a pointer with the wrong type.

The bug has first been discovered and fixed by Eduard Mueller:

http://old.nabble.com/inheritance-cast_graph-cache-bug-fix-p29936577.html

# diff --git a/src/inheritance.cpp b/src/inheritance.cpp 
# index 2e2ec90..b8467ad 100644 
# --- a/src/inheritance.cpp 
# +++ b/src/inheritance.cpp 
# @@ -190,7 +190,7 @@ std::pair<void*, int> cast_graph::impl::cast( 
#          } 
#      } 
#
# -    m_cache.put(src, target, dynamic_id, object_offset, cache::invalid, -1); 
# +    m_cache.put(src, target, dynamic_id, object_offset, 0, cache::invalid); 
#
#      return std::pair<void*, int>((void*)0, -1); 
#  } 

The above patch switches the arguments to store a correct cache entry
with offset equal to cache::invalid. The distance value is irrelevant,
as it is discarded in cast_graph::impl::cast(). Presumably the above
patch changes it from -1 to 0 to account for the std::size_t type used
in the signature of cache::put.

The patch below takes a different approach, as already suggested by the
author of the above patch. We swap the offset and distance arguments of
cache::put to match the order of the cache entry pair. Further, we
change the distance type in the cache::put signature from std::size_t
to int, to be consistent with the cache entry type. This inconsistency
stems from the changes in Luabind commit cf37774.

Note that under GNU/Linux, the cast graph cache bug manifests itself
on the x86 platform, but does not seem to affect Luabind on x86_64.

--- luabind-0.9.1.orig/src/inheritance.cpp
+++ luabind-0.9.1/src/inheritance.cpp
@@ -64,7 +64,7 @@
       void put(
           class_id src, class_id target, class_id dynamic_id
         , std::ptrdiff_t object_offset
-        , std::size_t distance, std::ptrdiff_t offset);
+        , std::ptrdiff_t offset, int distance);
 
       void invalidate();
 
@@ -90,7 +90,7 @@
 
   void cache::put(
       class_id src, class_id target, class_id dynamic_id
-    , std::ptrdiff_t object_offset, std::size_t distance, std::ptrdiff_t offset)
+    , std::ptrdiff_t object_offset, std::ptrdiff_t offset, int distance)
   {
       m_cache.insert(std::make_pair(
           key_type(src, target, dynamic_id, object_offset)
@@ -175,7 +175,7 @@
         {
             m_cache.put(
                 src, target, dynamic_id, object_offset
-              , qe.distance, (char*)qe.p - (char*)p
+              , (char*)qe.p - (char*)p, qe.distance
             );
 
             return std::make_pair(qe.p, qe.distance);