On Sat, 19 Feb 2022 01:05:04 +0900
Fujii Masao <masao.fu...@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 <nag...@sraoss.co.jp>


Reply via email to