Content-Type: multipart/mixed; boundary="------------mwpBN4U1ulyMaQCuFTJl9DcR"
Message-ID: <070467b6-1ead-8921-89a1-5ec39d1f2103@oss.nttdata.com>
Date: Sat, 19 Feb 2022 01:05:04 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.0
Subject: Re: maximum number of backtrace frames logged by backtrace_functions
Content-Language: en-US
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
 Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-docs@lists.postgresql.org
References: <0f0ed9f3-3892-e8a3-51c9-ed268dff6bdd@oss.nttdata.com>
 <4e16a3e9-e717-05e1-d905-6c21beba80f8@enterprisedb.com>
 <b22cd3c0-d76b-0379-5971-8637b65b8e87@oss.nttdata.com>
 <b219d7c9-fb66-c762-ce90-ef20c0667a02@enterprisedb.com>
 <252159.1643899696@sss.pgh.pa.us>
 <e312116f-9c44-b7f6-3cfc-4c67f50bbbc7@oss.nttdata.com>
 <33675df8-60e1-7da6-8995-3743668fd682@oss.nttdata.com>
 <b56ce3bf-d308-8dcf-7eaf-e1d3acd021a1@enterprisedb.com>
 <e4ea7dc1-2925-21e1-27b1-44e47b2122bb@oss.nttdata.com>
 <9b3e1b97-4d2f-25af-fa52-1b2c31511444@enterprisedb.com>
 <7fce4874-1433-42e2-6649-e2c57ce50d4e@oss.nttdata.com>
 <5c089e04-1f73-f9f1-b60d-d71d6c33e3f2@enterprisedb.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
In-Reply-To: <5c089e04-1f73-f9f1-b60d-d71d6c33e3f2@enterprisedb.com>
Archived-At: 
 <https://www.postgresql.org/message-id/070467b6-1ead-8921-89a1-5ec39d1f2103%40oss.nttdata.com>
Precedence: bulk

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


On 2022/02/18 19:59, Peter Eisentraut wrote:
> On 18.02.22 09:24, Fujii Masao wrote:
>> Or even backtrace should be logged by write_stderr() so that it's written to eventlog if necessary? I just wonder why backtrace_symbols_fd() is used only in ExceptionalCondition().
> 
> Probably because it was simpler.  It would also make sense to convert the whole thing to use write_stderr() consistently.
+1
Attached is the updated version of the patch that uses write_stderr() to log the backtrace in assertion failure case.

+		if (nframes >= lengthof(buf))
+			appendStringInfo(&errtrace, "\n(backtrace limited to %zu frames)",
+							 lengthof(buf));

I found this doesn't work on FreeBSD, at least FreeBSD 13 that cfbot uses on Cirrus CI. When the backtrace is larger than 100, on FreeBSD, backtrace() seems to write the *99* (not 100) most recent function calls to the buffer. That is, the variable "nframes" is 99 while lengthof(buf) indicates 100. So the above message about backtrace limit will never be logged on FreeBSD. OTOH, on Linux and MacOS, backtrace() writes the 100 most recent function calls. I'm not sure if such a behavior on FreeBSD is expected or a bug.

To issue the message whatever OS is, probably we need to modify set_backtrace() as follows, for example. But is this overkill? Thought?


-		void	   *buf[100];
+#define	BACKTRACE_MAX_FRAMES	100
+		void	   *buf[BACKTRACE_MAX_FRAMES + 1];
  		int			nframes;
  		char	  **strfrms;
  
  		nframes = backtrace(buf, lengthof(buf));
+		if (nframes > BACKTRACE_MAX_FRAMES)
+			nframes = BACKTRACE_MAX_FRAMES;
  		strfrms = backtrace_symbols(buf, nframes);
  		if (strfrms == NULL)
  			return;
  
  		for (int i = num_skip; i < nframes; i++)
  			appendStringInfo(&errtrace, "\n%s", strfrms[i]);
+		if (nframes >= BACKTRACE_MAX_FRAMES)
+			appendStringInfo(&errtrace, "\n(backtrace limited to %d frames)",
+							 BACKTRACE_MAX_FRAMES);

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
--------------mwpBN4U1ulyMaQCuFTJl9DcR
Content-Type: text/plain; charset=UTF-8;
 name="backtrace_limit_report_v3.patch"
Content-Disposition: attachment; filename="backtrace_limit_report_v3.patch"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2Vycm9yL2Fzc2VydC5jIGIvc3JjL2Jh
Y2tlbmQvdXRpbHMvZXJyb3IvYXNzZXJ0LmMKaW5kZXggMmRhNTEyYTJmMS4uY2FlYmFmMjNi
MiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRpbHMvZXJyb3IvYXNzZXJ0LmMKKysrIGIv
c3JjL2JhY2tlbmQvdXRpbHMvZXJyb3IvYXNzZXJ0LmMKQEAgLTQzLDIwICs0MywzMCBAQCBF
eGNlcHRpb25hbENvbmRpdGlvbihjb25zdCBjaGFyICpjb25kaXRpb25OYW1lLAogCQkJCQkg
ZXJyb3JUeXBlLCBjb25kaXRpb25OYW1lLAogCQkJCQkgZmlsZU5hbWUsIGxpbmVOdW1iZXIs
IChpbnQpIGdldHBpZCgpKTsKIAotCS8qIFVzdWFsbHkgdGhpcyBzaG91bGRuJ3QgYmUgbmVl
ZGVkLCBidXQgbWFrZSBzdXJlIHRoZSBtc2cgd2VudCBvdXQgKi8KLQlmZmx1c2goc3RkZXJy
KTsKLQogCS8qIElmIHdlIGhhdmUgc3VwcG9ydCBmb3IgaXQsIGR1bXAgYSBzaW1wbGUgYmFj
a3RyYWNlICovCiAjaWZkZWYgSEFWRV9CQUNLVFJBQ0VfU1lNQk9MUwogCXsKIAkJdm9pZAkg
ICAqYnVmWzEwMF07CiAJCWludAkJCW5mcmFtZXM7CisJCWNoYXIJICAqKnN0cmZybXM7CiAK
IAkJbmZyYW1lcyA9IGJhY2t0cmFjZShidWYsIGxlbmd0aG9mKGJ1ZikpOwotCQliYWNrdHJh
Y2Vfc3ltYm9sc19mZChidWYsIG5mcmFtZXMsIGZpbGVubyhzdGRlcnIpKTsKKwkJc3RyZnJt
cyA9IGJhY2t0cmFjZV9zeW1ib2xzKGJ1ZiwgbmZyYW1lcyk7CisJCWlmIChzdHJmcm1zICE9
IE5VTEwpCisJCXsKKwkJCWZvciAoaW50IGkgPSAwOyBpIDwgbmZyYW1lczsgaSsrKQorCQkJ
CXdyaXRlX3N0ZGVycigiJXNcbiIsIHN0cmZybXNbaV0pOworCQkJaWYgKG5mcmFtZXMgPj0g
bGVuZ3Rob2YoYnVmKSkKKwkJCQl3cml0ZV9zdGRlcnIoIihiYWNrdHJhY2UgbGltaXRlZCB0
byAlenUgZnJhbWVzKVxuIiwKKwkJCQkJCQkgbGVuZ3Rob2YoYnVmKSk7CisJCQlmcmVlKHN0
cmZybXMpOworCQl9CiAJfQogI2VuZGlmCiAKKwkvKiBVc3VhbGx5IHRoaXMgc2hvdWxkbid0
IGJlIG5lZWRlZCwgYnV0IG1ha2Ugc3VyZSB0aGUgbXNnIHdlbnQgb3V0ICovCisJZmZsdXNo
KHN0ZGVycik7CisKIAkvKgogCSAqIElmIGNvbmZpZ3VyZWQgdG8gZG8gc28sIHNsZWVwIGlu
ZGVmaW5pdGVseSB0byBhbGxvdyB1c2VyIHRvIGF0dGFjaCBhCiAJICogZGVidWdnZXIuICBJ
dCB3b3VsZCBiZSBuaWNlIHRvIHVzZSBwZ191c2xlZXAoKSBoZXJlLCBidXQgdGhhdCBjYW4g
c2xlZXAKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2Vycm9yL2Vsb2cuYyBiL3Ny
Yy9iYWNrZW5kL3V0aWxzL2Vycm9yL2Vsb2cuYwppbmRleCA3NDAyNjk2OTg2Li45OTMzMzg2
OTU5IDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC91dGlscy9lcnJvci9lbG9nLmMKKysrIGIv
c3JjL2JhY2tlbmQvdXRpbHMvZXJyb3IvZWxvZy5jCkBAIC05NjYsNiArOTY2LDkgQEAgc2V0
X2JhY2t0cmFjZShFcnJvckRhdGEgKmVkYXRhLCBpbnQgbnVtX3NraXApCiAKIAkJZm9yIChp
bnQgaSA9IG51bV9za2lwOyBpIDwgbmZyYW1lczsgaSsrKQogCQkJYXBwZW5kU3RyaW5nSW5m
bygmZXJydHJhY2UsICJcbiVzIiwgc3RyZnJtc1tpXSk7CisJCWlmIChuZnJhbWVzID49IGxl
bmd0aG9mKGJ1ZikpCisJCQlhcHBlbmRTdHJpbmdJbmZvKCZlcnJ0cmFjZSwgIlxuKGJhY2t0
cmFjZSBsaW1pdGVkIHRvICV6dSBmcmFtZXMpIiwKKwkJCQkJCQkgbGVuZ3Rob2YoYnVmKSk7
CiAJCWZyZWUoc3RyZnJtcyk7CiAJfQogI2Vsc2UK

--------------mwpBN4U1ulyMaQCuFTJl9DcR--