Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

abort during getline in yyfill #223

Closed
kevin-- opened this issue Mar 15, 2016 · 9 comments
Closed

abort during getline in yyfill #223

kevin-- opened this issue Mar 15, 2016 · 9 comments

Comments

@kevin--
Copy link
Contributor

kevin-- commented Mar 15, 2016

I'm experiencing a runtime abort with MSVC 2015 14.0.24720.00 Update 1 while attempting to read a file using the ">>" operator. The file was written by this library as well, using the "<<" operator.
A dialog appears with "abort() has been called", hitting retry drops me into the debugger.

I don't think there's anything very novel about my code:
json document; std::ifstream in; in.exceptions(std::ifstream::failbit | std::ifstream::badbit); in.open(filepath); in >> document;

For now the work around is to read the file into a string, then call parse()

here's the data I'm trying to read
file_in_question.json.txt

Stack trace
> bumped.exe!__GSHandlerCheck_EH(_EXCEPTION_RECORD * ExceptionRecord, void * EstablisherFrame, _CONTEXT * ContextRecord, _DISPATCHER_CONTEXT * DispatcherContext) Line 101 C

...bunch of msvc runtime...

bumped.exe!std::getline<char,std::char_traits<char>,std::allocator<char> >(std::basic_istream<char,std::char_traits<char> > && _Istr, std::basic_string<char,std::char_traits<char>,std::allocator<char> > & _Str, const char _Delim) Line 120 C++

bumped.exe!std::getline<char,std::char_traits<char>,std::allocator<char> >(std::basic_istream<char,std::char_traits<char> > & _Istr, std::basic_string<char,std::char_traits<char>,std::allocator<char> > & _Str) Line 162 C++

bumped.exe!nlohmann::basic_json<std::map,std::vector,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,bool,__int64,double,std::allocator>::lexer::yyfill() Line 7636 C++

bumped.exe!nlohmann::basic_json<std::map,std::vector,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,bool,__int64,double,std::allocator>::lexer::scan() Line 6877 C++

bumped.exe!nlohmann::basic_json<std::map,std::vector,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,bool,__int64,double,std::allocator>::parser::get_token() Line 8118 C++

bumped.exe!nlohmann::basic_json<std::map,std::vector,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,bool,__int64,double,std::allocator>::parser::parse_internal(bool keep) Line 7961 C++

bumped.exe!nlohmann::basic_json<std::map,std::vector,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,bool,__int64,double,std::allocator>::parser::parse() Line 7889 C++

bumped.exe!nlohmann::operator>>(std::basic_istream<char,std::char_traits<char> > & i, nlohmann::basic_json<std::map,std::vector,std::basic_string<char,std::char_traits<char>,std::allocator<char> >,bool,__int64,double,std::allocator> & j) Line 5287 C++

@kevin--
Copy link
Contributor Author

kevin-- commented Mar 15, 2016

by the way, working from the v1.1.0 tag

@nlohmann
Copy link
Owner

Hi @kevin--, sorry about the bug. As I do not work with Windows/MSVC myself, it's difficult do dig deeper into the issue.

@nlohmann nlohmann added the state: help needed the issue needs help to proceed label Mar 20, 2016
@kevin--
Copy link
Contributor Author

kevin-- commented Mar 20, 2016

I'll take a crack at it this week when I get a chance

@duncanwerner
Copy link

According to http://en.cppreference.com/w/cpp/string/basic_string/getline

If no characters were extracted for whatever reason (not even the discarded delimiter), getline sets failbit and returns.

it seems like there's always a call to yyfill at the end of the stream, so if you are masking failbit, it will throw an exception. As to why you're getting an abort(), perhaps you're using the wrong exception model?

@kevin--
Copy link
Contributor Author

kevin-- commented Mar 21, 2016

@duncanwerner good call. If I do not set the exception mask, then I do not get the abort. Still want to look at this, because my work around of reading into the string then parsing still works. Maybe that method does not use getline.

@nlohmann
Copy link
Owner

@duncanwerner Thanks for this note. I see if I can "force" this behavior on a system where I can debug.

@kevin--
Copy link
Contributor Author

kevin-- commented Mar 30, 2016

There are two points of failure, both related to calling std::getline

  1. if the input file is empty (0 bytes), then the abort occurs on the first call to std::getline (2nd line of the explicit lexer(std::istream* s) noexcept constructor).
  2. Otherwise, it happens when std::getline gets no content at the end of the file in yyfill

As I have discovered, the reason for the abort is because of the use of the noexcept specifier on all of the parser and related methods.
Per the spec ( http://en.cppreference.com/w/cpp/language/noexcept_spec ) runtimes must invoke std::terminate if an exception is being thrown through such a method. So therefore MSVC is actually acting correctly (for once).

The take away here, that I didn't really think of, is that setting failbit is inappropriate in most cases. Having read the documentation more thoroughly, it seems this failbit is more likely to give false-positives.

I have opened a pull request which adds a note to the documentation to this effect.

@nlohmann
Copy link
Owner

I merged #227.

@kevin--, is the issue fixed for you?

@nlohmann nlohmann removed the state: help needed the issue needs help to proceed label Mar 30, 2016
@kevin--
Copy link
Contributor Author

kevin-- commented Mar 30, 2016

Yes. There was no real bug, just a clash between failbit, getline reading 0 chars, and noexcept. Though I expect you should be able to reproduce this in other compilers.

@nlohmann nlohmann removed the platform: visual studio related to MSVC label Mar 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants