Skip to content

Commit

Permalink
Fix arithmetic overflow and set int boundary value (vesoft-inc#1181)
Browse files Browse the repository at this point in the history 
* Fix arithmetic overflow

* fix vesoft-inc#826 Int value range is incorrect

* rebase upstream

* rebase upstream

Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
  • Loading branch information
@laura-ding @dutor
laura-ding and dutor committed Jan 7, 2020
1 parent afe638a commit fa6791c
Show file tree
Hide file tree
Showing 6 changed files with 224 additions and 17 deletions.
77 changes: 74 additions & 3 deletions src/common/filter/Expressions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,13 +687,56 @@ OptVariantType ArithmeticExpression::eval(Getters &getters) const {
auto l = left.value();
auto r = right.value();

static constexpr int64_t maxInt = std::numeric_limits<int64_t>::max();
static constexpr int64_t minInt = std::numeric_limits<int64_t>::min();

auto isAddOverflow = [] (int64_t lv, int64_t rv) -> bool {
if (lv >= 0 && rv >= 0) {
return maxInt - lv < rv;
} else if (lv < 0 && rv < 0) {
return minInt - lv > rv;
} else {
return false;
}
};

auto isSubOverflow = [] (int64_t lv, int64_t rv) -> bool {
if (lv > 0 && rv < 0) {
return maxInt - lv < -rv;
} else if (lv < 0 && rv > 0) {
return minInt - lv > -rv;
} else {
return false;
}
};

auto isMulOverflow = [] (int64_t lv, int64_t rv) -> bool {
if (lv > 0 && rv > 0) {
return maxInt / lv < rv;
} else if (lv < 0 && rv < 0) {
return maxInt / lv > rv;
} else if (lv > 0 && rv < 0) {
return maxInt / lv < -rv;
} else if (lv < 0 && rv > 0) {
return maxInt / lv > -rv;
} else {
return false;
}
};

switch (op_) {
case ADD:
if (isArithmetic(l) && isArithmetic(r)) {
if (isDouble(l) || isDouble(r)) {
return OptVariantType(asDouble(l) + asDouble(r));
}
return OptVariantType(asInt(l) + asInt(r));
int64_t lValue = asInt(l);
int64_t rValue = asInt(r);
if (isAddOverflow(lValue, rValue)) {
return Status::Error(folly::stringPrintf("Out of range %ld + %ld",
lValue, rValue));
}
return OptVariantType(lValue + rValue);
}

if (isString(l) && isString(r)) {
Expand All @@ -705,30 +748,58 @@ OptVariantType ArithmeticExpression::eval(Getters &getters) const {
if (isDouble(l) || isDouble(r)) {
return OptVariantType(asDouble(l) - asDouble(r));
}
return OptVariantType(asInt(l) - asInt(r));
int64_t lValue = asInt(l);
int64_t rValue = asInt(r);
if (isSubOverflow(lValue, rValue)) {
return Status::Error(folly::stringPrintf("Out of range %ld - %ld",
lValue, rValue));
}
return OptVariantType(lValue - rValue);
}
break;
case MUL:
if (isArithmetic(l) && isArithmetic(r)) {
if (isDouble(l) || isDouble(r)) {
return OptVariantType(asDouble(l) * asDouble(r));
}
return OptVariantType(asInt(l) * asInt(r));
int64_t lValue = asInt(l);
int64_t rValue = asInt(r);
if (isMulOverflow(lValue, rValue)) {
return Status::Error("Out of range %ld * %ld", lValue, rValue);
}
return OptVariantType(lValue * rValue);
}
break;
case DIV:
if (isArithmetic(l) && isArithmetic(r)) {
if (isDouble(l) || isDouble(r)) {
if (abs(asDouble(r)) < 1e-8) {
// When Null is supported, should be return NULL
return Status::Error("Division by zero");
}
return OptVariantType(asDouble(l) / asDouble(r));
}

if (abs(asInt(r)) == 0) {
// When Null is supported, should be return NULL
return Status::Error("Division by zero");
}
return OptVariantType(asInt(l) / asInt(r));
}
break;
case MOD:
if (isArithmetic(l) && isArithmetic(r)) {
if (isDouble(l) || isDouble(r)) {
if (abs(asDouble(r)) < 1e-8) {
// When Null is supported, should be return NULL
return Status::Error("Division by zero");
}
return fmod(asDouble(l), asDouble(r));
}
if (abs(asInt(r)) == 0) {
// When Null is supported, should be return NULL
return Status::Error("Division by zero");
}
return OptVariantType(asInt(l) % asInt(r));
}
break;
Expand Down
84 changes: 84 additions & 0 deletions src/graph/test/YieldTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,89 @@ TEST_F(YieldTest, Error) {
}
}

TEST_F(YieldTest, calculateOverflow) {
auto client = gEnv->getClient();
ASSERT_NE(nullptr, client);
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 9223372036854775807+1";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD -9223372036854775807-2";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD -9223372036854775807+-2";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 1-(-9223372036854775807)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 9223372036854775807*2";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD -9223372036854775807*-2";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 9223372036854775807*-2";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD -9223372036854775807*2";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 1/0";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 2%0";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD 9223372036854775807";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD -9223372036854775808";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "YIELD -9223372036854775809";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_SYNTAX_ERROR, code);
}
}

TEST_F(YieldTest, AggCall) {
{
cpp2::ExecutionResponse resp;
Expand Down Expand Up @@ -653,3 +736,4 @@ TEST_F(YieldTest, EmptyInput) {
}
} // namespace graph
} // namespace nebula

69 changes: 58 additions & 11 deletions src/parser/parser.yy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class GraphScanner;
static int yylex(nebula::GraphParser::semantic_type* yylval,
nebula::GraphParser::location_type *yylloc,
nebula::GraphScanner& scanner);

void ifOutOfRange(const int64_t input,
const nebula::GraphParser::location_type& loc);
}

%union {
Expand Down Expand Up @@ -131,7 +134,7 @@ class GraphScanner;
%type <strval> name_label unreserved_keyword agg_function
%type <expr> expression logic_xor_expression logic_or_expression logic_and_expression
%type <expr> relational_expression multiplicative_expression additive_expression arithmetic_xor_expression
%type <expr> unary_expression primary_expression equality_expression
%type <expr> unary_expression primary_expression equality_expression base_expression
%type <expr> src_ref_expression
%type <expr> dst_ref_expression
%type <expr> input_ref_expression
Expand Down Expand Up @@ -266,6 +269,10 @@ unreserved_keyword
| KW_BIT_AND { $$ = new std::string("bit_and"); }
| KW_BIT_OR { $$ = new std::string("bit_or"); }
| KW_BIT_XOR { $$ = new std::string("bit_xor"); }
| KW_PATH { $$ = new std::string("path"); }
| KW_DATA { $$ = new std::string("data"); }
| KW_LEADER { $$ = new std::string("leader"); }
| KW_UUID { $$ = new std::string("uuid"); }
;

agg_function
Expand All @@ -283,9 +290,21 @@ agg_function

primary_expression
: INTEGER {
ifOutOfRange($1, @1);
$$ = new PrimaryExpression($1);
}
| DOUBLE {
| MINUS INTEGER {
$$ = new PrimaryExpression(-$2);;
}
| MINUS base_expression {
$$ = new UnaryExpression(UnaryExpression::NEGATE, $2);
}
| base_expression {
$$ = $1;
}

base_expression
: DOUBLE {
$$ = new PrimaryExpression($1);
}
| STRING {
Expand Down Expand Up @@ -414,9 +433,6 @@ unary_expression
| PLUS unary_expression {
$$ = new UnaryExpression(UnaryExpression::PLUS, $2);
}
| MINUS unary_expression {
$$ = new UnaryExpression(UnaryExpression::NEGATE, $2);
}
| NOT unary_expression {
$$ = new UnaryExpression(UnaryExpression::NOT, $2);
}
Expand Down Expand Up @@ -551,8 +567,14 @@ go_sentence

step_clause
: %empty { $$ = new StepClause(); }
| INTEGER KW_STEPS { $$ = new StepClause($1); }
| KW_UPTO INTEGER KW_STEPS { $$ = new StepClause($2, true); }
| INTEGER KW_STEPS {
ifOutOfRange($1, @1);
$$ = new StepClause($1);
}
| KW_UPTO INTEGER KW_STEPS {
ifOutOfRange($2, @2);
$$ = new StepClause($2, true);
}
;

from_clause
Expand Down Expand Up @@ -589,12 +611,14 @@ vid

unary_integer
: PLUS INTEGER {
ifOutOfRange($2, @2);
$$ = $2;
}
| MINUS INTEGER {
$$ = -$2;
}
| INTEGER {
ifOutOfRange($1, @1);
$$ = $1;
}
;
Expand Down Expand Up @@ -868,7 +892,10 @@ find_path_sentence

find_path_upto_clause
: %empty { $$ = new StepClause(5, true); }
| KW_UPTO INTEGER KW_STEPS { $$ = new StepClause($2, true); }
| KW_UPTO INTEGER KW_STEPS {
ifOutOfRange($2, @2);
$$ = new StepClause($2, true);
}
;

to_clause
Expand All @@ -881,9 +908,20 @@ to_clause
;

limit_sentence
: KW_LIMIT INTEGER { $$ = new LimitSentence(0, $2); }
| KW_LIMIT INTEGER COMMA INTEGER { $$ = new LimitSentence($2, $4); }
| KW_LIMIT INTEGER KW_OFFSET INTEGER { $$ = new LimitSentence($2, $4); }
: KW_LIMIT INTEGER {
ifOutOfRange($2, @2);
$$ = new LimitSentence(0, $2);
}
| KW_LIMIT INTEGER COMMA INTEGER {
ifOutOfRange($2, @2);
ifOutOfRange($4, @2);
$$ = new LimitSentence($2, $4);
}
| KW_LIMIT INTEGER KW_OFFSET INTEGER {
ifOutOfRange($2, @2);
ifOutOfRange($4, @4);
$$ = new LimitSentence($2, $4);
}
;

group_by_sentence
Expand Down Expand Up @@ -1632,9 +1670,11 @@ space_opt_list

space_opt_item
: KW_PARTITION_NUM ASSIGN INTEGER {
ifOutOfRange($3, @3);
$$ = new SpaceOptItem(SpaceOptItem::PARTITION_NUM, $3);
}
| KW_REPLICA_FACTOR ASSIGN INTEGER {
ifOutOfRange($3, @3);
$$ = new SpaceOptItem(SpaceOptItem::REPLICA_FACTOR, $3);
}
// TODO(YT) Create Spaces for different engines
Expand Down Expand Up @@ -1822,6 +1862,7 @@ balance_sentence
$$ = new BalanceSentence(BalanceSentence::SubType::kData);
}
| KW_BALANCE KW_DATA INTEGER {
ifOutOfRange($3, @3);
$$ = new BalanceSentence($3);
}
| KW_BALANCE KW_DATA KW_STOP {
Expand Down Expand Up @@ -1967,6 +2008,12 @@ void nebula::GraphParser::error(const nebula::GraphParser::location_type& loc,
errmsg = os.str();
}

void ifOutOfRange(const int64_t input,
const nebula::GraphParser::location_type& loc) {
if ((uint64_t)input == 9223372036854775808ULL) {
throw nebula::GraphParser::syntax_error(loc, "out of range");
}
}

static int yylex(nebula::GraphParser::semantic_type* yylval,
nebula::GraphParser::location_type *yylloc,
Expand Down
6 changes: 5 additions & 1 deletion src/parser/scanner.lex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,11 @@ IP_OCTET ([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])
{DEC}+ {
try {
folly::StringPiece text(yytext, yyleng);
yylval->intval = folly::to<int64_t>(text);
uint64_t val = folly::to<uint64_t>(text);
if (val > 9223372036854775808ULL) {
yyterminate();
}
yylval->intval = val;
} catch (...) {
yyterminate();
}
Expand Down
Loading

0 comments on commit fa6791c

Please sign in to comment.