Thread: "Conditional jump or move depends on uninitialised value(s)" within tsginidx.c

I see this when I run the regression tests with valgrind (memcheck) on
master's tip:

2014-03-25 22:38:40.850 PDT 31570 LOG:  statement: SET enable_seqscan=OFF;
2014-03-25 22:38:40.859 PDT 31570 LOG:  statement: SELECT count(*)
FROM test_tsvector WHERE a @@ 'wr|qh';
==31570== Conditional jump or move depends on uninitialised value(s)
==31570==    at 0x8A58F4: gin_tsquery_triconsistent (tsginidx.c:329)
==31570==    by 0x8F8659: FunctionCall7Coll (fmgr.c:1471)
==31570==    by 0x488F20: directTriConsistentFn (ginlogic.c:97)
==31570==    by 0x480026: keyGetItem (ginget.c:1094)
==31570==    by 0x480191: scanGetItem (ginget.c:1182)
==31570==    by 0x481A11: gingetbitmap (ginget.c:1791)
==31570==    by 0x8F7F7E: FunctionCall2Coll (fmgr.c:1324)
==31570==    by 0x4C8406: index_getbitmap (indexam.c:651)
==31570==    by 0x663A46: MultiExecBitmapIndexScan (nodeBitmapIndexscan.c:89)
==31570==    by 0x64C516: MultiExecProcNode (execProcnode.c:550)
==31570==    by 0x662944: BitmapHeapNext (nodeBitmapHeapscan.c:104)
==31570==    by 0x657739: ExecScanFetch (execScan.c:82)
==31570==  Uninitialised value was created by a stack allocation
==31570==    at 0x8A585B: gin_tsquery_triconsistent (tsginidx.c:299)

It looks like a "recheck" stack variable isn't every being set within
TS_execute_ternary() (which has a pointer to that variable on the
stack) - ultimately, the checkcondition_gin() callback will set the
flag, but only to 'true' (iff that's appropriate). When that doesn't
happen, it just contains a garbage uninitialized value, and yet
evidently control flow depends on that value.

I propose that we initialize the variable to false, since there
appears to be a tacit assumption that that is already the case, as
with the plain consistent GIN support function in the same file.
Attached patch does just that.

--
Peter Geoghegan

On 03/26/2014 09:21 AM, Peter Geoghegan wrote:
> It looks like a "recheck" stack variable isn't every being set within
> TS_execute_ternary() (which has a pointer to that variable on the
> stack) - ultimately, the checkcondition_gin() callback will set the
> flag, but only to 'true' (iff that's appropriate). When that doesn't
> happen, it just contains a garbage uninitialized value, and yet
> evidently control flow depends on that value.
>
> I propose that we initialize the variable to false, since there
> appears to be a tacit assumption that that is already the case, as
> with the plain consistent GIN support function in the same file.
> Attached patch does just that.

Yep, fixed. Thanks!

- Heikki