Content-Type: multipart/mixed; boundary="------------kf13i9NkJIf5k6flk3AvTjgK"
Message-ID: <37001bcc-7341-526c-a94d-26169f6f7282@gmail.com>
Date: Mon, 4 Mar 2024 18: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 #18374: Printing memory contexts on OOM condition might lead
 to segmentation fault
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
References: <18374-ebb8113ce4d02f0d@postgresql.org>
 <3120721.1709395887@sss.pgh.pa.us>
 <b1a1eaf3-d5b7-da52-6bb7-c5b3fbe47f3e@gmail.com>
 <3140126.1709405398@sss.pgh.pa.us> <3148162.1709409519@sss.pgh.pa.us>
 <3399097.1709501969@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <3399097.1709501969@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/37001bcc-7341-526c-a94d-26169f6f7282%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------kf13i9NkJIf5k6flk3AvTjgK
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hello Tom,

04.03.2024 00:39, Tom Lane wrote:
> I wrote:
>> I find this in [1]:
>>
>>    The C language stack growth does an implicit mremap. If you want absolute
>>    guarantees and run close to the edge you MUST mmap your stack for the
>>    largest size you think you will need. For typical stack usage this does
>>    not matter much but it's a corner case if you really really care
>>
>> Seems like we need to do some more work at startup to enforce that
>> we have the amount of stack we think we do, if we're on Linux.
> After thinking about that some more, I'm really quite unenthused about
> trying to remap the stack for ourselves.  It'd be both platform- and
> architecture-dependent, and I'm afraid it'd introduce as many failure
> modes as it removes.  (Notably, I'm not sure we could guarantee
> there's a guard page below the stack.)  Since we've not seen reports
> of this failure from the wild, I doubt it's worth the trouble.

I have perhaps a naive idea, but it apparently eliminates the segmentation
fault for the given test case. (Please look at a quick draft attached.)

Though maybe the issue can really wait for complaints from outside or for
a simpler/cheaper solution, integrated with the existing (or future)
stack-overflow protection.

> I do think it's probably worth reducing MemoryContextDelete's stack
> usage to O(1), just to ensure we can't get into stack trouble during
> transaction abort.  That's not hard at all, as attached.

Yeah, Heikki proposed something similar as part of
0003-Avoid-recursion-in-MemoryContext-functions.patch there:
https://www.postgresql.org/message-id/6b48c746-9704-46dc-b9be-01fe4137c824%40iki.fi

> I tried to make MemoryContextResetChildren work similarly, but that
> doesn't work because if we're not removing child contexts then we
> need extra state to tell which ones we've done already.  For the
> same reason my idea for bounding the stack space needed by
> MemoryContextStats doesn't seem to work.  We could possibly make it
> work if we were willing to add a temporary-use pointer field to all
> MemoryContext headers, but I'm unconvinced that'd be a good tradeoff.

Best regards,
Alexander
--------------kf13i9NkJIf5k6flk3AvTjgK
Content-Type: text/x-patch; charset=UTF-8;
 name="acquire-memory-for-stack.patch"
Content-Disposition: attachment; filename="acquire-memory-for-stack.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3Rjb3AvcG9zdGdyZXMuYyBiL3NyYy9iYWNrZW5k
L3Rjb3AvcG9zdGdyZXMuYwppbmRleCA1OWFiODEyZDJlLi5iYjlkMzA5ZjRlIDEwMDY0NAot
LS0gYS9zcmMvYmFja2VuZC90Y29wL3Bvc3RncmVzLmMKKysrIGIvc3JjL2JhY2tlbmQvdGNv
cC9wb3N0Z3Jlcy5jCkBAIC0zNDY0LDYgKzM0NjQsMTQgQEAgUHJvY2Vzc0ludGVycnVwdHMo
dm9pZCkKIAkJSGFuZGxlUGFyYWxsZWxBcHBseU1lc3NhZ2VzKCk7CiB9CiAKK3ZvaWQgcHJl
cGFyZV9zdGFjayhpbnQgbikKK3sKKwljaGFyIGJ1ZmZlclsxMDI0ICogMTAyNF07CisJbWVt
c2V0KGJ1ZmZlciwgMCwgc2l6ZW9mKGJ1ZmZlcikpOworCWlmIChuID4gMCkKKwkJcHJlcGFy
ZV9zdGFjayhuIC0gMSk7Cit9CisKIC8qCiAgKiBzZXRfc3RhY2tfYmFzZTogc2V0IHVwIHJl
ZmVyZW5jZSBwb2ludCBmb3Igc3RhY2sgZGVwdGggY2hlY2tpbmcKICAqCkBAIC0zNDkwLDYg
KzM0OTgsNyBAQCBzZXRfc3RhY2tfYmFzZSh2b2lkKQogCXN0YWNrX2Jhc2VfcHRyID0gJnN0
YWNrX2Jhc2U7CiAjZW5kaWYKIAorCXByZXBhcmVfc3RhY2soKG1heF9zdGFja19kZXB0aF9i
eXRlcyArIFNUQUNLX0RFUFRIX1NMT1ApIC8gKDEwMjQgKiAxMDI0KSk7CiAJcmV0dXJuIG9s
ZDsKIH0KIAo=

--------------kf13i9NkJIf5k6flk3AvTjgK--