[PATCH i-g-t] lib/igt_core: Print stacktrace when receiving one of the crash signals.

[PATCH i-g-t] lib/igt_core: Print stacktrace when receiving one of the crash signals.

[Marius Vlad]

While at it add SIGFPE as a crash signal.

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 801f02f..67e1a4f 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1482,7 +1482,8 @@ static bool exit_handler_disabled;
 #define SILENT(x) { x, NULL, 0 }
 static const struct { int number; const char *name; size_t name_len; } handled_signals[] =
 	{ SILENT(SIGINT), SILENT(SIGHUP), SILENT(SIGTERM), SILENT(SIGQUIT),
-	  SILENT(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) };
+	  SILENT(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS),
+	  SIGDEF(SIGFPE) };
 #undef SILENT
 #undef SIGDEF
 
@@ -1542,6 +1543,7 @@ static bool crash_signal(int sig)
 	switch (sig) {
 	case SIGILL:
 	case SIGBUS:
+	case SIGFPE:
 	case SIGSEGV:
 		return true;
 	default:
@@ -1571,7 +1573,7 @@ static void fatal_sig_handler(int sig)
 				igt_exitcode = 128 + sig;
 
 			failed_one = true;
-
+			print_backtrace();
 			exit_subtest("CRASH");
 		}
 		break;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] lib/igt_core: Print stacktrace when receiving one of the crash signals.

[Chris Wilson]

On Sat, Aug 20, 2016 at 06:32:36PM +0300, Marius Vlad wrote:
> While at it add SIGFPE as a crash signal.
> 
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>

Seems reasonable. Though did you check that print_backtrace() was
sigsafe? It doesn't use stdio, malloc, or any of the usual suspects?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v2] lib/igt_core: Print stacktrace when receiving one of the crash signals.

[Marius Vlad]

While at it add SIGFPE as a crash signal.

v2: Added some helpers to avoid printf() inside a signal handler.
(Chris Wilson)

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
CC: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/igt_core.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 163 insertions(+), 2 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 801f02f..472d01a 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1098,6 +1098,165 @@ static void print_backtrace(void)
 		       (unsigned int) off);
 	}
 }
+
+static const char hex[] = "0123456789abcdef";
+typedef void (*print_func)(int ch);
+
+static void
+xputch(int c)
+{
+	igt_assert_eq(write(STDERR_FILENO, (const void *) &c, 1), 1);
+}
+
+static void
+printnum(print_func putch, unsigned long long num, unsigned base)
+{
+	/* recursively print all preceding  digits */
+	if (num >= base) {
+		printnum(putch, num / base, base);
+	}
+
+	/* print the least significant digit */
+	putch(hex[num % base]);
+}
+
+static size_t
+xstrlcpy(char *dst, const char *src, size_t size)
+{
+	char *dst_in;
+
+	dst_in = dst;
+	if (size > 0) {
+		while (--size > 0 && *src != '\0')
+			*dst++ = *src++;
+		*dst = '\0';
+	}
+
+	return dst - dst_in;
+}
+
+static void
+xprintfmt(print_func putch, const char *fmt, va_list ap)
+{
+	const char *p;
+	int ch, base;
+	unsigned long long num;
+
+	while (1) {
+		while ((ch = *(unsigned char *) fmt++) != '%') {
+			if (ch == '\0') {
+				return;
+			}
+			putch(ch);
+		}
+
+		ch = *(unsigned char *) fmt++;
+		switch (ch) {
+		/* character */
+		case 'c':
+			putch(va_arg(ap, int));
+			break;
+		/* string */
+		case 's':
+			if ((p = va_arg(ap, char *)) == NULL) {
+				p = "(null)";
+			}
+
+			for (; (ch = *p++) != '\0';) {
+				if (ch < ' ' || ch > '~') {
+					putch('?');
+				} else {
+					putch(ch);
+				}
+			}
+			break;
+		/* (signed) decimal */
+		case 'd':
+			num = va_arg(ap, int);
+			if ((long long) num < 0) {
+				putch('-');
+				num = -(long long) num;
+			}
+			base = 10;
+			goto number;
+		/* unsigned decimal */
+		case 'u':
+			num = va_arg(ap, unsigned int);
+			base = 10;
+			goto number;
+		/* (unsigned) hexadecimal */
+		case 'x':
+			num = va_arg(ap, unsigned int);
+			base = 16;
+number:
+			printnum(putch, num, base);
+			break;
+
+		/* The following are not implemented */
+
+		/* width field */
+		case '1': case '2':
+		case '3': case '4':
+		case '5': case '6':
+		case '7': case '8':
+		case '9':
+		case '.': case '#':
+		/* long */
+		case 'l':
+		/* octal */
+		case 'o':
+		/* pointer */
+		case 'p':
+		/* float */
+		case 'f':
+			abort();
+		/* escaped '%' character */
+		case '%':
+			putch(ch);
+			break;
+		/* unrecognized escape sequence - just print it literally */
+		default:
+			putch('%');
+			for (fmt--; fmt[-1] != '%'; fmt--)
+				; /* do nothing */
+			break;
+		}
+	}
+}
+
+/* async-safe printf */
+static void
+xprintf(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	xprintfmt((void *) xputch, fmt, ap);
+	va_end(ap);
+}
+
+static void print_backtrace_sig_safe(void)
+{
+	unw_cursor_t cursor;
+	unw_context_t uc;
+	int stack_num = 0;
+
+	igt_assert_eq(write(STDERR_FILENO, "Stack trace: \n", 15), 15);
+
+	unw_getcontext(&uc);
+	unw_init_local(&cursor, &uc);
+	while (unw_step(&cursor) > 0) {
+		char name[255];
+		unw_word_t off;
+
+		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
+			xstrlcpy(name, "<unknown>", 9);
+
+		xprintf(" #%d [%s+0x%x]\n", stack_num++, name,
+				(unsigned int) off);
+
+	}
+}
 #endif
 
 void __igt_fail_assert(const char *domain, const char *file, const int line,
@@ -1482,7 +1641,8 @@ static bool exit_handler_disabled;
 #define SILENT(x) { x, NULL, 0 }
 static const struct { int number; const char *name; size_t name_len; } handled_signals[] =
 	{ SILENT(SIGINT), SILENT(SIGHUP), SILENT(SIGTERM), SILENT(SIGQUIT),
-	  SILENT(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) };
+	  SILENT(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS),
+	  SIGDEF(SIGFPE) };
 #undef SILENT
 #undef SIGDEF
 
