Merge branch 'crashlog_improvements' into jgrpp

config.lib

@@ -97,6 +97,7 @@ set_default() {
 	with_sse="1"
 	with_libbfd="1"
 	with_bfd_extra_debug="1"
+	with_dbg_gdb="1"
 
 	save_params_array="
 		build
@@ -176,6 +177,7 @@ set_default() {
 		with_sse
 		with_libbfd
 		with_bfd_extra_debug
+		with_dbg_gdb
 	CC CXX CFLAGS CXXFLAGS LDFLAGS CFLAGS_BUILD CXXFLAGS_BUILD LDFLAGS_BUILD"
 }
 
@@ -477,6 +479,10 @@ detect_params() {
 			--with-bfd-extra-debug)       with_bfd_extra_debug="1";;
 			--with-bfd-extra-debug=*)     with_bfd_extra_debug="$optarg";;
 
+			--without-self-gdb-debug)     with_dbg_gdb="0";;
+			--with-self-gdb-debug)        with_dbg_gdb="1";;
+			--with-self-gdb-debug=*)      with_dbg_gdb="$optarg";;
+
 			CC=* | --CC=*)                CC="$optarg";;
 			CXX=* | --CXX=*)              CXX="$optarg";;
 			CFLAGS=* | --CFLAGS=*)        CFLAGS="$optarg";;
@@ -1629,6 +1635,7 @@ EOL
 			CFLAGS="$CFLAGS -DWITH_DL"
 		fi
 
+		LIBBFD_LIBS=
 		if [ "$with_libbfd" = "1" ]; then
 			if test_compile_libbfd "-lbfd -lz"; then
 				LIBBFD_LIBS="-lbfd -lz"
@@ -1636,20 +1643,67 @@ EOL
 				LIBBFD_LIBS="-lbfd -liberty -lz"
 			elif test_compile_libbfd "-lbfd -liberty -lintl -lz"; then
 				LIBBFD_LIBS="-lbfd -liberty -lintl -lz"
-			else
-				LIBBFD_LIBS=
 			fi
 			if [ -n "$LIBBFD_LIBS" ]; then
 				log 1 "checking libbfd... found"
 				LIBS="$LIBS $LIBBFD_LIBS"
 				CFLAGS="$CFLAGS -DWITH_BFD"
-				if [ $enable_debug -lt 1 ] && [ "$with_bfd_extra_debug" = "1" ]; then
-					CFLAGS="$CFLAGS -g1"
-				fi
 			else
 				log 1 "checking libbfd... no"
 			fi
 		fi
+
+		HAVE_GDB_DBG=
+		if [ "$with_dbg_gdb" = "1" ]; then
+			log 2 "executing $cc_host $CFLAGS $LDFLAGS $STATIC_FLAGS -o tmp.config.dbggdb -x c++ -"
+			"$cc_host" $CFLAGS $LDFLAGS $STATIC_FLAGS -o tmp.config.dbggdb -x c++ - 2> /dev/null << EOL
+				#include <unistd.h>
+				#include <sys/stat.h>
+				#include <fcntl.h>
+				#include <sys/syscall.h>
+				#include <sys/types.h>
+				#include <sys/wait.h>
+				int main() {
+					pid_t tid = syscall(SYS_gettid);
+					int status;
+					waitpid((pid_t) 0, &status, 0);
+					return WIFEXITED(status) && WEXITSTATUS(status);
+				}
+EOL
+			ret=$?
+			rm -f tmp.config.dbggdb
+			log 2 "  exit code $ret"
+			if [ $ret -ne 0 ]; then
+				log 1 "checking dbg gdb... no"
+			else
+				log 1 "checking dbg gdb... found"
+				CFLAGS="$CFLAGS -DWITH_DBG_GDB"
+				HAVE_GDB_DBG=1
+
+				log 2 "executing $cc_host $CFLAGS $LDFLAGS $STATIC_FLAGS -o tmp.config.dbggdbprctl -x c++ -"
+				"$cc_host" $CFLAGS $LDFLAGS $STATIC_FLAGS -o tmp.config.dbggdbprctl -x c++ - 2> /dev/null << EOL
+					#include <sys/prctl.h>
+					int main() {
+						return prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0);
+					}
+EOL
+				ret=$?
+				rm -f tmp.config.dbggdbprctl
+				log 2 "  exit code $ret"
+				if [ $ret -ne 0 ]; then
+					log 1 "checking dbg gdb (prctl)... no"
+				else
+					log 1 "checking dbg gdb (prctl)... found"
+					CFLAGS="$CFLAGS -DWITH_PRCTL_PT"
+				fi
+			fi
+		fi
+
+		if [ -n "$LIBBFD_LIBS" -o -n "$HAVE_GDB_DBG" ]; then
+			if [ $enable_debug -lt 1 ] && [ "$with_bfd_extra_debug" = "1" ]; then
+				CFLAGS="$CFLAGS -g1"
+			fi
+		fi
 	fi
 
 	if [ "$os" = "MINGW" ]; then
@@ -3710,6 +3764,7 @@ showhelp() {
 	echo "  --without-sse                  disable SSE support (x86/x86_64 only)"
 	echo "  --without-libbfd               disable libbfd support, used for improved crash logs (MinGW and Unix/glibc only)"
 	echo "  --without-bfd-extra-debug      disable extra debugging information when using libbfd (MinGW and Unix/glibc only)"
+	echo "  --without-self-gdb-debug       disable improved crash logs using gdb (Linux only)"
 	echo ""
 	echo "Some influential environment variables:"
 	echo "  CC                             C compiler command"

src/os/unix/crashlog_unix.cpp

@@ -21,6 +21,19 @@
 #include <sys/utsname.h>
 #include <setjmp.h>
 
+#if defined(WITH_DBG_GDB)
+#include <unistd.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#endif /* WITH_DBG_GDB */
+
+#if defined(WITH_PRCTL_PT)
+#include <sys/prctl.h>
+#endif /* WITH_PRCTL_PT */
+
 #if defined(__GLIBC__)
 /* Execinfo (and thus making stacktraces) is a GNU extension */
 #	include <execinfo.h>
@@ -131,6 +144,79 @@ class CrashLogUnix : public CrashLog {
 	}
 #endif
 
+	/**
+	 * Get a stack backtrace of the current thread's stack using the gdb debugger, if available.
+	 *
+	 * Using GDB is useful as it knows about inlined functions and locals, and generally can
+	 * do a more thorough job than in LogStacktrace.
+	 * This is done in addition to LogStacktrace as gdb cannot be assumed to be present
+	 * and there is some potentially useful information in the output from LogStacktrace
+	 * which is not in gdb's output.
+	 */
+	char *LogStacktraceGdb(char *buffer, const char *last) const
+	{
+#if defined(WITH_DBG_GDB)
+
+#if defined(WITH_PRCTL_PT)
+		prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY, 0, 0, 0);
+#endif /* WITH_PRCTL_PT */
+
+		pid_t tid = syscall(SYS_gettid);
+
+		int pipefd[2];
+		if (pipe(pipefd) == -1) return buffer;
+
+		int pid = fork();
+		if (pid < 0) return buffer;
+
+		if (pid == 0) {
+			/* child */
+
+			close(pipefd[0]); /* Close unused read end */
+			dup2(pipefd[1], STDOUT_FILENO);
+			close(pipefd[1]);
+			int null_fd = open("/dev/null", O_RDWR);
+			if (null_fd != -1) {
+				dup2(null_fd, STDERR_FILENO);
+				dup2(null_fd, STDIN_FILENO);
+			}
+			char buffer[16];
+			seprintf(buffer, lastof(buffer), "%d", tid);
+			execlp("gdb", "gdb", "-n", "-p", buffer, "-batch", "-ex", "bt full", NULL);
+			exit(42);
+		}
+
+		/* parent */
+
+		close(pipefd[1]); /* Close unused write end */
+
+		char *buffer_orig = buffer;
+
+		buffer += seprintf(buffer, last, "Stacktrace (GDB):\n");
+		while (buffer < last) {
+			ssize_t res = read(pipefd[0], buffer, last - buffer);
+			if (res < 0) {
+				if (errno == EINTR) continue;
+				break;
+			} else if (res == 0) {
+				break;
+			} else {
+				buffer += res;
+			}
+		}
+		buffer += seprintf(buffer, last, "\n");
+
+		int status;
+		int wait_ret = waitpid(pid, &status, 0);
+		if (wait_ret == -1 || !WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+			/* gdb did not appear to run successfully */
+			buffer = buffer_orig;
+		}
+#endif /* WITH_DBG_GDB */
+
+		return buffer;
+	}
+
 	/**
 	 * Get a stack backtrace of the current thread's stack.
 	 *
@@ -156,6 +242,8 @@ class CrashLogUnix : public CrashLog {
 	 */
 	/* virtual */ char *LogStacktrace(char *buffer, const char *last) const
 	{
+		buffer = LogStacktraceGdb(buffer, last);
+
 		buffer += seprintf(buffer, last, "Stacktrace:\n");
 
 #if defined(__GLIBC__)

src/stdafx.h

@@ -444,6 +444,14 @@ assert_compile(SIZE_MAX >= UINT32_MAX);
 	#define CloseConnection OTTD_CloseConnection
 #endif /* __APPLE__ */
 
+#ifdef __GNUC__
+#define likely(x)       __builtin_expect(!!(x), 1)
+#define unlikely(x)     __builtin_expect(!!(x), 0)
+#else
+#define likely(x)       (x)
+#define unlikely(x)     (x)
+#endif
+
 void NORETURN CDECL usererror(const char *str, ...) WARN_FORMAT(1, 2);
 void NORETURN CDECL error(const char *str, ...) WARN_FORMAT(1, 2);
 void NORETURN CDECL assert_msg_error(int line, const char *file, const char *expr, const char *str, ...) WARN_FORMAT(4, 5);
@@ -455,13 +463,13 @@ void NORETURN CDECL assert_msg_error(int line, const char *file, const char *exp
  */
 #if (defined(_MSC_VER) && defined(NDEBUG) && defined(WITH_ASSERT)) || (!defined(_MSC_VER) && !defined(NDEBUG) && !defined(_DEBUG))
 	#undef assert
-	#define assert(expression) if (!(expression)) error("Assertion failed at line %i of %s: %s", __LINE__, __FILE__, #expression);
+	#define assert(expression) if (unlikely(!(expression))) error("Assertion failed at line %i of %s: %s", __LINE__, __FILE__, #expression);
 #endif
 
 /* Asserts are enabled if NDEBUG isn't defined, or if we are using MSVC and WITH_ASSERT is defined. */
 #if !defined(NDEBUG) || (defined(_MSC_VER) && defined(WITH_ASSERT))
 	#define OTTD_ASSERT
-	#define assert_msg(expression, ...) if (!(expression)) assert_msg_error(__LINE__, __FILE__, #expression, __VA_ARGS__);
+	#define assert_msg(expression, ...) if (unlikely(!(expression))) assert_msg_error(__LINE__, __FILE__, #expression, __VA_ARGS__);
 #else
 	#define assert_msg(expression, ...)
 #endif
@@ -546,14 +554,6 @@ static inline void free(const void *ptr)
 	#define PREFETCH_NTA(address)
 #endif
 
-#ifdef __GNUC__
-#define likely(x)       __builtin_expect(!!(x), 1)
-#define unlikely(x)     __builtin_expect(!!(x), 0)
-#else
-#define likely(x)       (x)
-#define unlikely(x)     (x)
-#endif
-
 #if !defined(DISABLE_SCOPE_INFO) && (__cplusplus >= 201103L || defined(__STDCXX_VERSION__) || defined(__GXX_EXPERIMENTAL_CXX0X__) || defined(__GXX_EXPERIMENTAL_CPP0X__))
 #define USE_SCOPE_INFO
 #endif

src/tile_map.h

@@ -30,7 +30,7 @@
  */
 static inline uint TileHeight(TileIndex tile)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X", tile, MapSize());
 	return _m[tile].height;
 }
 
@@ -48,7 +48,7 @@ uint TileHeightOutsideMap(int x, int y);
  */
 static inline void SetTileHeight(TileIndex tile, uint height)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X", tile, MapSize());
 	assert(height <= MAX_TILE_HEIGHT);
 	_m[tile].height = height;
 }
