*_to_xml() should copy SPI_processed/SPI_tuptable

From: Chapman Flack <chap(at)anastigmatix(dot)net>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: *_to_xml() should copy SPI_processed/SPI_tuptable
Date: 2018-09-05 21:09:59
Message-ID: 9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I encountered this in 9.5 and haven't yet written a reproducer
for 10 or 11, but the code in question looks the same in master.

In xml.c, query_to_xml_internal() contains a loop that refers
to SPI_processed every iteration:

for (i = 0; i < SPI_processed; i++)
SPI_sql_row_to_xmlelement(i, result, tablename, nulls,
tableforest, targetns, top_level);

likewise, that function it is calling contains a loop that
repeatedly dereferences SPI_tuptable:

for (i = 1; i <= SPI_tuptable->tupdesc->natts; i++)
...
... map_sql_value_to_xml_value(colval,
SPI_gettypeid(SPI_tuptable->tupdesc, ...

and map_sql_value_to_xml_value can use OidOutputFunctionCall. If that
attribute's data type has an output function that uses SPI, the next
iteration here dereferences a null SPI_tuptable.

The root of the problem does not seem to be an output function that
uses SPI; indeed, the chance that one might do so seems to have been
the original motivation for OutputFunctionCall().

The essence of the problem here seems to be simply that these routines
in xml.c are not making local-variable copies of SPI_processed and
SPI_tuptable, as the docs for SPI_execute recommend.

Would it be worth also expanding that note in the docs? It currently
says:

---
All SPI query-execution functions set both SPI_processed and
SPI_tuptable .... Save these two global variables into local procedure
variables if you need to access the result table of SPI_execute or
another query-execution function across later calls.
---

but might it bear emphasis that those later calls can happen
indirectly from things that you call (e.g. output functions)?
Those will be nested in their own SPI_connect/SPI_finish, which
a reasonable person might guess would save/restore those globals,
but they don't; SPI_finish merely clears them, on the assumption
that you've got your local copies if you need them.

-Chap

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2018-09-05 21:18:31 Re: Collation versioning
Previous Message Alexander Korotkov 2018-09-05 20:54:54 Re: Bug in ginRedoRecompress that causes opaque data on page to be overrun