This patch adds a backtrace helper.
1) On LASSERTS it will pop-up a dialog notifying the user an assert
has occurred and that a back-trace has been generated. It will then
continue. There were some arguments for aborting LyX, but I think they
were made on the understanding that aborting LyX was the only way to
get a back-trace. Anyway if the new behavior is not desired one can
set the LYX_NO_BACKTRACE_HELPER command-line variable.
2) On SIGSEGV's and SIGABRTS it will print a notification of the
back-trace being generated on the terminal, and popup the backtrace
this is not as pretty as the QEditText proposal, but this is
sufficient for getting the backtrace to the user. This will be
discussed a bit below.

Compatibililty:
   Compilers: This patch only supports GNUC (gcc, mingw etc.). Other
compilers get the old behavior. It should be easy to extend to other
compilers however (1-5 LOC per compiler).
   OS: This patch should support all OS's LyX supports, but I have
only tested Linux. There are some OS specific #ifdef's I have not
tested.

I have noticed a bug where error_handler() is not called if there was
a stack_overflow (where would the local variables  needed by the
handler be allocated?). This means no backtrace and no emergency save.
We could perhaps fix this with "sigaltstack", but this particular
patch does not do so.

On Wed, Mar 10, 2010 at 2:44 AM, Pavel Sanda <sa...@lyx.org> wrote:
> why to care about async-safe code, why are already killed at this point, no?
> pavel

OK, at worst we don't get a backtrace. I am avoiding the use of QtGui
in the signal hander though since that seems a bit much.

This patch does not implement your idea where we pipe data into
separate process with a QTextEdit. However, it does get the job done
of displaying the needed information to the users and I recall that
you wanted to avoid over-engineering. Would you still like this
functionality?
 I note that such a binary would be about  330K (or 27k stripped).
Would you like a separate binary, called say "lyx-backtrace-dialog"
or to instead embed it into LyX so e.g that you do
  " gdb ... | lyx --backtrace-dialog"

-- 
John C. McCabe-Dansted
Index: LyX.cpp
===================================================================
--- LyX.cpp	(revision 33691)
+++ LyX.cpp	(working copy)
@@ -611,14 +611,34 @@
 
 	// This shouldn't matter here, however, as we've already invoked
 	// emergencyCleanup.