@@ -88,7 +88,7 @@ static inline uint TilePixelHeightOutsideMap(int x, int y)
  */
 static inline TileType GetTileType(TileIndex tile)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X", tile, MapSize());
 	return (TileType)GB(_m[tile].type, 4, 4);
 }
 
@@ -101,7 +101,7 @@ static inline TileType GetTileType(TileIndex tile)
  */
 static inline bool IsInnerTile(TileIndex tile)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X", tile, MapSize());
 
 	uint x = TileX(tile);
 	uint y = TileY(tile);
@@ -123,11 +123,11 @@ static inline bool IsInnerTile(TileIndex tile)
  */
 static inline void SetTileType(TileIndex tile, TileType type)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X, type: %d", tile, MapSize(), type);
 	/* VOID tiles (and no others) are exactly allowed at the lower left and right
 	 * edges of the map. If _settings_game.construction.freeform_edges is true,
 	 * the upper edges of the map are also VOID tiles. */
-	assert(IsInnerTile(tile) == (type != MP_VOID));
+	assert_msg(IsInnerTile(tile) == (type != MP_VOID), "tile: 0x%X (%d), type: %d", tile, IsInnerTile(tile), type);
 	SB(_m[tile].type, 4, 4, type);
 }
 
@@ -170,9 +170,8 @@ static inline bool IsValidTile(TileIndex tile)
  */
 static inline Owner GetTileOwner(TileIndex tile)
 {
-	assert(IsValidTile(tile));
+	assert_msg(IsValidTile(tile), "tile: 0x%X, size: 0x%X", tile, MapSize());
-	assert(!IsTileType(tile, MP_HOUSE));
+	assert_msg(!IsTileType(tile, MP_HOUSE) && !IsTileType(tile, MP_INDUSTRY), "tile: 0x%X (%d)", tile, GetTileType(tile));
-	assert(!IsTileType(tile, MP_INDUSTRY));
 
 	return (Owner)GB(_m[tile].m1, 0, 5);
 }
