public inbox for [email protected]  
help / color / mirror / Atom feed
From: Fujii Masao <[email protected]>
To: Peter Eisentraut <[email protected]>
To: Tom Lane <[email protected]>
Cc: [email protected]
Subject: Re: maximum number of backtrace frames logged by backtrace_functions
Date: Sat, 19 Feb 2022 01:05:04 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>



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
diff --git a/src/backend/utils/error/assert.c b/src/backend/utils/error/assert.c
index 2da512a2f1..caebaf23b2 100644
--- a/src/backend/utils/error/assert.c
+++ b/src/backend/utils/error/assert.c
@@ -43,20 +43,30 @@ ExceptionalCondition(const char *conditionName,
 					 errorType, conditionName,
 					 fileName, lineNumber, (int) getpid());
 
-	/* Usually this shouldn't be needed, but make sure the msg went out */
-	fflush(stderr);
-
 	/* If we have support for it, dump a simple backtrace */
 #ifdef HAVE_BACKTRACE_SYMBOLS
 	{
 		void	   *buf[100];
 		int			nframes;
+		char	  **strfrms;
 
 		nframes = backtrace(buf, lengthof(buf));
-		backtrace_symbols_fd(buf, nframes, fileno(stderr));
+		strfrms = backtrace_symbols(buf, nframes);
+		if (strfrms != NULL)
+		{
+			for (int i = 0; i < nframes; i++)
+				write_stderr("%s\n", strfrms[i]);
+			if (nframes >= lengthof(buf))
+				write_stderr("(backtrace limited to %zu frames)\n",
+							 lengthof(buf));
+			free(strfrms);
+		}
 	}
 #endif
 
+	/* Usually this shouldn't be needed, but make sure the msg went out */
+	fflush(stderr);
+
 	/*
 	 * If configured to do so, sleep indefinitely to allow user to attach a
 	 * debugger.  It would be nice to use pg_usleep() here, but that can sleep
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 7402696986..9933386959 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -966,6 +966,9 @@ set_backtrace(ErrorData *edata, int num_skip)
 
 		for (int i = num_skip; i < nframes; i++)
 			appendStringInfo(&errtrace, "\n%s", strfrms[i]);
+		if (nframes >= lengthof(buf))
+			appendStringInfo(&errtrace, "\n(backtrace limited to %zu frames)",
+							 lengthof(buf));
 		free(strfrms);
 	}
 #else


Attachments:

  [text/plain] backtrace_limit_report_v3.patch (1.7K, 2-backtrace_limit_report_v3.patch)
  download | inline diff:
diff --git a/src/backend/utils/error/assert.c b/src/backend/utils/error/assert.c
index 2da512a2f1..caebaf23b2 100644
--- a/src/backend/utils/error/assert.c
+++ b/src/backend/utils/error/assert.c
@@ -43,20 +43,30 @@ ExceptionalCondition(const char *conditionName,
 					 errorType, conditionName,
 					 fileName, lineNumber, (int) getpid());
 
-	/* Usually this shouldn't be needed, but make sure the msg went out */
-	fflush(stderr);
-
 	/* If we have support for it, dump a simple backtrace */
 #ifdef HAVE_BACKTRACE_SYMBOLS
 	{
 		void	   *buf[100];
 		int			nframes;
+		char	  **strfrms;
 
 		nframes = backtrace(buf, lengthof(buf));
-		backtrace_symbols_fd(buf, nframes, fileno(stderr));
+		strfrms = backtrace_symbols(buf, nframes);
+		if (strfrms != NULL)
+		{
+			for (int i = 0; i < nframes; i++)
+				write_stderr("%s\n", strfrms[i]);
+			if (nframes >= lengthof(buf))
+				write_stderr("(backtrace limited to %zu frames)\n",
+							 lengthof(buf));
+			free(strfrms);
+		}
 	}
 #endif
 
+	/* Usually this shouldn't be needed, but make sure the msg went out */
+	fflush(stderr);
+
 	/*
 	 * If configured to do so, sleep indefinitely to allow user to attach a
 	 * debugger.  It would be nice to use pg_usleep() here, but that can sleep
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 7402696986..9933386959 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -966,6 +966,9 @@ set_backtrace(ErrorData *edata, int num_skip)
 
 		for (int i = num_skip; i < nframes; i++)
 			appendStringInfo(&errtrace, "\n%s", strfrms[i]);
+		if (nframes >= lengthof(buf))
+			appendStringInfo(&errtrace, "\n(backtrace limited to %zu frames)",
+							 lengthof(buf));
 		free(strfrms);
 	}
 #else


reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: maximum number of backtrace frames logged by backtrace_functions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox