boostorg / spirit

Boost.org spirit module
http://boost.org/libs/spirit
392 stars 161 forks source link

parse_nan() dereferences out of range iterator #350

Closed oracleoftroy closed 6 years ago

oracleoftroy commented 6 years ago

In parse_nan(), after detecting the string "nan", while looking for parens, there is no check to make sure we didn't already reach the end of the range before dereferencing the iterator.

https://github.com/boostorg/spirit/blob/54d5ea1ea84013c66a138db57e6275a5aaed5035/include/boost/spirit/home/x3/numeric/real_policies.hpp#L112-L116

I believe the second if (line 115) should look like:

if (first != last && *first == '(')

This causes asserts to trigger with debug iterators in MSVC when using std::string or std::string_view. Here's an example with a custom iterator that throws when dereferencing the end.

http://coliru.stacked-crooked.com/a/4301f38c79b25ed6

djowel commented 6 years ago

Good catch! Will you issue a PR for this?

Kojoley commented 6 years ago

A smiley thing that Qi has the check https://github.com/boostorg/spirit/blob/54d5ea1ea84013c66a138db57e6275a5aaed5035/include/boost/spirit/home/qi/numeric/real_policies.hpp#L128-L132

djowel commented 6 years ago

@Kojoley, we should probably start thinking about unifying the common X3 and Qi facilities.

Kojoley commented 6 years ago

Actually I would prefer that string to number parsing be a separate library. Current spirit parser does not use any speed up tricks except loop unrolling. Even nan parsing could be done in one full sized register compare while currently it loops over chars like an LL(1) parser.

djowel commented 6 years ago

Actually I would prefer that string to number parsing be a separate library. Current spirit parser does not use any speed up tricks except loop unrolling. Even nan parsing could be done in one full sized register compare while currently it loops over chars like an LL(1) parser.

Interesting ideas!

octopus-prime commented 6 years ago

we should probably start thinking about unifying the common X3 and Qi facilities

Does this mean that Qi and X3 should share code? The shared code has to be in C++03?! So we give up the advantage of C++14 for a separated X3 impl...

djowel commented 6 years ago

Does this mean that Qi and X3 should share code? The shared code has to be in C++03?! So we give up the advantage of C++14 for a separated X3 impl...

Very good point actually. No, X3 should be free from C++03. But, it is very possible to separate 03 code from 14, in separate files even, just like what I did with Fusion, for example.