Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 92a0342

Browse filesBrowse files
anarazelanarazel
committed
Correct overflow handling in pgbench.
This patch attempts, although it's quite possible there are a few holes, to properly detect and reported signed integer overflows in pgbench. Author: Fabien Coelho Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20171212052943.k2hlckfkeft3eiio@alap3.anarazel.de
1 parent 78ea8b5 commit 92a0342
Copy full SHA for 92a0342

File tree

Expand file treeCollapse file tree

7 files changed

+174
-58
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+174
-58
lines changed

‎doc/src/sgml/ref/pgbench.sgml

Copy file name to clipboardExpand all lines: doc/src/sgml/ref/pgbench.sgml
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -989,6 +989,13 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
989989
are <literal>FALSE</literal>.
990990
</para>
991991

992+
<para>
993+
Too large or small integer and double constants, as well as
994+
integer arithmetic operators (<literal>+</literal>,
995+
<literal>-</literal>, <literal>*</literal> and <literal>/</literal>)
996+
raise errors on overflows.
997+
</para>
998+
992999
<para>
9931000
When no final <token>ELSE</token> clause is provided to a
9941001
<token>CASE</token>, the default value is <literal>NULL</literal>.

‎src/bin/pgbench/exprparse.y

Copy file name to clipboardExpand all lines: src/bin/pgbench/exprparse.y
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ static PgBenchExpr *make_case(yyscan_t yyscanner, PgBenchExprList *when_then_lis
6161
%type <bval> BOOLEAN_CONST
6262
%type <str> VARIABLE FUNCTION
6363

64-
%token NULL_CONST INTEGER_CONST DOUBLE_CONST BOOLEAN_CONST VARIABLE FUNCTION
64+
%token NULL_CONST INTEGER_CONST MAXINT_PLUS_ONE_CONST DOUBLE_CONST
65+
%token BOOLEAN_CONST VARIABLE FUNCTION
6566
%token AND_OP OR_OP NOT_OP NE_OP LE_OP GE_OP LS_OP RS_OP IS_OP
6667
%token CASE_KW WHEN_KW THEN_KW ELSE_KW END_KW
6768

@@ -90,6 +91,9 @@ expr: '(' expr ')' { $$ = $2; }
9091
/* unary minus "-x" implemented as "0 - x" */
9192
| '-' expr %prec UNARY { $$ = make_op(yyscanner, "-",
9293
make_integer_constant(0), $2); }
94+
/* special PG_INT64_MIN handling, only after a unary minus */
95+
| '-' MAXINT_PLUS_ONE_CONST %prec UNARY
96+
{ $$ = make_integer_constant(PG_INT64_MIN); }
9397
/* binary ones complement "~x" implemented as 0xffff... xor x" */
9498
| '~' expr { $$ = make_op(yyscanner, "#",
9599
make_integer_constant(~INT64CONST(0)), $2); }

‎src/bin/pgbench/exprscan.l

Copy file name to clipboardExpand all lines: src/bin/pgbench/exprscan.l
+18-3Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,16 +195,31 @@ notnull [Nn][Oo][Tt][Nn][Uu][Ll][Ll]
195195
yylval->bval = false;
196196
return BOOLEAN_CONST;
197197
}
198+
"9223372036854775808" {
199+
/*
200+
* Special handling for PG_INT64_MIN, which can't
201+
* accurately be represented here, as the minus sign is
202+
* lexed separately and INT64_MIN can't be represented as
203+
* a positive integer.
204+
*/
205+
return MAXINT_PLUS_ONE_CONST;
206+
}
198207
{digit}+ {
199-
yylval->ival = strtoint64(yytext);
208+
if (!strtoint64(yytext, true, &yylval->ival))
209+
expr_yyerror_more(yyscanner, "bigint constant overflow",
210+
strdup(yytext));
200211
return INTEGER_CONST;
201212
}
202213
{digit}+(\.{digit}*)?([eE][-+]?{digit}+)? {
203-
yylval->dval = atof(yytext);
214+
if (!strtodouble(yytext, true, &yylval->dval))
215+
expr_yyerror_more(yyscanner, "double constant overflow",
216+
strdup(yytext));
204217
return DOUBLE_CONST;
205218
}
206219
\.{digit}+([eE][-+]?{digit}+)? {
207-
yylval->dval = atof(yytext);
220+
if (!strtodouble(yytext, true, &yylval->dval))
221+
expr_yyerror_more(yyscanner, "double constant overflow",
222+
strdup(yytext));
208223
return DOUBLE_CONST;
209224
}
210225
{alpha}{alnum}* {

‎src/bin/pgbench/pgbench.c

Copy file name to clipboardExpand all lines: src/bin/pgbench/pgbench.c
+100-37Lines changed: 100 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@
3232
#endif
3333

3434
#include "postgres_fe.h"
35+
#include "common/int.h"
3536
#include "fe_utils/conditional.h"
36-
3737
#include "getopt_long.h"
3838
#include "libpq-fe.h"
3939
#include "portability/instr_time.h"
@@ -662,19 +662,27 @@ is_an_int(const char *str)
662662
/*
663663
* strtoint64 -- convert a string to 64-bit integer
664664
*
665-
* This function is a modified version of scanint8() from
665+
* This function is a slightly modified version of scanint8() from
666666
* src/backend/utils/adt/int8.c.
667+
*
668+
* The function returns whether the conversion worked, and if so
669+
* "*result" is set to the result.
670+
*
671+
* If not errorOK, an error message is also printed out on errors.
667672
*/
668-
int64
669-
strtoint64(const char *str)
673+
bool
674+
strtoint64(const char *str, bool errorOK, int64 *result)
670675
{
671676
const char *ptr = str;
672-
int64 result = 0;
673-
int sign = 1;
677+
int64 tmp = 0;
678+
bool neg = false;
674679

675680
/*
676681
* Do our own scan, rather than relying on sscanf which might be broken
677682
* for long long.
683+
*
684+
* As INT64_MIN can't be stored as a positive 64 bit integer, accumulate
685+
* value as a negative number.
678686
*/
679687

680688
/* skip leading spaces */
@@ -685,46 +693,80 @@ strtoint64(const char *str)
685693
if (*ptr == '-')
686694
{
687695
ptr++;
688-
689-
/*
690-
* Do an explicit check for INT64_MIN. Ugly though this is, it's
691-
* cleaner than trying to get the loop below to handle it portably.
692-
*/
693-
if (strncmp(ptr, "9223372036854775808", 19) == 0)
694-
{
695-
result = PG_INT64_MIN;
696-
ptr += 19;
697-
goto gotdigits;
698-
}
699-
sign = -1;
696+
neg = true;
700697
}
701698
else if (*ptr == '+')
702699
ptr++;
703700

704701
/* require at least one digit */
705-
if (!isdigit((unsigned char) *ptr))
706-
fprintf(stderr, "invalid input syntax for integer: \"%s\"\n", str);
702+
if (unlikely(!isdigit((unsigned char) *ptr)))
703+
goto invalid_syntax;
707704

708705
/* process digits */
709706
while (*ptr && isdigit((unsigned char) *ptr))
710707
{
711-
int64 tmp = result * 10 + (*ptr++ - '0');
708+
int8 digit = (*ptr++ - '0');
712709

713-
if ((tmp / 10) != result) /* overflow? */
714-
fprintf(stderr, "value \"%s\" is out of range for type bigint\n", str);
715-
result = tmp;
710+
if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) ||
711+
unlikely(pg_sub_s64_overflow(tmp, digit, &tmp)))
712+
goto out_of_range;
716713
}
717714

