[serialization] Problem with diamond hierarchy

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[serialization] Problem with diamond hierarchy

Loïc Joly-3
Hello,

I have a class hierarchy that looks like this :

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

In C, I define the serialize function like this :

template <class Archive>
void C::serialize(Archive &ar, const unsigned int version)
{
   BOOST_SERIALIZATION_BASE_OBJECT_NVP(B1);
   BOOST_SERIALIZATION_BASE_OBJECT_NVP(B2);
   // serialize fields specific to C
}

When I try to serialize an object of type C, it works just fine.
However, when I try to serialize an object of class D, it fails. I have
not yet tried to reproduce this problem in fully a toy example, but I'm
going to do so ASAP.

I'm using VC++8.0.

When I try to debug the system, here is the information I get :

At one time, I'm in
save_pointer_type<xml_oarchive, A*> polymorphic::save
in line
vp = serialization::void_downcast(*true_type, *this_type, &t);

At that place, when I inspect t, all is fine (especially the vfprt).
Then I go in this void_downcast, where I can't inspect data, since the
pointer has been cast to a void type. Next time I gets back some type
info, i when calling :

void_downcast(
     const extended_type_info & derived_type, // D
     const extended_type_info & base_type,    // A
     const void * const t,
     bool top)

In this function, I get an iterator at some point
void_cast_detail::void_caster_registry::const_iterator it;

That has the following content: Derived == D, Base == C.

Then, it calls void_caster_primitive::downcast with those informations,
where it calls
boost::smart_cast<const Derived *, const Base *>(
             static_cast<const Base *>(t)
// Derived == D, Base == C


However, in this function, declared that way:
template<class T, class U>
T smart_cast(U u);

If I try to look at the content of u, it seems totally broken. For
instance, the vfptr contains values such as
0xcdcdcdcd
0x00000000
...



 From my (limited) understanding of C++, casting from A* to void* and
back to C* is not supposed to work, but I'd like some confirmation.

Does someone have any idea about this problem, and if there is a
workaround ?


Best regards,

--
Loïc




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [serialization] Problem with diamond hierarchy

Robert Ramey
Check out test_diamond in the test suite.  It has been designed to
test and demonstrate exactly this functionality and the test passes
with VC 8.0 with all archives.

So, compare this with your own example to determine what has
to be fixed.  This should result in one of the following:

a) a usage error in the library.  If this turns out to be the case
I would be happy to recieve a suggestion for ehancing
the documenation or code (BOOST_STATIC_ASSERT
or something like that - or both.

b) an error in the serialiation library - in shis case I would
be happy to recieve a suggestion on how to enhance "test_diamond"
to detect and illustrate this error.

Robert Ramey

Loïc Joly wrote:

> Hello,
>
> I have a class hierarchy that looks like this :
>
> class A {/**/}; // Polymorphic
> class B1 : virtual public A {/**/};
> class B2 : virtual public A {/**/};
> class C : public B1, public B2 {/**/};
> class D : public C {/**/};
>
> In C, I define the serialize function like this :
>
> template <class Archive>
> void C::serialize(Archive &ar, const unsigned int version)
> {
>    BOOST_SERIALIZATION_BASE_OBJECT_NVP(B1);
>    BOOST_SERIALIZATION_BASE_OBJECT_NVP(B2);
>    // serialize fields specific to C
> }
>
> When I try to serialize an object of type C, it works just fine.
> However, when I try to serialize an object of class D, it fails.
I would be interesting to know how it fails.

Robert Ramey




_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [serialization] Problem with diamond hierarchy

Loïc Joly-3
In reply to this post by Loïc Joly-3
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

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [serialization] Problem with diamond hierarchy

Loïc Joly-3
Loïc Joly a écrit :
>
> 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 did this modification, and it works fine in my test case. Here is my
(ugly) patch file, I just commented out the offending lines:

252c252
<                     void_cast_detail::void_caster * vcp =
---
 >                     /*void_cast_detail::void_caster * vcp =
260c260
<                     );
---
 >                     );*/
310c310
<                 if(top){
---
 >                 if(top){/*
321c321
<                     );
---
 >                     );*/


Best regards

--
Loïc


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost