2

I have a large code base that can use boost::any or boost::spirit::hold_any (depending on a macro definition).

hold_any seems to be compatible with boost::any (e.g. How to print boost::any to a stream? or Type erasure - Part IV) and faster (Why you shouldn’t use boost::any) but I'm experiencing several segmentation fault errors using hold_any (Boost v1.55 / 1.54 / 1.53).

This is a minimal working example that exhibits the same problem as the original code:

#include <iostream>
#include <string>
#include <vector>

#include <boost/spirit/home/support/detail/hold_any.hpp>

typedef boost::spirit::hold_any any;
typedef std::vector<any> vany;

int main()
{
  vany data0, data1;

  for (unsigned i(0); i < 1000; ++i)
  {
    std::string s("test_test_test");
    data0.push_back(any(s));
  }

  const unsigned n(data0.size());
  vany::iterator iter(data0.begin());

  for (unsigned i(0); i < n; ++i)
  {
    std::cout << "Moving " << i << std::endl;

    data1.push_back(*iter);
    iter = data0.erase(iter);
  }

  return 0;
}

The program appears to work correctly:

  • changing from boost::spirit::hold_any to boost::any;
  • changing the content of the hold_any to a data type small enough to perform small buffer optimization (e.g. from std::string to int).

It seems strange that there could be some major bug in a widely used library such as Boost Spirit, but

  • I'm having a hard time finding a bug in the example;
  • I've tried g++ / clang++ without success.

What's wrong with the example?

Community
  • 1
  • 1
manlio
  • 18,345
  • 14
  • 76
  • 126

1 Answers1

2

You should not be using hold_any as it is in detail/hold_any.hpp for a reason.

That said, hold_any's copy-assignment appears to be broken. I've created a pull request on github with a proposed fix.

Without the fix, the following program demonstrates UB (because the compiler generates a shallow assignment operator which is preferred):

#include <iostream>
#include <string>

#include <boost/spirit/home/support/detail/hold_any.hpp>

typedef boost::spirit::hold_any any;

int main()
{
    any b;
    {
        any a;
        a = std::string("test_test_test");
        b = a;
    }

    std::cout << "b: " << b << '\n';
}

When run under valgrind:

==11827== Invalid read of size 8
==11827==    at 0x5E9D793: std::basic_ostream<char, std::char_traits<char> >& std::operator<< <char, std::char_traits<char>, std::allocator<char> >(std::basic_ostream<char, std::char_traits<char> >&, std::basic_string<char, std
==11827==    by 0x4012FC: boost::spirit::detail::fxns<mpl_::bool_<true> >::type<std::string, char>::stream_out(std::ostream&, void* const*) (hold_any.hpp:113)
==11827==    by 0x4010F5: std::basic_ostream<char, std::char_traits<char> >& boost::spirit::operator<< <char>(std::basic_ostream<char, std::char_traits<char> >&, boost::spirit::basic_hold_any<char> const&) (hold_any.hpp:368)
==11827==    by 0x400FC9: main (test.cpp:17)
==11827==  Address 0x8ac1650 is 0 bytes inside a block of size 39 free'd
==11827==    at 0x4C2BADC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11827==    by 0x5EC405E: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string() (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.18)
==11827==    by 0x401204: boost::spirit::detail::fxns<mpl_::bool_<true> >::type<std::string, char>::static_delete(void**) (hold_any.hpp:89)
==11827==    by 0x401328: boost::spirit::basic_hold_any<char>::~basic_hold_any() (hold_any.hpp:246)
==11827==    by 0x4010B4: boost::spirit::basic_hold_any<char>::~basic_hold_any() (hold_any.hpp:245)
==11827==    by 0x400FA0: main (test.cpp:15)
sehe
  • 374,641
  • 47
  • 450
  • 633
  • Thank you, I didn't know. It's a pity `hold_any` isn't a ready-to-use `boost:any` substitute: SBO is a big gain in my program. Do you know other differences? If this is the "only" one then, perhaps, it could be worked around. – manlio Jun 05 '14 at 20:47
  • 1
    Update **[pull request](https://github.com/boostorg/spirit/pull/36)** for proper c++11 support – sehe Jun 06 '14 at 00:18
  • Meanwhile I've found this old ticket: [basic_hold_any missing assignment operator](https://svn.boost.org/trac/boost/ticket/8268). It seems to me that also the non-template assignment constructor should be added to your patch. – manlio Jun 06 '14 at 09:12
  • @manlio it's actually ok, as long as we always provide a the 'best' overload for the copy assignment (see e.g. http://ericniebler.com/2013/08/07/universal-references-and-the-copy-constructo/) – sehe Jun 06 '14 at 09:17