Message-ID: <9f23591c-610a-60d4-2b29-26a860f7c3a8@gmail.com>
Date: Tue, 2 May 2023 16:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW
 UPDATE trigger
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Geoghegan <pg@bowt.ie>, pgsql-bugs@lists.postgresql.org,
 Richard Guo <guofenglinux@gmail.com>, Andres Freund <andres@anarazel.de>
References: <17798-0907404928dcf0dd@postgresql.org>
 <87fe6c28-80b2-f709-a8db-d105e5240e39@gmail.com>
 <CAMbWs4_pg7ab=OJF2CGLzs=noWEjujerxfo92sm3RL9ce=ogsQ@mail.gmail.com>
 <CAMbWs48FHhuuuWbxygO4Wey7ahdaSwRjzJODKXx7=GFsge_aLg@mail.gmail.com>
 <f5ff446f-72e2-44f7-80c6-4256cf7caeb3@gmail.com>
 <a4a8cf37-e9d5-3c58-fc2e-d217cf66f315@gmail.com>
 <1849610.1682807087@sss.pgh.pa.us>
 <bc6d1314-4e7a-2bde-1f01-a14de4909efd@gmail.com>
 <CAH2-Wzm3aQznsDrVu10C3hcvez8qwdy2578Hw4BgwXLWZ1NDQw@mail.gmail.com>
 <88b5750f-a5da-798b-e5a1-af2c3ecb9b60@gmail.com>
 <2155320.1682913887@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <2155320.1682913887@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/9f23591c-610a-60d4-2b29-26a860f7c3a8%40gmail.com>
Precedence: bulk

01.05.2023 07:04, Tom Lane wrote:
> Alexander Lakhin <exclusion@gmail.com> writes:
>> So I'm going to start it over and find a repro that will show something
>> interesting without Valgrind (and let estimate the probability of a
>> problem in the field).
> I think a probability estimate is not all that exciting: "it's low
> but not zero" is sufficient information to act on.  What I am curious
> about is how come 86dc90056's changes triggered a visible problem.
> Understanding that might give us a better feel for what is the real
> structural issue here --- which I'm worried about because I have
> little confidence that there aren't similar bugs lurking in other
> code paths.

I've found one significant difference between old and new code paths.
On 86dc90056 I see the following call chain:
ExecGetUpdateNewTuple(planSlot = epqslot_candidate, oldSlot = oldslot):
econtext->ecxt_scantuple = oldSlot;
-> internalGetUpdateNewTuple() -> ExecProject(projInfo=relinfo->ri_projectNew)
-> ExecEvalExprSwitchContext(state=projInfo->pi_state, econtext=projInfo->pi_exprContext)
-> state->evalfunc(state, econtext, isNull);
-> ExecInterpExpr():
scanslot = econtext->ecxt_scantuple;
...
EEOP_SCAN_FETCHSOME: slot_getsomeattrs(scanslot, op->d.fetch.last_var);

So someattrs loaded into oldslot, which is released after
         trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig)

But before 86dc90056 we had:
ExecFilterJunk(junkfilter=relinfo->ri_junkFilter, slot=epqslot_candidate):
-> slot_getallattrs(slot);
-> slot_getsomeattrs(slot, slot->tts_tupleDescriptor->natts);

I. e., someattrs were loaded into the epqslot_candidate slot,
which wasn't touched by ExecFetchSlotHeapTuple(oldslot,...) and
the newslot's contents survived the call.

It's not a final conclusion, but maybe it could be helpful for someone who
investigates it too.

Best regards,
Alexander