@@ -1542,6 +1702,7 @@ static bool crash_signal(int sig)
 	switch (sig) {
 	case SIGILL:
 	case SIGBUS:
+	case SIGFPE:
 	case SIGSEGV:
 		return true;
 	default:
@@ -1571,7 +1732,7 @@ static void fatal_sig_handler(int sig)
 				igt_exitcode = 128 + sig;
 
 			failed_one = true;
-
+			print_backtrace_sig_safe();
 			exit_subtest("CRASH");
 		}
 		break;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v2] lib/igt_core: Print stacktrace when receiving one of the crash signals.

[Chris Wilson]

On Tue, Aug 23, 2016 at 11:38:43AM +0300, Marius Vlad wrote:
> While at it add SIGFPE as a crash signal.
> 
> v2: Added some helpers to avoid printf() inside a signal handler.
> (Chris Wilson)
> 
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  lib/igt_core.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 163 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 801f02f..472d01a 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1098,6 +1098,165 @@ static void print_backtrace(void)
>  		       (unsigned int) off);
>  	}
>  }
> +
> +static const char hex[] = "0123456789abcdef";
> +typedef void (*print_func)(int ch);
> +
> +static void
> +xputch(int c)
> +{
> +	igt_assert_eq(write(STDERR_FILENO, (const void *) &c, 1), 1);

I thought you said you were avoiding !sigsafe functions....

igt_assert_eq() is unsafe. Just ignore the warn.

> +static void print_backtrace_sig_safe(void)
> +{
> +	unw_cursor_t cursor;
> +	unw_context_t uc;
> +	int stack_num = 0;
> +
> +	igt_assert_eq(write(STDERR_FILENO, "Stack trace: \n", 15), 15);

Will someone please turn off this compiler warning, it has resulted in
more broken code than fixes.

Other than recursively generating signals and calling !sigsafe functions
because of igt_assert_eq(), looks good. Though there is only one user of
xprintffmt, so you can simplify the chain a bit by removing the wrapper
and function pointer. At this point, we should be splitting off into
igt_signal.c
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v3] lib/igt_core: Print stacktrace when receiving one of the crash signals.

[Marius Vlad]

While at it add SIGFPE as a crash signal.

v3: Remove calls to igt_assert_eq() as these are not async-safe. As one
user of this method remove the function pointer and recursive call.
(Chris Wilson)
v2: Added some helpers to avoid printf() inside a signal handler.
(Chris Wilson)
---
 lib/igt_core.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 177 insertions(+), 2 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 801f02f..7e5bf76 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1098,6 +1098,179 @@ static void print_backtrace(void)
 		       (unsigned int) off);
 	}
 }