718-
gotdigits:
719-
720715
/* allow trailing whitespace, but not other trailing chars */
721716
while (*ptr != '\0' && isspace((unsigned char) *ptr))
722717
ptr++;
723718

724-
if (*ptr != '\0')
725-
fprintf(stderr, "invalid input syntax for integer: \"%s\"\n", str);
719+
if (unlikely(*ptr != '\0'))
720+
goto invalid_syntax;
721+
722+
if (!neg)
723+
{
724+
if (unlikely(tmp == PG_INT64_MIN))
725+
goto out_of_range;
726+
tmp = -tmp;
727+
}
728+
729+
*result = tmp;
730+
return true;
731+
732+
out_of_range:
733+
if (!errorOK)
734+
fprintf(stderr,
735+
"value \"%s\" is out of range for type bigint\n", str);
736+
return false;
726737

727-
return ((sign < 0) ? -result : result);
738+
invalid_syntax:
739+
if (!errorOK)
740+
fprintf(stderr,
741+
"invalid input syntax for type bigint: \"%s\"\n",str);
742+
return false;
743+
}
744+
745+
/* convert string to double, detecting overflows/underflows */
746+
bool
747+
strtodouble(const char *str, bool errorOK, double *dv)
748+
{
749+
char *end;
750+
751+
errno = 0;
752+
*dv = strtod(str, &end);
753+
754+
if (unlikely(errno != 0))
755+
{
756+
if (!errorOK)
757+
fprintf(stderr,
758+
"value \"%s\" is out of range for type double\n", str);
759+
return false;
760+
}
761+
762+
if (unlikely(end == str || *end != '\0'))
763+
{
764+
if (!errorOK)
765+
fprintf(stderr,
766+
"invalid input syntax for type double: \"%s\"\n",str);
767+
return false;
768+
}
769+
return true;
728770
}
729771

730772
/* random number generator: uniform distribution from min to max inclusive */
@@ -1320,14 +1362,19 @@ makeVariableValue(Variable *var)
13201362
}
13211363
else if (is_an_int(var->svalue))
13221364
{
1323-
setIntValue(&var->value, strtoint64(var->svalue));
1365+
/* if it looks like an int, it must be an int without overflow */
1366+
int64 iv;
1367+
1368+
if (!strtoint64(var->svalue, false, &iv))
1369+
return false;
1370+
1371+
setIntValue(&var->value, iv);
13241372
}
13251373
else /* type should be double */
13261374
{
13271375
double dv;
1328-
char xs;
13291376

1330-
if (sscanf(var->svalue, "%lf%c", &dv, &xs) != 1)
1377+
if (!strtodouble(var->svalue, true, &dv))
13311378
{
13321379
fprintf(stderr,
13331380
"malformed variable \"%s\" value: \"%s\"\n",
@@ -1943,7 +1990,8 @@ evalStandardFunc(TState *thread, CState *st,
19431990
else /* we have integer operands, or % */
19441991
{
19451992
int64 li,
1946-
ri;
1993+
ri,
1994+
res;
19471995

19481996
if (!coerceToInt(lval, &li) ||
19491997
!coerceToInt(rval, &ri))
@@ -1952,15 +2000,30 @@ evalStandardFunc(TState *thread, CState *st,
19522000
switch (func)
19532001
{
19542002
case PGBENCH_ADD:
1955-
setIntValue(retval, li + ri);
2003+
if (pg_add_s64_overflow(li, ri, &res))
2004+
{
2005+
fprintf(stderr, "bigint add out of range\n");
2006+
return false;
2007+
}
2008+
setIntValue(retval, res);
19562009
return true;
19572010

19582011
case PGBENCH_SUB:
1959-
setIntValue(retval, li - ri);
2012+
if (pg_sub_s64_overflow(li, ri, &res))
2013+
{
2014+
fprintf(stderr, "bigint sub out of range\n");
2015+
return false;
2016+
}
2017+
setIntValue(retval, res);
19602018
return true;
19612019

19622020
case PGBENCH_MUL:
1963-
setIntValue(retval, li * ri);
2021+
if (pg_mul_s64_overflow(li, ri, &res))
2022+
{
2023+
fprintf(stderr, "bigint mul out of range\n");
2024+
return false;
2025+
}
2026+
setIntValue(retval, res);
19642027
return true;
19652028

19662029
case PGBENCH_EQ:
@@ -1994,7 +2057,7 @@ evalStandardFunc(TState *thread, CState *st,
19942057
/* overflow check (needed for INT64_MIN) */
19952058
if (li == PG_INT64_MIN)
19962059
{
1997-
fprintf(stderr, "bigint out of range\n");
2060+
fprintf(stderr, "bigint div out of range\n");
19982061
return false;
19992062
}
20002063
else

‎src/bin/pgbench/pgbench.h

Copy file name to clipboardExpand all lines: src/bin/pgbench/pgbench.h
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ extern void syntax_error(const char *source, int lineno, const char *line,
160160
const char *cmd, const char *msg,
161161
const char *more, int col) pg_attribute_noreturn();
162162

163-
extern int64 strtoint64(const char *str);
163+
extern bool strtoint64(const char *str, bool errorOK, int64 *pi);
164+
extern bool strtodouble(const char *str, bool errorOK, double *pd);
164165

165166
#endif /* PGBENCH_H */

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.