Re: BUG #16186: The usage of undefined value in pgbench.c

你好 Jian,

> Bug reference: 16186
> Logged by: Jian Zhang
> Email address: starbugs(at)qq(dot)com
> PostgreSQL version: 12.1
> Operating system: Linux
> Description:
>
> We checked the code in file “pgbench.c” and there are three errors occurring
> in lines 1900, 2100 and 2357 in function evalStandardFunc.

> All the three errors are caused by the usage of variables with undefined
> values.

The word "usage" suggest that they are actually used, but I do not think
that it is the case.

> Firstly, in line 1900, the code is “if ((lval->type == PGBT_DOUBLE ||
> rval->type == PGBT_DOUBLE) && func != PGBENCH_MOD)”. The pointer “lval”
> mentioned in this line is defined by the code in line 1894 as
> “PgBenchValue *lval = &vargs[0], *rval = &vargs[1];”, so it is assigned
> as the address of “vargs[0]”.

Yes. So?

The vargs array is initialized at the beginning of the function in a for
loop, as you noted below.

The expression parser checks that the number of arguments is okay for each
function/operator (the arity is stored PGBENCH_FUNCTIONS array in
"exprparse.y" and checked in "make_func" default case in switch), so that
for all the functions of the case at hand we know that there have two
arguments, so vargs[0] and vargs[1] contents will be defined.

There is also an assert on nargs to cover that, just in case.

If there NULL they are dealt with at the beginning of the function
(has_null), and NO_VALUE is never used, so the type must be defined to
something.

Maybe some assert could be moved before the declarations for the
principle, but when the code was written pg was not C99 yet.

> Secondly, in line 2100, the code is “if (varg->type == PGBT_INT)”. The
> pointer “varg” mentioned in this line is defined by the code in line 2096:
> “PgBenchValue *varg = &vargs[0];”, so it is also assigned as the address of
> “vargs[0]”.

Same analysis: exprparse checked that abs has one arg, and moreover
there is an assert to cover it again.

> Lastly, in line 2357, the code is “vargs[0].type == vargs[1].type
> &&vargs[0].u.bval == vargs[1].u.bval);”. The 1st and 2nd elements of
> array “vargs” is directly used without confirming weather the array is
> correctly defined or not.

Same analysis.

Ok, the "bval" access is doubtful because we do not know whether it is
actually a boolean. I'll think about it.

> The array “vargs” is defined by the code “PgBenchValue
> vargs[MAX_FARGS];” in line 1855 and is initialized in the function of
> “evaluateExpr” in line 1861, the code is “if (!evaluateExpr(st, l->expr,
> &vargs[nargs]))”. So the assignment of array “vargs” depends on both the
> input pointer “st” and the pointer “I” defined by the input parameter
> “args”. All the input parameters of function “evalStandardFunc” are
> listed in line 1849. The code is “evalStandardFunc(CState *st,
> PgBenchFunction func, PgBenchExprLink *args, PgBenchValue *retval)”. The
> program should check the effectiveness of input parameters “st” and
> “args” to avoid these three errors.

AFAICS, this is done by the parser, and again with asserts on nargs.

So I do not think that there is an actual bug, although the why requires
advanced interprocedural analyses.

Maybe the bval access could be rethought, though.

--
Fabien.