+
+	// NOTE: this error handler is *not* called if a SIGSEGV was
+	// caused by a stack overflow. (or at least not on Linux)
+	// We will need to use sigaltstack to fix this. BUG!
+
 	switch (err_sig) {
 #ifdef SIGHUP
 	case SIGHUP:
 		lyxerr << "\nlyx: SIGHUP signal caught\nBye." << endl;
 		break;
 #endif
+/* We do not trap the following errors, maybe we should?
+   These errors generate a core dump, so it would make sense to output a backtrace.
+	case SIGILL:  //Illegal Instruction
+	case SIGQUIT: //User requests core dump
+	case SIGTRAP: //Debugger Trap
+	case SIGSYS:  //Bad syscall argument (might not be implemented)
+	case SIGEMT:  //Emulator trap (might not be implemented)
+	case SIGBUS:  //With these last three errors the core dump might fail
+	case SIGXCPU: //Process exceeded allowed CPU time
+	case SIGXFSZ: //Process attempt to generate too large file */
+	case SIGABRT:
+		lyxerr << "\nlyx: SIGABRT signal caught\nBye." << endl;
+		BACKTRACE_HERE
+		break;
 	case SIGFPE:
 		lyxerr << "\nlyx: SIGFPE signal caught\nBye." << endl;
+		BACKTRACE_HERE
 		break;
 	case SIGSEGV:
 		lyxerr << "\nlyx: SIGSEGV signal caught\n"
@@ -626,6 +646,7 @@
 			  "Please read the bug-reporting instructions "
 			  "in Help->Introduction and send us a bug report, "
 			  "if necessary. Thanks !\nBye." << endl;
+		BACKTRACE_HERE 
 		break;
 	case SIGINT:
 	case SIGTERM:
@@ -637,6 +658,7 @@
 #ifdef SIGHUP
 	signal(SIGHUP, SIG_DFL);
 #endif
+	signal(SIGABRT, SIG_DFL);
 	signal(SIGINT, SIG_DFL);
 	signal(SIGFPE, SIG_DFL);
 	signal(SIGSEGV, SIG_DFL);
@@ -668,6 +690,7 @@
 #ifdef SIGHUP
 	signal(SIGHUP, error_handler);
 #endif
+	signal(SIGABRT, error_handler);
 	signal(SIGFPE, error_handler);
 	signal(SIGSEGV, error_handler);
 	signal(SIGINT, error_handler);
Index: support/lassert.cpp
===================================================================
--- support/lassert.cpp	(revision 33691)
+++ support/lassert.cpp	(working copy)
@@ -14,13 +14,135 @@
 
 #include <boost/assert.hpp>
 
+#include <string>
+
+#if defined(__GNUC__) && !defined(NO_BACKTRACE_HELPER)
+#define BACKTRACE_HELPER
+#endif
+
+#ifdef BACKTRACE_HELPER
+#include "support/docstring.h"
+#include "support/environment.h"
+#include "support/FileName.h"
+#include "support/filetools.h"
+#include "support/lassert.h"
+#include "support/lstrings.h"
+#include "support/lyxtime.h"
+#include "support/os.h"
+#include "support/gettext.h"
+
+#include <boost/regex.hpp>
+#include <QtGui/qmessagebox.h>
+
+#include <cstdlib>
+#include <set>
+#include <sstream>
+
+using namespace lyx;
+using namespace lyx::support;
+#endif
+
+using namespace std;
+
 namespace lyx {
 
+static int backtrace_enabled() {
+	// Some users may not want LyX to generate backtraces for them.
+	// e.g. backtrace.
+	return (getEnv("LYX_NO_BACKTRACE_HELPER").empty());
+}
+
+#ifdef BACKTRACE_HELPER
+time_t last_backtrace_time_ = 0;
+
+static string doBacktrace_internal() {
+	if (backtrace_enabled()) {
+		string backtrace_filename = FileName::tempName("lyx.backtrace.XXXXXXXX.txt").absFilename();
+		std::stringstream gdb_cmd;
+#ifdef __GNUC__
+		gdb_cmd << "python -c  \"print 'set height 0'; print 'bt'; print '\\n'.join(['q' for n in xrange (1, 999999)])\" | gdb -q "
+			<< to_local8bit(from_utf8(os::utf8_argv(0))) << " " << getpid()
+			<< " > " << backtrace_filename;
+#else
+#error "BACKTRACE_HELPER enabled, but compiler not supported in doBacktrace"
+#endif
+		LYXERR0("Generating backtrace\n");
+		LYXERR0(gdb_cmd.str());
+		LYXERR0("GDB RESULT: " << system(gdb_cmd.str().c_str()) << "\n");
+		LYXERR0("Backtrace generated:" << backtrace_filename << "\n");
+		LYXERR0("Please attach the this backtrace to your bugreport\n");
+
+		return backtrace_filename;
+	} else {
+		return "DID NOT GENERATE BACKTRACE";
+	}
+}
+
+// In principle the following function needs to be async-safe
+// but since it is just a debug tool it probably isn't vital.
+// It is only called after LyX has crashed.
+void doBacktrace() {
+    // We do not create a backtrace if a LASSERT was triggered less than 2
+    // seconds ago. It is most likely caused by the same bug and the LASSERT 
+    // backtrace is probably more useful. If we produce a second backtrace
+    // the user may report the wrong one.
+	if (backtrace_enabled() && current_time() > last_backtrace_time_ + 2) {
+		string backtrace_filename = doBacktrace_internal();
+		// now present the backtrace to the user
+		string open_cmd;
+		#ifdef _WIN32
+		open_cmd = "start";
+		#elif defined(USE_MACOSX_PACKAGING)
+		open_cmd = "open";
+		#else
+		open_cmd = "xdg-open";
+		#endif
+		system((open_cmd + " \"" + backtrace_filename + "\"").c_str());
+	}
+}
+#endif
+	
+
 void doAssert(char const * expr,  char const * file, long line)
 {
-	LYXERR0("ASSERTION " << expr << " VIOLATED IN " << file << ":" << line);
+	stringstream assert_description;
+	assert_description << "ASSERTION " << expr << " VIOLATED IN " << file << ":" << line;
+
+	string assert_str = assert_description.str();
+
+	LYXERR0(assert_str);
 	// comment this out if not needed
+	// BOOST_ASSERT(false);
+
+#ifndef BACKTRACE_HELPER
 	BOOST_ASSERT(false);
+#else
+	// We don't report the same assertion multiple times, because LyX sometimes
+	// produces the same assertion hundreds of times in a row.
+	// We could also just use a timer (only report asserts every N seconds) but
+    // we probably don't need more than one backtrace of the same bug.
+	static set<string> reported_assertions;
+	if (reported_assertions.find(assert_str) == reported_assertions.end() && backtrace_enabled() ) {
+		string backtrace_filename = doBacktrace_internal();
+
+		const docstring str = bformat(_("LyX has triggered the assertion\n\n"
+			"%1$s\n\n"
+			"Sorry, you have found a bug in LyX. \n"
+			"Please read the bug-reporting instructions \n"
+			"in Help->Introduction and send us a bug report, \n"
+			"and attach the backtrace file\n\n%2$s\n\n"
+			"Thanks!"),
+				from_utf8(assert_description.str()),
+				from_utf8(backtrace_filename));
+
+		QMessageBox::critical(0,
+			QString(to_utf8(_("Assertion Triggered")).c_str()),
+			QString(to_utf8(str).c_str()) );
+
+		reported_assertions.insert(assert_str);
+		last_backtrace_time_ = current_time();
+	}
+#endif 
 }
 
 } // namespace lyx
Index: support/lassert.h
===================================================================
--- support/lassert.h	(revision 33691)
+++ support/lassert.h	(working copy)
@@ -14,10 +14,20 @@
 #define LASSERT_H
 
 #ifdef __cplusplus
+
+#if defined(__GNUC__) && !defined(NO_BACKTRACE_HELPER)
+#define BACKTRACE_HELPER
+#define BACKTRACE_HERE lyx::doBacktrace();
+#else
+#define BACKTRACE_HERE
+#endif
+
 namespace lyx {
 
 void doAssert(char const * expr, char const * file, long line);
 
+void doBacktrace();
+
 } // namespace lyx
 
 #define LASSERT(expr, escape) \

Reply via email to