search

mayah / tinytoml

A header only C++11 library for parsing TOML
BSD 2-Clause "Simplified" License
167 stars 31 forks source link

Signed integer overflow on libstdc++ #30

Open grievejia opened 7 years ago

grievejia commented 7 years ago

Here's the input that will trigger the issue: int_overflow.txt Feed it into master(c2444ed) parse_stdin built with UndefinedBehaviorSanitizer and linked with libstdc++(shipped with gcc 7.1.1) will crash the parser:

> cat int_overflow.txt | ./parse_stdin
/usr/lib64/gcc/x86_64-pc-linux-gnu/7.1.1/../../../../include/c++/7.1.1/chrono:176:38: runtime error: signed integer overflow: -61948886400 * 1000000000 cannot be represented in type 'long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/lib64/gcc/x86_64-pc-linux-gnu/7.1.1/../../../../include/c++/7.1.1/chrono:176:38 in

Interestingly, the issue would disappear if I choose to use libc++ instead.

mayah commented 7 years ago

Thanks for reporting.

d = 7-0-2

It's parsed as Time type. I think this is not a valid Time value, so this should be rejected in the parser, maybe :cry:

mayah commented 7 years ago

I've added a workaround, but this is not a complete fix, I believe.

grievejia commented 7 years ago

Unfortunately, it is not :( It neither rejects nonsensical date like "2000-2-30", nor accepts valid date like "1800-1-1". I admit that parsing datetime format in a RFC3339-conforming way is quite hard, though...

mayah commented 7 years ago

I know that does not reject nonsense date. Also, since we're using std::chrono::time_point to represent a time, date before unix epoch is not considered well. Supporting older date will cause a conversion problem between Gregorian calendar and Julian calendar, which I really don't want to touch.