Re: jsonpath - Mailing list pgsql-hackers

On 06.03.2018 00:29, Tomas Vondra wrote:

> Hi,
>
> The patch no longer applies - it got broken by fd1a421fe66 which changed
> columns in pg_proc. A rebase is needed.
>
> Fixing it is pretty simle, so I've done that locally and tried to run
> 'make check' under valgrind. And I got a bunch of reports about
> uninitialised values.

Thank you very much for your discovery. It is my fault that I never ran
PostgreSQL under valgrind. Rebased and fixed patches are attached.

>   Full report attached, but in general there seem to
> be two types of failures:
>
>    Conditional jump or move depends on uninitialised value(s)
>       at 0x57BB47A: vfprintf (in /usr/lib64/libc-2.24.so)
>       by 0x57E5478: vsnprintf (in /usr/lib64/libc-2.24.so)
>       by 0xA926D3: pvsnprintf (psprintf.c:121)
>       by 0x723E03: appendStringInfoVA (stringinfo.c:130)
>       by 0x723D58: appendStringInfo (stringinfo.c:87)
>       by 0x76BEFF: _outCoerceViaIO (outfuncs.c:1413)
>       by 0x776F99: outNode (outfuncs.c:3978)
>       by 0x76D7E7: _outJsonCoercion (outfuncs.c:1779)
>       by 0x777CB9: outNode (outfuncs.c:4398)
>       by 0x76D507: _outJsonExpr (outfuncs.c:1752)
>       by 0x777CA1: outNode (outfuncs.c:4395)
>       by 0x767000: _outList (outfuncs.c:187)
>       by 0x776874: outNode (outfuncs.c:3753)
>       by 0x76A4D2: _outTableFunc (outfuncs.c:1068)
>       by 0x776D89: outNode (outfuncs.c:3912)
>       by 0x7744FD: _outRangeTblEntry (outfuncs.c:3209)
>       by 0x777959: outNode (outfuncs.c:4290)
>       by 0x767000: _outList (outfuncs.c:187)
>       by 0x776874: outNode (outfuncs.c:3753)
>       by 0x773713: _outQuery (outfuncs.c:3049)
>     Uninitialised value was created by a stack allocation
>       at 0x5B0C19: base_yyparse (gram.c:26287)
>
> This happens when _outCoerceViaIO tries to output 'location' field
> (that's line 1413), so I guess it's not set/copied somewhere.

Yes, JSON FORMAT location was not set in gram.y.

> The second failure looks like this:
>
>    Conditional jump or move depends on uninitialised value(s)
>       at 0x49E58B: ginFillScanEntry (ginscan.c:72)
>       by 0x49EB56: ginFillScanKey (ginscan.c:221)
>       by 0x49EF72: ginNewScanKey (ginscan.c:369)
>       by 0x4A3875: gingetbitmap (ginget.c:1807)
>       by 0x4F620B: index_getbitmap (indexam.c:727)
>       by 0x6EE342: MultiExecBitmapIndexScan (nodeBitmapIndexscan.c:104)
>       by 0x6DA8F8: MultiExecProcNode (execProcnode.c:506)
>       by 0x6EC53D: BitmapHeapNext (nodeBitmapHeapscan.c:118)
>       by 0x6DC26D: ExecScanFetch (execScan.c:95)
>       by 0x6DC308: ExecScan (execScan.c:162)
>       by 0x6ED7E5: ExecBitmapHeapScan (nodeBitmapHeapscan.c:730)
>       by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
>       by 0x6E5961: ExecProcNode (executor.h:239)
>       by 0x6E5E25: fetch_input_tuple (nodeAgg.c:406)
>       by 0x6E8091: agg_retrieve_direct (nodeAgg.c:1736)
>       by 0x6E7C84: ExecAgg (nodeAgg.c:1551)
>       by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
>       by 0x6D1361: ExecProcNode (executor.h:239)
>       by 0x6D3BB7: ExecutePlan (execMain.c:1721)
>       by 0x6D1917: standard_ExecutorRun (execMain.c:361)
>     Uninitialised value was created by a heap allocation
>       at 0xA64FDC: palloc (mcxt.c:858)
>       by 0x938636: gin_extract_jsonpath_query (jsonb_gin.c:630)
>       by 0x938AB6: gin_extract_jsonb_query (jsonb_gin.c:746)
>       by 0xA340C0: FunctionCall7Coll (fmgr.c:1201)
>       by 0x49EE7F: ginNewScanKey (ginscan.c:313)
>       by 0x4A3875: gingetbitmap (ginget.c:1807)
>       by 0x4F620B: index_getbitmap (indexam.c:727)
>       by 0x6EE342: MultiExecBitmapIndexScan (nodeBitmapIndexscan.c:104)
>       by 0x6DA8F8: MultiExecProcNode (execProcnode.c:506)
>       by 0x6EC53D: BitmapHeapNext (nodeBitmapHeapscan.c:118)
>       by 0x6DC26D: ExecScanFetch (execScan.c:95)
>       by 0x6DC308: ExecScan (execScan.c:162)
>       by 0x6ED7E5: ExecBitmapHeapScan (nodeBitmapHeapscan.c:730)
>       by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
>       by 0x6E5961: ExecProcNode (executor.h:239)
>       by 0x6E5E25: fetch_input_tuple (nodeAgg.c:406)
>       by 0x6E8091: agg_retrieve_direct (nodeAgg.c:1736)
>       by 0x6E7C84: ExecAgg (nodeAgg.c:1551)
>       by 0x6DA80A: ExecProcNodeFirst (execProcnode.c:446)
>       by 0x6D1361: ExecProcNode (executor.h:239)
>
> So the extra_data allocated in gin_extract_jsonpath_query() get to
> ginFillScanEntry() uninitialised.

Yes, only the first element of extra_data[] was initialized and used later in
gin_consistent_jsonb(), but ginFillScanEntry() wants all of them to be
initialized.

> Both seem like a valid issues, I think.
>
> regards

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company