Skip to content
Permalink
Browse files

Make QgsExpression parser reentrant. Fixes crashes when expressions a…

…re parsed in multiple threads.

Sponsored by a big dose of caffeine.
  • Loading branch information
wonder-sk committed Sep 25, 2014
1 parent 721cab1 commit 2427546d8850c7f0b2ca191b238a77c49f648510
Showing with 76 additions and 31 deletions.
  1. +13 −14 src/core/qgsexpressionlexer.ll
  2. +42 −17 src/core/qgsexpressionparser.yy
  3. +21 −0 tests/src/core/testqgsexpression.cpp
@@ -18,6 +18,10 @@
%option never-interactive
%option nounput
%option prefix="exp_"
// this makes flex generate lexer with context + init/destroy functions
%option reentrant
// this makes Bison send yylex another argument to use instead of using the global variable yylval
%option bison-bridge

// ensure that lexer will be 8-bit (and not just 7-bit)
%option 8bit
@@ -27,6 +31,7 @@
#include <stdlib.h> // atof()

#include "qgsexpression.h"
struct expression_parser_context;
#include "qgsexpressionparser.hpp"
#include <QRegExp>
#include <QLocale>
@@ -43,10 +48,10 @@
#define YY_NO_UNISTD_H
#endif

#define B_OP(x) exp_lval.b_op = QgsExpression::x
#define U_OP(x) exp_lval.u_op = QgsExpression::x
#define TEXT exp_lval.text = new QString(); *exp_lval.text = QString::fromUtf8(yytext);
#define TEXT_FILTER(filter_fn) exp_lval.text = new QString(); *exp_lval.text = filter_fn( QString::fromUtf8(yytext) );
#define B_OP(x) yylval->b_op = QgsExpression::x
#define U_OP(x) yylval->u_op = QgsExpression::x
#define TEXT yylval->text = new QString(); *yylval->text = QString::fromUtf8(yytext);
#define TEXT_FILTER(filter_fn) yylval->text = new QString(); *yylval->text = filter_fn( QString::fromUtf8(yytext) );

static QString stripText(QString text)
{
@@ -154,14 +159,14 @@ string "'"{str_char}*"'"
"," { return COMMA; }
{num_float} { exp_lval.numberFloat = cLocale.toDouble( QString::fromAscii(yytext) ); return NUMBER_FLOAT; }
{num_float} { yylval->numberFloat = cLocale.toDouble( QString::fromAscii(yytext) ); return NUMBER_FLOAT; }
{num_int} {
bool ok;
exp_lval.numberInt = cLocale.toInt( QString::fromAscii(yytext), &ok, 10 );
yylval->numberInt = cLocale.toInt( QString::fromAscii(yytext), &ok, 10 );
if( ok )
return NUMBER_INT;
exp_lval.numberFloat = cLocale.toDouble( QString::fromAscii(yytext), &ok );
yylval->numberFloat = cLocale.toDouble( QString::fromAscii(yytext), &ok );
if( ok )
return NUMBER_FLOAT;
@@ -172,7 +177,7 @@ string "'"{str_char}*"'"
{special_col} { TEXT; return SPECIAL_COL; }
{column_ref} { TEXT; return QgsExpression::isFunctionName(*exp_lval.text) ? FUNCTION : COLUMN_REF; }
{column_ref} { TEXT; return QgsExpression::isFunctionName(*yylval->text) ? FUNCTION : COLUMN_REF; }
{column_ref_quoted} { TEXT_FILTER(stripColumnRef); return COLUMN_REF; }
@@ -181,9 +186,3 @@ string "'"{str_char}*"'"
. { return Unknown_CHARACTER; }
%%
void exp_set_input_buffer(const char* buffer)
{
exp__scan_string(buffer);
}
@@ -27,22 +27,37 @@
// don't redeclare malloc/free
#define YYINCLUDED_STDLIB_H 1

struct expression_parser_context;
#include "qgsexpressionparser.hpp"

//! from lexer
extern int exp_lex();
extern void exp_set_input_buffer(const char* buffer);
typedef void* yyscan_t;
typedef struct yy_buffer_state* YY_BUFFER_STATE;
extern int exp_lex_init(yyscan_t* scanner);
extern int exp_lex_destroy(yyscan_t scanner);
extern int exp_lex(YYSTYPE* yylval_param, yyscan_t yyscanner);
extern YY_BUFFER_STATE exp__scan_string(const char* buffer, yyscan_t scanner);

/** returns parsed tree, otherwise returns NULL and sets parserErrorMsg
(interface function to be called from QgsExpression)
*/
QgsExpression::Node* parseExpression(const QString& str, QString& parserErrorMsg);

/** error handler for bison */
void exp_error(const char* msg);
void exp_error(expression_parser_context* parser_ctx, const char* msg);

struct expression_parser_context
{
// lexer context
yyscan_t flex_scanner;

//! varible where the parser error will be stored
QString gExpParserErrorMsg;
QgsExpression::Node* gExpParserRootNode;
// varible where the parser error will be stored
QString errorMsg;
// root node of the expression
QgsExpression::Node* rootNode;
};

#define scanner parser_ctx->flex_scanner

// we want verbose error messages
#define YYERROR_VERBOSE 1
@@ -51,6 +66,11 @@ QgsExpression::Node* gExpParserRootNode;

%}

// make the parser reentrant
%define api.pure
%lex-param {void * scanner}
%parse-param {expression_parser_context* parser_ctx}

%name-prefix "exp_"

%union
@@ -130,7 +150,7 @@ QgsExpression::Node* gExpParserRootNode;

%%

root: expression { gExpParserRootNode = $1; }
root: expression { parser_ctx->rootNode = $1; }
;

expression:
@@ -162,13 +182,13 @@ expression:
{
// this should not actually happen because already in lexer we check whether an identifier is a known function
// (if the name is not known the token is parsed as a column)
exp_error("Function is not known");
exp_error(parser_ctx, "Function is not known");
YYERROR;
}
if ( QgsExpression::Functions()[fnIndex]->params() != -1
&& QgsExpression::Functions()[fnIndex]->params() != $3->count() )
{
exp_error("Function is called with wrong number of arguments");
exp_error(parser_ctx, "Function is called with wrong number of arguments");
YYERROR;
}
$$ = new QgsExpression::NodeFunction(fnIndex, $3);
@@ -195,7 +215,7 @@ expression:
{
if ( !QgsExpression::hasSpecialColumn( *$1 ) )
{
exp_error("Special column is not known");
exp_error(parser_ctx, "Special column is not known");
YYERROR;
}
// $var is equivalent to _specialcol_( "$var" )
@@ -234,28 +254,33 @@ when_then_clause:

%%


// returns parsed tree, otherwise returns NULL and sets parserErrorMsg
QgsExpression::Node* parseExpression(const QString& str, QString& parserErrorMsg)
{
gExpParserRootNode = NULL;
exp_set_input_buffer(str.toUtf8().constData());
int res = exp_parse();
expression_parser_context ctx;
ctx.rootNode = 0;

exp_lex_init(&ctx.flex_scanner);
exp__scan_string(str.toUtf8().constData(), ctx.flex_scanner);
int res = exp_parse(&ctx);
exp_lex_destroy(ctx.flex_scanner);

// list should be empty when parsing was OK
if (res == 0) // success?
{
return gExpParserRootNode;
return ctx.rootNode;
}
else // error?
{
parserErrorMsg = gExpParserErrorMsg;
parserErrorMsg = ctx.errorMsg;
return NULL;
}
}


void exp_error(const char* msg)
void exp_error(expression_parser_context* parser_ctx, const char* msg)
{
gExpParserErrorMsg = msg;
parser_ctx->errorMsg = msg;
}

@@ -16,6 +16,8 @@
#include <QObject>
#include <QString>
#include <QObject>
#include <QtConcurrentMap>

#include <qgsapplication.h>
//header for class being tested
#include <qgsexpression.h>
@@ -29,6 +31,16 @@
Q_DECLARE_METATYPE( QVariant )
#endif

static void _parseAndEvalExpr( int arg )
{
Q_UNUSED( arg );
for ( int i = 0; i < 100; ++i )
{
QgsExpression exp( "1 + 2 * 2" );
exp.evaluate();
}
}

class TestQgsExpression: public QObject
{
Q_OBJECT;
@@ -946,6 +958,15 @@ class TestQgsExpression: public QObject
QCOMPARE( QgsExpression::quotedString( "hello\\world" ), QString( "'hello\\\\world'" ) );
}

void reentrant()
{
// this simply should not crash

QList<int> lst;
for ( int i = 0; i < 10; ++i )
lst << i;
QtConcurrent::blockingMap( lst, _parseAndEvalExpr );
}
};

QTEST_MAIN( TestQgsExpression )

0 comments on commit 2427546

Please sign in to comment.
You can’t perform that action at this time.