set member_hook access violation

[toby_toby_toby@…]

Visual Studio 2012 express, all patches. 32 bits.

Iterator dereference returns wrong pointer (offset by 4 bytes from normal).

I think it's because of virtual inheritance, mistake in offset_from_pointer_to_member (intrusive/detail/parent_from_member.hpp)

Output of program below:

offsets: 4 and 8 pointers: 00BEF94C and 00BEF948 <<<

#include <iostream> #include <string> #include <boost/noncopyable.hpp> #include <boost/intrusive/set.hpp>

namespace intrusive = boost::intrusive;

class object : public boost::noncopyable { public:

object()

{ }

virtual ~object()

{ }

};

class signal : virtual public object { public:

signal() { } virtual ~signal() { }

};

class record : public signal { public:

record()

{ }

virtual ~record()

{ }

public:

virtual const std::string& get_buffer() const {

return m_buffer;

}

typedef intrusive::set_member_hook<

intrusive::link_mode<intrusive::auto_unlink>

hook;

hook m_hook;

std::string m_buffer;

};

template <class T, class M, M (T::*V)> struct member_comparator {

bool operator()(const T& t1, const T& t2) const {

return (t1.*V) < (t2.*V);

} bool operator()(const M& m, const T& t) const {

return m < (t.*V);

} bool operator()(const T& t, const M& m) const {

return (t.*V) < m;

}

};

typedef member_comparator<

record, std::string, &record::m_buffer

record_comparator;

typedef intrusive::set<

record, intrusive::compare<record_comparator>, intrusive::member_hook<

record, record::hook, &record::m_hook

,

intrusive::constant_time_size<false>

records

;

int main(int argc, char argv) {

union {

int32_t as_int; const record::hook record::* ptr_to_member;

} sss; sss.ptr_to_member = &record::m_hook;

std::cout << "offsets: " << sss.as_int << " and " << offsetof(record,m_hook) << std::endl;

records rr;

std::string key = "123"; records::insert_commit_data icd; std::pair<records::iterator,bool> ir = rr.insert_check(

key, record_comparator(), icd );

if ( !ir.second ) {

throw std::exception();

}

record rec; rec.m_buffer = key; records::iterator i = rr.insert_commit( rec, icd );

record* rrr = &(*i);

std::cout << "pointers: " << ((void*)rrr) << " and " << ((void*)&rec) << std::endl;

return 0;

}

[toby_toby_toby@…]

sorry for unformatted source - readded as separate file

[toby_toby_toby@…]

also must add that standard offsetof macro works well (it's based on getting address of member variable of NULL-object)

[Ion Gaztañaga]

Thanks for the report. MSVC pointer to member handling it's quite chaotic. Can you try the following patch?

diff U3 C:/Data/Libs/LocalSVN/boost-trunk/boost/intrusive/detail/parent_from_member.hpp C:/Data/Libs/boost/boost/intrusive/detail/parent_from_member.hpp
--- C:/Data/Libs/LocalSVN/boost-trunk/boost/intrusive/detail/parent_from_member.hpp     Sat Nov 24 09:48:40 2012
+++ C:/Data/Libs/boost/boost/intrusive/detail/parent_from_member.hpp    Sun Apr 28 23:53:19 2013
@@ -37,7 +37,15 @@
       boost::int32_t offset;
    } caster;
    caster.ptr_to_member = ptr_to_member;
-   return std::ptrdiff_t(caster.offset);
+   //MSVC ABI can use up to 3 int32 to represent pointer to member data
+   //with virtual base classes, in that case we must
+   //add the size of the extra __vfptr vtable.
+   if(sizeof(caster) > sizeof(boost::int32_t)){
+      return std::ptrdiff_t(caster.offset) + sizeof(void*);
+   }
+   else{
+      return std::ptrdiff_t(caster.offset);
+   }
    //This works with gcc, msvc, ac++, ibmcpp
    #elif defined(__GNUC__)   || defined(__HP_aCC) || defined(BOOST_INTEL) || \
          defined(__IBMCPP__) || defined(__DECCXX)

[toby_toby_toby@…]

Thanks, it helped.

PS: But I've got a feeling that previously I was able to make 8-byte difference (by playing with inheritance/member vars/additional virtual functions). I didn't payed attention because I thought that any difference is the same bad thing, that fix will not be for 4-byte only. Now I'm trying to simulate this 8-byte difference again. I will confirm any result, even if I was mistaken and there were only 4-byte errors.

[anonymous]

Well, now it's broken in different place (intrusive::list_member_hook).

We hit patched condition "if(sizeof(caster) > sizeof(boost::int32_t))" and add sizeof(void*), 8+4=12, but actual difference is 8.

Now I'm gonna update source to include check for this also.

PS: Sorry if this question is dumb, but why don't we use VC/stddef.h offsetof or it's approach with getting address of member of null-object?

[toby_toby_toby@…]

after patch set is working, but list is not

[toby_toby_toby@…]

Switched to base hooks with tags, because fixing of member hook failed:

1) offsetof's approach not working - access violation, null can't be dereferenced

2) fake object "char temp[sizeof(Parent)]" also not working - it actually accesses vptr

3) inspection of additional integers in caster gave nothing - they are the same for both cases, no way to deduce

[Ion Gaztañaga]

I don't know if virtual or multiple inheritance is involved in these issues, and it seems that MSVC's pointer to member data can have the size of 1, 2 or 3 ints.

Can you please provide a test case with a 8 byte difference? I guess it could help me to deduce MSVC's ABI.

[toby_toby_toby@…]

I was unable to simulate 8 byte difference, it may never exist in fact. I just had feeling that it was, but it seems that I was wrong.

But set_and_list.cpp attachment gives you an example when your current patch is not working, so you can try to deduce something on this.. I was unable to. set_item is working fine with +sizeof(void*), but list_item gives wrong pointer with this void* addon.

[Ion Gaztañaga]

It seems that even there is no virtual inheritance boundary between the member and the parent, MSVC ABI requires pointer adjustment operations with information from the virtual base table. asm code for pointer to member dereferencing shows vbtable is read to correct parent pointer before adding the offset stored in the pointer to member data.

So it seems very difficult or maybe impossible, to obtain the pointer adjustment value from the vtable to go back from the member to the parent (we need a way to obtain access to the vbtable without using a valid parent pointer).

I think the library should give a compile time error (if pointer to member is not a simple offset) and document that for Windows compilers member hooks have limitations and don't work if types have direct or indirect virtual bases.

[toby_toby_toby@…]

Static assertion is a good idea, even better with unit tests.

[Ion Gaztañaga]

After some research even reverse engineering MSVC RTTI we might not solve this issue. Boost 1.54 will include the static assertion for member hooks that need complex pointers to member data structures in MSVC ABI. Thanks for the report.

[toby_toby_toby@…]

Just an idea, add new option to hook:

typedef boost::intrusive::list_member_hook<
  boost::intrusive::store_pointer_to_parent_inside<true>
  >

So we will have

parent* _parent;

inside hook (null in constructor) for platforms that require it, if GCC does not need - then just do nothing with this option.

Then, when we call container.insert( item ), we actually have reference to item so we can assign it:

(item.*hook_member_ptr)._parent = &item;

And clear on erase. Now we don't need offsetoff or etc.

Static assertion will only be rised when compiler requires this (for people like me who actually expirienced problems). For other users it will be backward compatible: no need to specify option, sizeof same. Option only required for msvc+virtual inheritance.