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

From: Peter Geoghegan <pg(at)heroku(dot)com>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: "Conditional jump or move depends on uninitialised value(s)" within tsginidx.c
Date: 2014-03-26 07:21:04
Message-ID: CAM3SWZS5LPs7bftkq+nSYK84-WKQokiRcqgun1M5yvdB+V06KA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
uninitialized_recheck.patch text/x-patch 673 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2014-03-26 08:10:05 Re: Still something fishy in the fastpath lock stuff
Previous Message Michael Paquier 2014-03-26 06:39:47 New parameter RollbackError to control rollback behavior on error