From 1100bb8a5beee54746e4ceefa830ceb07070bb35 Mon Sep 17 00:00:00 2001 From: Jesse Haber-Kucharsky Date: Fri, 30 Jun 2017 11:32:56 -0400 Subject: [PATCH] cql: Eagerly throw lexing and parsing exceptions Previously, lexing and parsing errors were aggregated while CQL queries were evaluated. Afterwards, the first collected error (if present) would be thrown as an exception. The problem was that when parsing and lexing errors were aggregated this way, the parser would continue even in spite of errors like "no viable alternative". Semantic actions attached to grammar rules would still execute, though with variables that had not yet been initialized. This would crash Scylla. This change modifies the error-handling strategy of CQL parsing. Rather than aggregate errors, we throw an exception on the first error we encounter. This ensures that grammar actions never execute unless there is a precise match. One possible issue with this approach is that the generated C++ code from the ANTLR grammar may not be exception-safe. I compiled Scylla in debug-mode with ASan support and executed several erroneous CQL queries with `cqlsh`. No memory leaks were reported. Fixes #2466. Signed-off-by: Jesse Haber-Kucharsky Message-Id: --- cql3/error_collector.hh | 24 ++++++++---------------- cql3/util.hh | 2 -- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/cql3/error_collector.hh b/cql3/error_collector.hh index c7ce4fc834..9a49714c78 100644 --- a/cql3/error_collector.hh +++ b/cql3/error_collector.hh @@ -67,10 +67,6 @@ class error_collector : public error_listener */ const sstring_view _query; - /** - * The error messages. - */ - std::vector _error_msgs; public: /** @@ -81,7 +77,10 @@ public: */ error_collector(const sstring_view& query) : _query(query) {} - virtual void syntax_error(RecognizerType& recognizer, ANTLR_UINT8** token_names, ExceptionBaseType* ex) override { + /** + * Format and throw a new \c exceptions::syntax_exception. + */ + [[noreturn]] virtual void syntax_error(RecognizerType& recognizer, ANTLR_UINT8** token_names, ExceptionBaseType* ex) override { auto hdr = get_error_header(ex); auto msg = get_error_message(recognizer, ex, token_names); std::stringstream result; @@ -90,22 +89,15 @@ public: if (recognizer instanceof Parser) appendQuerySnippet((Parser) recognizer, builder); #endif - _error_msgs.emplace_back(result.str()); - } - virtual void syntax_error(RecognizerType& recognizer, const sstring& msg) override { - _error_msgs.emplace_back(msg); + throw exceptions::syntax_exception(result.str()); } /** - * Throws the first syntax error found by the lexer or the parser if it exists. - * - * @throws SyntaxException the syntax error. + * Throw a new \c exceptions::syntax_exception. */ - void throw_first_syntax_error() { - if (!_error_msgs.empty()) { - throw exceptions::syntax_exception(_error_msgs[0]); - } + [[noreturn]] virtual void syntax_error(RecognizerType&, const sstring& msg) override { + throw exceptions::syntax_exception(msg); } private: diff --git a/cql3/util.hh b/cql3/util.hh index a8bbacb7fe..77ec553015 100644 --- a/cql3/util.hh +++ b/cql3/util.hh @@ -51,8 +51,6 @@ Result do_with_parser(const sstring_view& cql, Func&& f) { cql3_parser::CqlParser parser{&tstream}; parser.set_error_listener(parser_error_collector); auto result = f(parser); - lexer_error_collector.throw_first_syntax_error(); - parser_error_collector.throw_first_syntax_error(); return result; }