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.

Sign up for GitHub

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

Fix arithmetic overflow and set int boundary value #1181

Merged
merged 5 commits into from
Jan 7, 2020
Merged

Fix arithmetic overflow and set int boundary value #1181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
037881a
Fix arithmetic overflow
laura-ding Oct 30, 2019
5c81c42
fix #826 Int value range is incorrect
laura-ding Nov 1, 2019
41a317d
rebase upstream
laura-ding Nov 1, 2019
90129ef
rebase upstream
laura-ding Dec 31, 2019
fd06c48
Merge branch 'master' into fixOverflow
dutor Jan 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 74 additions & 3 deletions src/common/filter/Expressions.cpp
View file
Open in desktop
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 {
laura-ding marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Open in desktop
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