+
+static const char hex[] = "0123456789abcdef";
+
+static void
+xputch(int c)
+{
+	write(STDERR_FILENO, (const void *) &c, 1);
+}
+
+static int
+xpow(int base, int pow)
+{
+	int i, r = 1;
+
+	for (i = 0; i < pow; i++)
+		r *= base;
+
+	return r;
+}
+
+static void
+printnum(unsigned long long num, unsigned base)
+{
+	int i = 0;
+	unsigned long long __num;
+
+	/* determine from where we should start dividing */
+	do {
+		__num /= base;
+		i++;
+	} while (__num);
+
+	while (i--)
+		xputch(hex[num / xpow(base, i) % base]);
+}
+
+static size_t
+xstrlcpy(char *dst, const char *src, size_t size)
+{
+	char *dst_in;
+
+	dst_in = dst;
+	if (size > 0) {
+		while (--size > 0 && *src != '\0')
+			*dst++ = *src++;
+		*dst = '\0';
+	}
+
+	return dst - dst_in;
+}
+
+static void
+xprintfmt(const char *fmt, va_list ap)
+{
+	const char *p;
+	int ch, base;
+	unsigned long long num;
+
+	while (1) {
+		while ((ch = *(unsigned char *) fmt++) != '%') {
+			if (ch == '\0') {
+				return;
+			}
+			xputch(ch);
+		}
+
+		ch = *(unsigned char *) fmt++;
+		switch (ch) {
+		/* character */
+		case 'c':
+			xputch(va_arg(ap, int));
+			break;
+		/* string */
+		case 's':
+			if ((p = va_arg(ap, char *)) == NULL) {
+				p = "(null)";
+			}
+
+			for (; (ch = *p++) != '\0';) {
+				if (ch < ' ' || ch > '~') {
+					xputch('?');
+				} else {
+					xputch(ch);
+				}
+			}
+			break;
+		/* (signed) decimal */
+		case 'd':
+			num = va_arg(ap, int);
+			if ((long long) num < 0) {
+				xputch('-');
+				num = -(long long) num;
+			}
+			base = 10;
+			goto number;
+		/* unsigned decimal */
+		case 'u':
+			num = va_arg(ap, unsigned int);
+			base = 10;
+			goto number;
+		/* (unsigned) hexadecimal */
+		case 'x':
+			num = va_arg(ap, unsigned int);
+			base = 16;
+number:
+			printnum(num, base);
+			break;
+
+		/* The following are not implemented */
+
+		/* width field */
+		case '1': case '2':
+		case '3': case '4':
+		case '5': case '6':
+		case '7': case '8':
+		case '9':
+		case '.': case '#':
+		/* long */
+		case 'l':
+		/* octal */
+		case 'o':
+		/* pointer */
+		case 'p':
+		/* float */
+		case 'f':
+			abort();
+		/* escaped '%' character */
+		case '%':
+			xputch(ch);
+			break;
+		/* unrecognized escape sequence - just print it literally */
+		default:
+			xputch('%');
+			for (fmt--; fmt[-1] != '%'; fmt--)
+				; /* do nothing */
+			break;
+		}
+	}
+}
+
+/* async-safe printf */
+static void
+xprintf(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	xprintfmt(fmt, ap);
+	va_end(ap);
+}
+
+static void print_backtrace_sig_safe(void)
+{
+	unw_cursor_t cursor;
+	unw_context_t uc;
+	int stack_num = 0;
+
+	write(STDERR_FILENO, "Stack trace: \n", 15);
+
+	unw_getcontext(&uc);
+	unw_init_local(&cursor, &uc);
+	while (unw_step(&cursor) > 0) {
+		char name[255];
+		unw_word_t off;
+
+		if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
+			xstrlcpy(name, "<unknown>", 9);
+
+		xprintf(" #%d [%s+0x%x]\n", stack_num++, name,
+				(unsigned int) off);
+
+	}
+}
 #endif
 
 void __igt_fail_assert(const char *domain, const char *file, const int line,
@@ -1482,7 +1655,8 @@ static bool exit_handler_disabled;
 #define SILENT(x) { x, NULL, 0 }
 static const struct { int number; const char *name; size_t name_len; } handled_signals[] =
 	{ SILENT(SIGINT), SILENT(SIGHUP), SILENT(SIGTERM), SILENT(SIGQUIT),
-	  SILENT(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS) };
+	  SILENT(SIGPIPE), SIGDEF(SIGABRT), SIGDEF(SIGSEGV), SIGDEF(SIGBUS),
+	  SIGDEF(SIGFPE) };
 #undef SILENT
 #undef SIGDEF
 
@@ -1542,6 +1716,7 @@ static bool crash_signal(int sig)
 	switch (sig) {
 	case SIGILL:
 	case SIGBUS:
+	case SIGFPE:
 	case SIGSEGV:
 		return true;
 	default:
@@ -1571,7 +1746,7 @@ static void fatal_sig_handler(int sig)
 				igt_exitcode = 128 + sig;
 
 			failed_one = true;
-
+			print_backtrace_sig_safe();
 			exit_subtest("CRASH");
 		}
 		break;
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx