Date: Wed, 2 Mar 2022 15:47:04 +0900
From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
        Tom Lane
 <tgl@sss.pgh.pa.us>, pgsql-docs@lists.postgresql.org
Subject: Re: maximum number of backtrace frames logged by
 backtrace_functions
Message-Id: <20220302154704.516286896b6f8ca924b2e384@sraoss.co.jp>
In-Reply-To: <070467b6-1ead-8921-89a1-5ec39d1f2103@oss.nttdata.com>
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>
	<070467b6-1ead-8921-89a1-5ec39d1f2103@oss.nttdata.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/20220302154704.516286896b6f8ca924b2e384%40sraoss.co.jp>
Precedence: bulk

On Sat, 19 Feb 2022 01:05:04 +0900
Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

> 
> 
> 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);

I think it would be better that users can get the same results in
different OS as as possible.

I have another comment. When the backtrace is output from elog, 
two rows are skipped by num_skip and only 98 frames will appear
even though it is said "backtrace limited to 100 frames". So, how
about reporting the limiting number subtracted by num_skip, like this:

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

 or

	if (nframes >= BACKTRACE_MAX_FRAMES)
			appendStringInfo(&errtrace, "\n(backtrace limited to %d frames)",
							 BACKTRACE_MAX_FRAMES - num_skip);

Regards,
Yugo Nagata

-- 
Yugo NAGATA <nagata@sraoss.co.jp>