From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: obartunov@gmail.com
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: jsonpath
Date: Tue, 6 Mar 2018 02:48:38 +0300
Message-ID: <fcc5050c-a4d1-530d-8ea6-ddaa85e72b57@postgrespro.ru> (raw)
In-Reply-To: <698a11a8-2ed6-7477-2fa0-9a7c262e6c3c@2ndquadrant.com>
References: <fcc6fc6a-b497-f39a-923d-aa34d0c588e8@2ndQuadrant.com>
<CAFj8pRAhPnD=sPu-E=rwfcbgeZ8PLczYEzCsvgNYLtybSy9-=g@mail.gmail.com>
<9f845926-0d7d-7c4d-75e3-b1e921a1677a@2ndQuadrant.com>
<CAFj8pRBCvhgLCROfSCPGmwWZpSY+dKQN4O9xRe+1-wjBiXiHkA@mail.gmail.com>
<bad98003-31ba-523e-d76c-3679efb2e732@2ndQuadrant.com>
<CAFj8pRDWSCWd1JxA8srwbfL5cPgz2b5minq2UsM=cxgsFF-orw@mail.gmail.com>
<48f13b75-0be7-c356-ff26-1db743add56e@postgrespro.ru>
<7b4b8973-43f5-71c5-bf86-51cb5798dd88@2ndQuadrant.com>
<f0b44fe7-d47a-eff2-592c-d9dbe4a63f75@2ndQuadrant.com>
<5290ddd1-f23e-a53a-cae0-830dbde0a5fe@2ndQuadrant.com>
<604ffe9c-b42e-7c42-e16b-aa44c3a12c3c@postgrespro.ru>
<927f72f0-6832-a4ca-5676-b0ab5a2895d4@postgrespro.ru>
<CAF4Au4wxHuEeZrMxxKXUYcs1kDws-f0oW0-sEyun388-U-NOxw@mail.gmail.com>
<9a1ac993-7608-797c-2095-c735605da82f@postgrespro.ru>
<698a11a8-2ed6-7477-2fa0-9a7c262e6c3c@2ndquadrant.com>
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
Attachments:
[application/gzip] sqljson_jsonpath_v11.tar.gz (78.6K, ../fcc5050c-a4d1-530d-8ea6-ddaa85e72b57@postgrespro.ru/2-sqljson_jsonpath_v11.tar.gz)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: n.gluhov@postgrespro.ru, tomas.vondra@2ndquadrant.com, obartunov@gmail.com, andrew.dunstan@2ndquadrant.com, pavel.stehule@gmail.com
Subject: Re: jsonpath
In-Reply-To: <fcc5050c-a4d1-530d-8ea6-ddaa85e72b57@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox