I found another bug of this category using Valgrind (memcheck). When
the standard regression tests are run against master's git tip, I see
the following:
2014-03-26 12:58:21.246 PDT 28882 LOG: statement: select * from
json_to_record('{"a":1,"b":"foo","c":"bar"}',true)
as x(a int, b text, d text);
==28882== Conditional jump or move depends on uninitialised value(s)
==28882== at 0x837610: populate_record_worker (jsonfuncs.c:2240)
==28882== by 0x836D42: json_to_record (jsonfuncs.c:2032)
==28882== by 0x650096: ExecMakeTableFunctionResult (execQual.c:2164)
==28882== by 0x6713D9: FunctionNext (nodeFunctionscan.c:94)
==28882== by 0x657799: ExecScanFetch (execScan.c:82)
==28882== by 0x657808: ExecScan (execScan.c:132)
==28882== by 0x67172E: ExecFunctionScan (nodeFunctionscan.c:266)
==28882== by 0x64C314: ExecProcNode (execProcnode.c:426)
==28882== by 0x64A08D: ExecutePlan (execMain.c:1475)
==28882== by 0x6481FC: standard_ExecutorRun (execMain.c:308)
==28882== by 0x648073: ExecutorRun (execMain.c:256)
==28882== by 0x7B837B: PortalRunSelect (pquery.c:946)
==28882== Uninitialised value was created by a heap allocation
==28882== at 0x91CE4B: MemoryContextAlloc (mcxt.c:585)
==28882== by 0x8371F0: populate_record_worker (jsonfuncs.c:2157)
==28882== by 0x836D42: json_to_record (jsonfuncs.c:2032)
==28882== by 0x650096: ExecMakeTableFunctionResult (execQual.c:2164)
==28882== by 0x6713D9: FunctionNext (nodeFunctionscan.c:94)
==28882== by 0x657799: ExecScanFetch (execScan.c:82)
==28882== by 0x657808: ExecScan (execScan.c:132)
==28882== by 0x67172E: ExecFunctionScan (nodeFunctionscan.c:266)
==28882== by 0x64C314: ExecProcNode (execProcnode.c:426)
==28882== by 0x64A08D: ExecutePlan (execMain.c:1475)
==28882== by 0x6481FC: standard_ExecutorRun (execMain.c:308)
==28882== by 0x648073: ExecutorRun (execMain.c:256)
(It's worth noting that I pass --track-origins=yes to Valgrind to get
extra information about where the uninitialized memory came from when
this happens).
It looks to me like there is an oversight within
populate_record_worker() that sees not all codepaths initialize
my_extra's ColumnIOData array. Attached patch has this happen as part
of fcinfo->flinfo->fn_extra initialization, which results in all 4
such instances of this warning not appearing in subsequent runs
(ncolumns is also assigned).
There are still some other warnings not related to json appearing in
this set of results. By volume, they relate primarily to SP-GiST
picksplits, and GinFormTuple(). There was some discussion of at least
the former issue before [1], but I guess no one has since picked it
up.
The other things I see include:
==10833== 7 errors in context 2 of 2:
==10833== Invalid read of size 8
==10833== at 0x4C2E478: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:882)
==10833== by 0x460496: heap_fill_tuple (heaptuple.c:248)
==10833== by 0x4617E4: heap_form_tuple (heaptuple.c:716)
==10833== by 0x6588B2: ExecCopySlotTuple (execTuples.c:573)
==10833== by 0x658BF9: ExecMaterializeSlot (execTuples.c:765)
==10833== by 0x66E91D: ExecInsert (nodeModifyTable.c:179)
==10833== by 0x66FCEF: ExecModifyTable (nodeModifyTable.c:1024)
==10833== by 0x64C242: ExecProcNode (execProcnode.c:377)
==10833== by 0x64A08D: ExecutePlan (execMain.c:1475)
==10833== by 0x6481FC: standard_ExecutorRun (execMain.c:308)
==10833== by 0x648073: ExecutorRun (execMain.c:256)
==10833== by 0x7B710C: ProcessQuery (pquery.c:185)
==10833== Address 0x6ab9028 is 6,344 bytes inside a block of size 8,192 alloc'd
==10833== at 0x4C2C934: malloc (vg_replace_malloc.c:291)
==10833== by 0x91AA46: AllocSetAlloc (aset.c:853)
==10833== by 0x91D2A6: palloc (mcxt.c:657)
==10833== by 0x6841B4: initStringInfo (stringinfo.c:50)
==10833== by 0x7B5FD5: PostgresMain (postgres.c:3914)
==10833== by 0x738E4B: BackendRun (postmaster.c:4089)
==10833== by 0x73858B: BackendStartup (postmaster.c:3778)
==10833== by 0x734D06: ServerLoop (postmaster.c:1569)
==10833== by 0x734369: PostmasterMain (postmaster.c:1222)
==10833== by 0x696333: main (main.c:203)
and:
==10833== 1 errors in context 1 of 2:
==10833== Invalid read of size 8
==10833== at 0x4C2E39E: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:882)
==10833== by 0x7FD1D7: datumCopy (datum.c:132)
==10833== by 0x7146EE: evaluate_expr (clauses.c:4583)
==10833== by 0x713AB3: evaluate_function (clauses.c:4122)
==10833== by 0x712F7D: simplify_function (clauses.c:3761)
==10833== by 0x710A29: eval_const_expressions_mutator (clauses.c:2455)
==10833== by 0x69B734: expression_tree_mutator (nodeFuncs.c:2508)
==10833== by 0x712999: eval_const_expressions_mutator (clauses.c:3414)
==10833== by 0x69B911: expression_tree_mutator (nodeFuncs.c:2557)
==10833== by 0x712999: eval_const_expressions_mutator (clauses.c:3414)
==10833== by 0x7103D7: eval_const_expressions (clauses.c:2297)
==10833== by 0x6F7EC7: preprocess_expression (planner.c:675)
==10833== Address 0xc04ce80 is 80 bytes inside a block of size 8,192 alloc'd
==10833== at 0x4C2C934: malloc (vg_replace_malloc.c:291)
==10833== by 0x91AA46: AllocSetAlloc (aset.c:853)
==10833== by 0x91D2A6: palloc (mcxt.c:657)
==10833== by 0x818449: line_construct_pp (geo_ops.c:1137)
==10833== by 0x64FCCB: ExecMakeFunctionResultNoSets (execQual.c:2001)
==10833== by 0x6506A6: ExecEvalFunc (execQual.c:2385)
==10833== by 0x6546DB: ExecEvalExprSwitchContext (execQual.c:4322)
==10833== by 0x714689: evaluate_expr (clauses.c:4561)
==10833== by 0x713AB3: evaluate_function (clauses.c:4122)
==10833== by 0x712F7D: simplify_function (clauses.c:3761)
==10833== by 0x710A29: eval_const_expressions_mutator (clauses.c:2455)
==10833== by 0x69B734: expression_tree_mutator (nodeFuncs.c:2508)
It would be nice if we could more formally target having zero such
Valgrind memcheck errors...
[1]
http://www.postgresql.org/message-id/1385504078.81471.yahoomail...@web162905.mail.bf1.yahoo.com
--
Peter Geoghegan
*** a/src/backend/utils/adt/jsonfuncs.c
--- b/src/backend/utils/adt/jsonfuncs.c
*************** populate_record_worker(FunctionCallInfo
*** 2160,2165 ****
--- 2160,2167 ----
my_extra = (RecordIOData *) fcinfo->flinfo->fn_extra;
my_extra->record_type = InvalidOid;
my_extra->record_typmod = 0;
+ my_extra->ncolumns = ncolumns;
+ MemSet(my_extra->columns, 0, sizeof(ColumnIOData) * ncolumns);
}
if (have_record_arg && (my_extra->record_type != tupType ||
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
