Hello Andres,
I'll come up with a patch for that sometime soon.
ISTM that you have not sent any patch on the subject, otherwise I would
have reviewed it. Maybe I could do one some time later, unless you think
that I should not.
Here is a patch which detects pgbench overflows on int & double constants,
and on integer operators.
--
Fabien.
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 88cf8b3933..8c464c9d42 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -989,6 +989,13 @@ pgbench <optional> <replaceable>options</replaceable>
</optional> <replaceable>d
are <literal>FALSE</literal>.
</para>
+ <para>
+ Too large or small integer and double constants, as well as
+ integer arithmetic operators (<literal>+</literal>,
+ <literal>-</literal>, <literal>*</literal> and <literal>/</literal>)
+ raise errors on overflows.
+ </para>
+
<para>
When no final <token>ELSE</token> clause is provided to a
<token>CASE</token>, the default value is <literal>NULL</literal>.
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index 5c1bd88128..3e82437bc8 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -192,15 +192,21 @@ notnull [Nn][Oo][Tt][Nn][Uu][Ll][Ll]
return BOOLEAN_CONST;
}
{digit}+ {
- yylval->ival = strtoint64(yytext);
+ if (!strtoint64(yytext, true,
&yylval->ival))
+ expr_yyerror_more(yyscanner,
"bigint constant overflow",
+
strdup(yytext));
return INTEGER_CONST;
}
{digit}+(\.{digit}*)?([eE][-+]?{digit}+)? {
- yylval->dval = atof(yytext);
+ if (!strtodouble(yytext, true,
&yylval->dval))
+ expr_yyerror_more(yyscanner,
"double constant overflow",
+
strdup(yytext));
return DOUBLE_CONST;
}
\.{digit}+([eE][-+]?{digit}+)? {
- yylval->dval = atof(yytext);
+ if (!strtodouble(yytext, true,
&yylval->dval))
+ expr_yyerror_more(yyscanner,
"double constant overflow",
+
strdup(yytext));
return DOUBLE_CONST;
}
{alpha}{alnum}* {
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index f0c5149523..3f1392fc03 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -38,6 +38,8 @@
#include "libpq-fe.h"
#include "portability/instr_time.h"
+#include "common/int.h" /* integer overflow checks */
+
#include <ctype.h>
#include <float.h>
#include <limits.h>
@@ -627,19 +629,24 @@ is_an_int(const char *str)
/*
* strtoint64 -- convert a string to 64-bit integer
*
- * This function is a modified version of scanint8() from
+ * This function is a slightly modified version of scanint8() from
* src/backend/utils/adt/int8.c.
+ *
+ * If not errorOK, an error message is printed out.
*/
-int64
-strtoint64(const char *str)
+bool
+strtoint64(const char *str, bool errorOK, int64 *result)
{
const char *ptr = str;
- int64 result = 0;
- int sign = 1;
+ int64 tmp = 0;
+ bool neg = false;
/*
* Do our own scan, rather than relying on sscanf which might be broken
* for long long.
+ *
+ * As INT64_MIN can't be stored as a positive 64 bit integer, accumulate
+ * value as a negative number.
*/
/* skip leading spaces */
@@ -650,46 +657,80 @@ strtoint64(const char *str)
if (*ptr == '-')
{
ptr++;
-
- /*
- * Do an explicit check for INT64_MIN. Ugly though this is,
it's
- * cleaner than trying to get the loop below to handle it
portably.
- */
- if (strncmp(ptr, "9223372036854775808", 19) == 0)
- {
- result = PG_INT64_MIN;
- ptr += 19;
- goto gotdigits;
- }
- sign = -1;
+ neg = true;
}
else if (*ptr == '+')
ptr++;
/* require at least one digit */
- if (!isdigit((unsigned char) *ptr))
- fprintf(stderr, "invalid input syntax for integer: \"%s\"\n",
str);
+ if (unlikely(!isdigit((unsigned char) *ptr)))
+ goto invalid_syntax;
/* process digits */
while (*ptr && isdigit((unsigned char) *ptr))
{
- int64 tmp = result * 10 + (*ptr++ - '0');
+ int8 digit = (*ptr++ - '0');
- if ((tmp / 10) != result) /* overflow? */
- fprintf(stderr, "value \"%s\" is out of range for type
bigint\n", str);
- result = tmp;
+ if (unlikely(pg_mul_s64_overflow(tmp, 10, &tmp)) ||
+ unlikely(pg_sub_s64_overflow(tmp, digit, &tmp)))
+ goto out_of_range;
}
-gotdigits:
-
/* allow trailing whitespace, but not other trailing chars */
while (*ptr != '\0' && isspace((unsigned char) *ptr))
ptr++;
- if (*ptr != '\0')
- fprintf(stderr, "invalid input syntax for integer: \"%s\"\n",
str);
+ if (unlikely(*ptr != '\0'))
+ goto invalid_syntax;
- return ((sign < 0) ? -result : result);
+ if (!neg)
+ {
+ if (unlikely(tmp == PG_INT64_MIN))
+ goto out_of_range;
+ tmp = -tmp;
+ }
+
+ *result = tmp;
+ return true;
+
+out_of_range:
+ if (!errorOK)
+ fprintf(stderr,
+ "value \"%s\" is out of range for type
bigint\n", str);
+ return false;
+
+invalid_syntax:
+ /* this can't happen here, function called on int-looking strings. */
+ if (!errorOK)
+ fprintf(stderr,
+ "invalid input syntax for type bigint:
\"%s\"\n",str);
+ return false;
+}
+
+/* convert string to double, detecting overflows/underflows */
+bool
+strtodouble(const char *str, bool errorOK, double *dv)
+{
+ char *end;
+
+ *dv = strtod(str, &end);
+
+ if (unlikely(errno != 0))
+ {
+ if (!errorOK)
+ fprintf(stderr,
+ "value \"%s\" is out of range for type
double\n", str);
+ return false;
+ }
+
+ if (unlikely(end == str || *end != '\0'))
+ {
+ if (!errorOK)
+ fprintf(stderr,
+ "invalid input syntax for type double:
\"%s\"\n",str);
+ return false;
+ }
+ return true;
}
/* random number generator: uniform distribution from min to max inclusive */
@@ -1282,20 +1323,26 @@ makeVariableValue(Variable *var)
}
else if (is_an_int(var->svalue))
{
- setIntValue(&var->value, strtoint64(var->svalue));
+ /* if it looks like an int, it must be an int without overflow
*/
+ int64 iv;
+
+ if (!strtoint64(var->svalue, false, &iv))
+ return false;
+
+ setIntValue(&var->value, iv);
}
else /* type should be
double */
{
double dv;
- char xs;
- if (sscanf(var->svalue, "%lf%c", &dv, &xs) != 1)
+ if (!strtodouble(var->svalue, true, &dv))
{
fprintf(stderr,
"malformed variable \"%s\" value:
\"%s\"\n",
var->name, var->svalue);
return false;
}
+
setDoubleValue(&var->value, dv);
}
return true;
@@ -1905,7 +1952,8 @@ evalStandardFunc(TState *thread, CState *st,
else /* we have integer
operands, or % */
{
int64 li,
- ri;
+ ri,
+ res;
if (!coerceToInt(lval, &li) ||
!coerceToInt(rval, &ri))
@@ -1914,14 +1962,29 @@ evalStandardFunc(TState *thread, CState *st,
switch (func)
{
case PGBENCH_ADD:
- setIntValue(retval, li
+ ri);
+ if
(unlikely(pg_add_s64_overflow(li, ri, &res)))
+ {
+ fprintf(stderr,
"bigint add out of range\n");
+ return false;
+ }
+ setIntValue(retval,
res);
return true;
case PGBENCH_SUB:
- setIntValue(retval, li
- ri);
+ if
(unlikely(pg_sub_s64_overflow(li, ri, &res)))
+ {
+ fprintf(stderr,
"bigint sub out of range\n");
+ return false;
+ }
+ setIntValue(retval,
res);
return true;
case PGBENCH_MUL:
+ if
(unlikely(pg_mul_s64_overflow(li, ri, &res)))
+ {
+ fprintf(stderr,
"bigint mul out of range\n");
+ return false;
+ }
setIntValue(retval, li
* ri);
return true;
@@ -1954,9 +2017,9 @@ evalStandardFunc(TState *thread, CState *st,
if (func ==
PGBENCH_DIV)
{
/*
overflow check (needed for INT64_MIN) */
- if (li
== PG_INT64_MIN)
+ if
(unlikely(li == PG_INT64_MIN))
{
-
fprintf(stderr, "bigint out of range\n");
+
fprintf(stderr, "bigint div out of range\n");
return false;
}
else
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index 6983865b92..de50340434 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -160,6 +160,7 @@ extern void syntax_error(const char *source, int lineno,
const char *line,
const char *cmd, const char *msg,
const char *more, int col) pg_attribute_noreturn();
-extern int64 strtoint64(const char *str);
+extern bool strtoint64(const char *str, bool errorOK, int64 *pi);
+extern bool strtodouble(const char *str, bool errorOK, double *pd);
#endif /* PGBENCH_H */
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl
b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 2fc021dde7..87ac919e0a 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -255,7 +255,7 @@ COMMIT;
# test expressions
# command 1..3 and 23 depend on random seed which is used to call srandom.
pgbench(
- '--random-seed=5432 -t 1 -Dfoo=-10.1 -Dbla=false -Di=+3
-Dminint=-9223372036854775808 -Dn=null -Dt=t -Df=of -Dd=1.0',
+ '--random-seed=5432 -t 1 -Dfoo=-10.1 -Dbla=false -Di=+3 -Dn=null -Dt=t
-Df=of -Dd=1.0',
0,
[ qr{type: .*/001_pgbench_expressions}, qr{processed: 1/1} ],
[
@@ -278,7 +278,6 @@ pgbench(
qr{command=15.: double 15\b},
qr{command=16.: double 16\b},
qr{command=17.: double 17\b},
- qr{command=18.: int 9223372036854775807\b},
qr{command=20.: int \d\b}, # zipfian random: 1 on linux
qr{command=21.: double -27\b},
qr{command=22.: double 1024\b},
@@ -345,10 +344,9 @@ pgbench(
\set pi debug(pi() * 4.9)
\set d4 debug(greatest(4, 2, -1.17) * 4.0 * Ln(Exp(1.0)))
\set d5 debug(least(-5.18, .0E0, 1.0/0) * -3.3)
--- forced overflow
-\set maxint debug(:minint - 1)
--- reset a variable
+-- reset variables
\set i1 0
+\set d1 false
-- yet another integer function
\set id debug(random_zipfian(1, 9, 1.3))
--- pow and power
@@ -601,16 +599,10 @@ SELECT LEAST(:i, :i, :i, :i, :i, :i, :i, :i, :i, :i, :i);
[qr{invalid variable name}], q{\set . 1}
],
[
- 'set int overflow', 0,
- [qr{double to int overflow for 100}], q{\set i int(1E32)}
+ 'set division by zero', 0,
+ [qr{division by zero}], q{\set i 1/0}
],
- [ 'set division by zero', 0, [qr{division by zero}], q{\set i 1/0} ],
- [
- 'set bigint out of range', 0,
- [qr{bigint out of range}], q{\set i 9223372036854775808 / -1}
- ],
- [
- 'set undefined variable',
+ [ 'set undefined variable',
0,
[qr{undefined variable "nosuchvariable"}],
q{\set i :nosuchvariable}
@@ -630,7 +622,7 @@ SELECT LEAST(:i, :i, :i, :i, :i, :i, :i, :i, :i, :i, :i);
'set random range too large',
0,
[qr{random range is too large}],
- q{\set i random(-9223372036854775808, 9223372036854775807)}
+ q{\set i random(-:maxint - 1, :maxint)}
],
[
'set gaussian param too small',
@@ -693,6 +685,18 @@ SELECT LEAST(:i, :i, :i, :i, :i, :i, :i, :i, :i, :i, :i);
[qr{at least one argument expected}], q{\set i greatest())}
],
+ # SET: ARITHMETIC OVERFLOW DETECTION
+ [ 'set double to int overflow', 0,
+ [ qr{double to int overflow for 100} ], q{\set i int(1E32)} ],
+ [ 'set bigint add overflow', 0,
+ [ qr{int add out} ], q{\set i (1<<62) + (1<<62)} ],
+ [ 'set bigint sub overflow', 0,
+ [ qr{int sub out} ], q{\set i 0 - (1<<62) - (1<<62) - (1<<62)}
],
+ [ 'set bigint mul overflow', 0,
+ [ qr{int mul out} ], q{\set i 2 * (1<<62)} ],
+ [ 'set bigint div out of range', 0,
+ [ qr{bigint div out of range} ], q{\set i (-:maxint - 1) / -1}
],
+
# SETSHELL
[
'setshell not an int', 0,
@@ -740,7 +744,7 @@ for my $e (@errors)
my $n = '001_pgbench_error_' . $name;
$n =~ s/ /_/g;
pgbench(
- '-n -t 1 -Dfoo=bla -Dnull=null -Dtrue=true -Done=1 -Dzero=0.0
-Dbadtrue=trueXXX -M prepared',
+ '-n -t 1 -Dfoo=bla -Dnull=null -Dtrue=true -Done=1 -Dzero=0.0
-Dbadtrue=trueXXX -Dmaxint=9223372036854775807 -M prepared',
$status,
[ $status ? qr{^$} : qr{processed: 0/1} ],
$re,
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl
b/src/bin/pgbench/t/002_pgbench_no_server.pl
index a2845a583b..11e6828b32 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -270,6 +270,22 @@ my @script_tests = (
'endif syntax error',
[qr{unexpected argument in command "endif"}],
{ 'endif-bad.sql' => "\\if 0\n\\endif BAD\n" }
+ ],
+ # overflow
+ [
+ 'bigint overflow',
+ [qr{bigint constant overflow}],
+ { 'overflow-1.sql' => "\\set i 100000000000000000000\n" }
+ ],
+ [
+ 'double overflow 1',
+ [qr{double constant overflow}],
+ { 'overflow-2.sql' => "\\set d 1.0E309\n" }
+ ],
+ [
+ 'double overflow 2',
+ [qr{double constant overflow}],
+ { 'overflow-2.sql' => "\\set d .1E310\n" }
],);
for my $t (@script_tests)
