Boost Spirit Qi crashes for memory violation

Question

But I cannot figure out why...?

http://coliru.stacked-crooked.com/a/2912593bb421a35e

#include <boost/fusion/adapted/struct.hpp>
#include <boost/spirit/include/qi.hpp>

namespace bsq = boost::spirit::qi;

int main()
{        
    std::uint16_t major, minor, build, revision;

    auto versionParser =
        bsq::uint_
        >> -('.' >> bsq::uint_)
        >> -('.' >> bsq::uint_)
        >> -('.' >> bsq::uint_);

    std::string version = "3.5.1";

    auto start = version.begin();
    if (!bsq::parse(start, version.end(), versionParser, major, minor, build, revision))
    {
        std::cout << "Error!\n";
    }

    std::cout << major << "-" << minor << "-" << build << "-" << revision << std::endl;

    return 0;
}

The call to parse() causes a memory access violation.

I swear I had this working at one time but... maybe I was daydreaming. I've tried on Windows with Visual Studio 2017 and also on Coliru with clang. I cannot see the error.

Thank you.

Answer 1

The problem is the use of auto expressions to capture rules, which deduces the types from the parser expressions. That type is a proto-expression tree, which captures any relations by reference, but that means many of _the intermediates are gone after the end of the enclosing full-expresion (see C++: Life span of temporary arguments?).

This is pretty well-known, as you can see here:

• Assigning parsers to auto variables
• boost spirit V2 qi bug associated with optimization level
• undefined behaviour somewhere in boost::spirit::qi::phrase_parse
• And some more

Here's the simplest fix:

auto versionParser = bsq::copy(
    bsq::uint_
    >> -('.' >> bsq::uint_)
    >> -('.' >> bsq::uint_)
    >> -('.' >> bsq::uint_));

If you also fix the missing intialization of the local variables it works correctly:

Live On Coliru

#include <boost/fusion/adapted/struct.hpp>
#include <boost/spirit/include/qi.hpp>

namespace bsq = boost::spirit::qi;

int main()
{    
    std::cout << "BOOST_VERSION: " << BOOST_VERSION << std::endl;

    std::uint16_t major = 0, minor = 0, build = 0, revision = 0;

    auto versionParser = bsq::copy(
        bsq::uint_
        >> -('.' >> bsq::uint_)
        >> -('.' >> bsq::uint_)
        >> -('.' >> bsq::uint_));

    std::string version = "3.5.1";

    auto start = version.begin();
    if (!bsq::parse(start, version.end(), versionParser, major, minor, build, revision))
    {
        std::cout << "Error!\n";
    }

    std::cout << major << "-" << minor << "-" << build << "-" << revision << std::endl;
}

Prints

BOOST_VERSION: 106600
3-5-1-0

Additional Notes

1. To avoid the whole "unitialized attribute" situation, let's make it so the parser assigns to all elements, even if unspecified in the input text:

       >> ('.' >> bsq::uint_ | bsq::attr(0))
       >> ('.' >> bsq::uint_ | bsq::attr(0))
       >> ('.' >> bsq::uint_ | bsq::attr(0))
   

2. To diagnose errors where there is trailing "garbage" (like with "3.4bogus"), you could add a check that the full input is parsed:

   auto versionParser = bsq::copy(
       bsq::uint_
       >> ('.' >> bsq::uint_ | bsq::attr(0))
       >> ('.' >> bsq::uint_ | bsq::attr(0))
       >> ('.' >> bsq::uint_ | bsq::attr(0))
       >> bsq::eoi);
   

3. Because a version is semantically a tuple, why not represent it as such?

   using Version = std::tuple<uint16_t, uint16_t, uint16_t, uint16_t>;
   Version parsed;
   
   if (!bsq::parse(version.begin(), version.end(), versionParser, parsed))
       std::cout << "Error!\n";
   

   That way you can even say:

   using boost::fusion::operator<<;
   
   auto obsolete = parsed < Version(3, 4, 0, 0);
   std::cout << "Version " << parsed << " " << (obsolete? "too old" : "supported") << "\n";
   

Combining those:

Live On Coliru

#include <boost/fusion/adapted/std_tuple.hpp>
#include <boost/spirit/include/qi.hpp>

namespace bsq = boost::spirit::qi;

int main() {    
    auto versionParser = bsq::copy(
        bsq::uint_
        >> ('.' >> bsq::uint_ | bsq::attr(0))
        >> ('.' >> bsq::uint_ | bsq::attr(0))
        >> ('.' >> bsq::uint_ | bsq::attr(0))
        >> bsq::eoi);

    std::string version = "3.5.1";

    using Version = std::tuple<uint16_t, uint16_t, uint16_t, uint16_t>;
    Version parsed;

    if (!bsq::parse(version.begin(), version.end(), versionParser, parsed))
        std::cout << "Error!\n";

    using boost::fusion::operator<<;

    auto obsolete = parsed < Version(3, 4, 0, 0);
    std::cout << "Version " << parsed << " " << (obsolete? "too old" : "supported") << "\n";
}

Prints

Version (3 5 1 0) supported

std::tuple sucks?

I agree. So, equivalently write your own struct:

Live On Coliru

struct Version {
    uint16_t major, minor, revision, build;

    auto key() const { return std::tie(major, minor, revision, build); }
    bool operator<(Version const& b) const { return key() < b.key(); }
};

BOOST_FUSION_ADAPT_STRUCT(Version, major, minor, revision, build)

Gettin' With The Times

Note that Spirit X3 (Getting into boost spirit; Qi or X3?) doesn't have the auto-issues that you ran into:

Live On Coliru

#include <boost/fusion/adapted/struct.hpp>
#include <boost/spirit/home/x3.hpp>

#include <boost/fusion/include/io.hpp>
#include <iostream>

namespace bsx = boost::spirit::x3;

struct Version {
    uint16_t major, minor, revision, build;

    auto key() const { return std::tie(major, minor, revision, build); }
    bool operator<(Version const& b) const { return key() < b.key(); }
};

BOOST_FUSION_ADAPT_STRUCT(Version, major, minor, revision, build)

int main() {    
    auto versionParser = bsx::uint_
        >> ('.' >> bsx::uint_ | bsx::attr(0))
        >> ('.' >> bsx::uint_ | bsx::attr(0))
        >> ('.' >> bsx::uint_ | bsx::attr(0))
        >> bsx::eoi;

    std::string version = "3.5.1";

    Version parsed;

    if (!parse(version.begin(), version.end(), versionParser, parsed))
        std::cout << "Error!\n";

    using boost::fusion::operator<<;

    auto obsolete = parsed < Version{3, 4, 0, 0};
    std::cout << "Version " << parsed << " " << (obsolete? "too old" : "supported") << "\n";
}

Also printing the same.