@@ -190,9 +189,8 @@ static inline Owner GetTileOwner(TileIndex tile)
  */
 static inline void SetTileOwner(TileIndex tile, Owner owner)
 {
-	assert(IsValidTile(tile));
+	assert_msg(IsValidTile(tile), "tile: 0x%X, size: 0x%X, owner: %d", tile, MapSize(), owner);
-	assert(!IsTileType(tile, MP_HOUSE));
+	assert_msg(!IsTileType(tile, MP_HOUSE) && !IsTileType(tile, MP_INDUSTRY), "tile: 0x%X (%d), owner: %d", tile, GetTileType(tile), owner);
-	assert(!IsTileType(tile, MP_INDUSTRY));
 
 	SB(_m[tile].m1, 0, 5, owner);
 }
@@ -217,8 +215,8 @@ static inline bool IsTileOwner(TileIndex tile, Owner owner)
  */
 static inline void SetTropicZone(TileIndex tile, TropicZone type)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X, type: %d", tile, MapSize(), type);
-	assert(!IsTileType(tile, MP_VOID) || type == TROPICZONE_NORMAL);
+	assert_msg(!IsTileType(tile, MP_VOID) || type == TROPICZONE_NORMAL, "tile: 0x%X (%d), type: %d", tile, GetTileType(tile), type);
 	SB(_m[tile].type, 0, 2, type);
 }
 
@@ -230,19 +228,19 @@ static inline void SetTropicZone(TileIndex tile, TropicZone type)
  */
 static inline TropicZone GetTropicZone(TileIndex tile)
 {
-	assert(tile < MapSize());
+	assert_msg(tile < MapSize(), "tile: 0x%X, size: 0x%X", tile, MapSize());
 	return (TropicZone)GB(_m[tile].type, 0, 2);
 }
 
 /**
  * Get the current animation frame
  * @param t the tile
- * @pre IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) ||IsTileType(t, MP_STATION)
+ * @pre IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) || IsTileType(t, MP_STATION)
  * @return frame number
  */
 static inline byte GetAnimationFrame(TileIndex t)
 {
-	assert(IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) ||IsTileType(t, MP_STATION));
+	assert_msg(IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) || IsTileType(t, MP_STATION), "tile: 0x%X (%d)", t, GetTileType(t));
 	return _me[t].m7;
 }
 
@@ -250,11 +248,11 @@ static inline byte GetAnimationFrame(TileIndex t)
  * Set a new animation frame
  * @param t the tile
  * @param frame the new frame number
- * @pre IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) ||IsTileType(t, MP_STATION)
+ * @pre IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) || IsTileType(t, MP_STATION)
  */
 static inline void SetAnimationFrame(TileIndex t, byte frame)
 {
-	assert(IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) ||IsTileType(t, MP_STATION));
+	assert_msg(IsTileType(t, MP_HOUSE) || IsTileType(t, MP_OBJECT) || IsTileType(t, MP_INDUSTRY) || IsTileType(t, MP_STATION), "tile: 0x%X (%d)", t, GetTileType(t));
 	_me[t].m7 = frame;
 }