Re: [boost] [serialization] Problem with diamond hierarchy

From: Loïc Joly (loic.joly_at_[hidden])
Date: 2006-01-30 13:40:23

Loïc Joly a écrit :

Ok, this looks like it is definitely a bug in boost::serialization, due 
to the layout of objects in MS VC++ 8.0 (I have not tested with other 
versions).

I improved the test_diamond example, after I understood what happened.

> class A {/**/}; // Polymorphic
> class B1 : virtual public A {/**/};
> class B2 : virtual public A {/**/};
> class C : public B1, public B2 {/**/};
> class D : public C {/**/};

With the names of my initial post, the problem arises only if class D 
has some data, and we serialize two classes, one C and one D. The fact 
is that in MSVC, the offset from the C part to the A part of an object 
is not the same if the object is a C or a D.

If we serialize a C first, a void_cast_detail::void_caster_derived is 
created with the offset between the C part and the A part in a C object. 
Then, when we serialize a D, the same offset is reused, even if the 
value should be different.

Tomorrow, I will try to remove the creation of the 
void_cast_detail::void_caster_derived and it's registration, since I 
believe this is just an optimisation, to check if that definitively 
solves the problem.

I have however no ideas about how to modify the library to have is both 
functionnal and fast.

Here is an updated test_diamond.cpp that calculates the offsets (check 
the "// Look this line" comment), and exposes the problem. I think that 
with just a little clean-up, it could be introduced in the test suite, 
but I'm not accustomed to Boost naming conventions.

/////////1/////////2/////////3/////////4/////////5/////////6/////////7/////////8
// test_diamond.cpp

// (C) Copyright 2002 Vladimir Prus.
// Use, modification and distribution is subject to the Boost Software
// License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at
// http://www.boost.org/LICENSE_1_0.txt)

// test of serialization library for diamond intheritence situations
#pragma warning (disable:4996)

#include <fstream>
#include <iostream>

#include <cstdio> // remove
#include <boost/config.hpp>
#if defined(BOOST_NO_STDC_NAMESPACE)
namespace std{
     using ::remove;
}
#endif

#define BOOST_ARCHIVE_TEST xml_archive.hpp
#include "test_tools.hpp"
#include <boost/preprocessor/stringize.hpp>
#include BOOST_PP_STRINGIZE(BOOST_ARCHIVE_TEST)

#include <boost/serialization/map.hpp>
#include <boost/serialization/utility.hpp>
#include <boost/serialization/split_member.hpp>
#include <boost/serialization/tracking.hpp>
#include <boost/serialization/base_object.hpp>
#include <boost/serialization/nvp.hpp>
#include <boost/serialization/export.hpp>

using namespace boost;

int save_count = 0; // used to detect when base class is saved multiple 
times
int load_count = 0; // used to detect when base class is loaded multiple 
times

class base {
public:
     base() : i(0) {}
     base(int i) : i(i)
     {
         m[i] = "text";
     }

     template<class Archive>
     void save(Archive &ar, const unsigned int /* file_version */) const
     {
         std::cout << "Saving base\n";
         ar << BOOST_SERIALIZATION_NVP(i) << BOOST_SERIALIZATION_NVP(m);
         ++save_count;
     }

     template<class Archive>
     void load(Archive & ar, const unsigned int /* file_version */)
     {
         std::cout << "Restoring base\n";
         ar >> BOOST_SERIALIZATION_NVP(i) >> BOOST_SERIALIZATION_NVP(m);
         ++load_count;
     }

     BOOST_SERIALIZATION_SPLIT_MEMBER()

     bool operator==(const base& another) const
     {
         return i == another.i && m == another.m;
     }
     // make virtual to evade gcc quirk
     virtual ~base() {};
private:
     int i;
     std::map<int, std::string> m;
};

// note: the default is for object tracking to be performed if and only
// if and object of the corresponding class is anywhere serialized
// through a pointer.  In this example, that doesn't occur so
// by default, the shared base object wouldn't normally be tracked.
// This would leave to multiple save/load operation of the data in
// this shared base class.  This wouldn't cause an error, but it would
// be a waste of time.  So set the tracking behavior trait of the base
// class to always track serialized objects of that class.  This permits
// the system to detect and elminate redundent save/load operations.
// (It is concievable that this might someday be detected automatically
// but for now, this is not done so we have to rely on the programmer
// to specify this trait)
BOOST_CLASS_TRACKING(base, track_always)

class derived1 : virtual public base {
public:
        int myData1;
        derived1(int data) : myData1(data) {}
        derived1() {}
     template<class Archive>
     void save(Archive &ar, const unsigned int /* file_version */) const
     {
         std::cout << "Saving derived1\n";
         ar << BOOST_SERIALIZATION_BASE_OBJECT_NVP(base);
         ar << BOOST_SERIALIZATION_NVP(myData1);
     }

     template<class Archive>
     void load(Archive & ar, const unsigned int /* file_version */)
     {
         std::cout << "Restoring derived1\n";
         ar >> BOOST_SERIALIZATION_BASE_OBJECT_NVP(base);
         ar >> BOOST_SERIALIZATION_NVP(myData1);
     }

     BOOST_SERIALIZATION_SPLIT_MEMBER()
};

class derived2 : virtual public base {
public:
        int myData2;
        derived2(int data) : myData2(data) {}
        derived2() {}
     template<class Archive>
     void save(Archive &ar, const unsigned int /* file_version */) const
     {
         std::cout << "Saving derived2\n";
         ar << BOOST_SERIALIZATION_BASE_OBJECT_NVP(base);
         ar << BOOST_SERIALIZATION_NVP(myData2);
     }

     template<class Archive>
     void load(Archive & ar, const unsigned int /* file_version */)
     {
         std::cout << "Restoring derived2\n";
         ar >> BOOST_SERIALIZATION_BASE_OBJECT_NVP(base);
         ar >> BOOST_SERIALIZATION_NVP(myData2);
    }

     BOOST_SERIALIZATION_SPLIT_MEMBER()
};

class final : public derived1, public derived2 {
public:
        int finalData;
     final() {}
     final(int i, int j, int k, int l) : base(i), derived1(j), 
derived2(k), finalData(l) {}

     template<class Archive>
     void save(Archive &ar, const unsigned int /* file_version */) const
     {
         std::cout << "Saving final\n";
         ar << BOOST_SERIALIZATION_BASE_OBJECT_NVP(derived1);
         ar << BOOST_SERIALIZATION_BASE_OBJECT_NVP(derived2);
                ar << BOOST_SERIALIZATION_NVP(finalData);
     }

     template<class Archive>
     void load(Archive & ar, const unsigned int /* file_version */)
     {
         std::cout << "Restoring final\n";
         ar >> BOOST_SERIALIZATION_BASE_OBJECT_NVP(derived1);
         ar >> BOOST_SERIALIZATION_BASE_OBJECT_NVP(derived2);
                ar >> BOOST_SERIALIZATION_NVP(finalData);
     }

     BOOST_SERIALIZATION_SPLIT_MEMBER()
};
BOOST_CLASS_EXPORT(final)

class derived_of_final : public final {
public:
        int newData;
     derived_of_final() {}
     derived_of_final(int i, int j, int k, int l, int m) : base(i), 
final(i, j, k, l), newData(m) {}

     template<class Archive>
     void save(Archive &ar, const unsigned int /* file_version */) const
     {
         std::cout << "Saving derived_of_final\n";
         ar << BOOST_SERIALIZATION_BASE_OBJECT_NVP(final);
                ar << BOOST_SERIALIZATION_NVP(newData);
     }

     template<class Archive>
     void load(Archive & ar, const unsigned int /* file_version */)
     {
         std::cout << "Restoring derived_of_final\n";
         ar >> BOOST_SERIALIZATION_BASE_OBJECT_NVP(final);
                ar >> BOOST_SERIALIZATION_NVP(newData);
     }

     BOOST_SERIALIZATION_SPLIT_MEMBER()
};

BOOST_CLASS_EXPORT(derived_of_final)

int
test_main( int /* argc */, char* /* argv */[] )
{
     const char * testfile = boost::archive::tmpnam(NULL);

     BOOST_REQUIRE(NULL != testfile);

     save_count = 0;
     load_count = 0;

     const base* bpinit = new final( 1, 2, 3, 4);
     const base* bp = new derived_of_final( 3, 42, 314, 1592, 653589 );

        // Code that shows the offset problem
        const final* final_bpinit = dynamic_cast<const final*>(bpinit);
        const final* final_bp = dynamic_cast<const final*>(bp);
        const char*v00 = reinterpret_cast<const char *>(bpinit);
        const char*v01 = reinterpret_cast<const char *>(final_bpinit);
        const char*v10 = reinterpret_cast<const char *>(bp);
        const char*v11 = reinterpret_cast<const char *>(final_bp);
        int offset1 = v01 - v00;
        int offset2 = v11 - v10;
        BOOST_CHECK_EQUAL(offset1, offset2); // Look this line

        // serialization code
     {
         test_ostream ofs(testfile);
         test_oarchive oa(ofs);
         oa << BOOST_SERIALIZATION_NVP(bpinit);
         oa << BOOST_SERIALIZATION_NVP(bp);
     }

     base* bpinit2;
     base* bp2;
     {
         test_istream ifs(testfile);
         test_iarchive ia(ifs);
         ia >> BOOST_SERIALIZATION_NVP(bpinit2);
         ia >> BOOST_SERIALIZATION_NVP(bp2);
     }

     BOOST_CHECK(1 == save_count);
     BOOST_CHECK(1 == load_count);
     BOOST_CHECK(*bp2 == *bp);
     std::remove(testfile);

     return EXIT_SUCCESS;
}

/**
  * End of file test_diamond.cpp
  */

Best regards,

-- 
Loïc Joly