[kvm-unit-tests 0/5] Debugging aids

[kvm-unit-tests 0/5] Debugging aids

[Peter Feiner]

Some aids for debugging: stack dumping, register dumping, and printing
failing assertion conditions.

N.B. I haven't tried compiling on arm or ppc.

Peter Feiner (5):
  lib: print failing assert cond
  lib: backtrace printing
  x86: lib: debug dump on unhandled exceptions
  lib: dump stack on abort()
  scripts: pretty print stack traces

 Makefile                       |  2 +-
 arm/Makefile.common            |  1 +
 lib/arm/dump_stack.c           |  6 ++++
 lib/libcflat.h                 | 11 +++++--
 lib/powerpc/dump_stack.c       |  6 ++++
 lib/printf.c                   | 37 ++++++++++++++++++++++
 lib/x86/desc.c                 | 69 ++++++++++++++++++++++++++++++++++++++----
 lib/x86/dump_stack.c           | 24 +++++++++++++++
 powerpc/Makefile.common        |  1 +
 run_tests.sh                   | 10 ++++--
 scripts/pretty_print_stacks.py | 61 +++++++++++++++++++++++++++++++++++++
 x86/Makefile.common            |  1 +
 12 files changed, 218 insertions(+), 11 deletions(-)
 create mode 100644 lib/arm/dump_stack.c
 create mode 100644 lib/powerpc/dump_stack.c
 create mode 100644 lib/x86/dump_stack.c
 create mode 100755 scripts/pretty_print_stacks.py

-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests 1/5] lib: print failing assert cond

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 39100f2..1f0049c 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -74,7 +74,8 @@ extern int report_summary(void);
 #define assert(cond)							\
 do {									\
 	if (!(cond))							\
-		printf("%s:%d: assert failed\n", __FILE__, __LINE__),	\
+		printf("%s:%d: assert failed: %s\n",			\
+		       __FILE__, __LINE__, #cond),			\
 		abort();						\
 } while (0)
 
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests 1/5] lib: print failing assert cond

[Andrew Jones]

On Tue, Mar 01, 2016 at 01:27:34PM -0800, Peter Feiner wrote:
> Signed-off-by: Peter Feiner <pfeiner@google.com>

Reviewed-by: Andrew Jones <drjones@redhat.com>

> ---
>  lib/libcflat.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 39100f2..1f0049c 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -74,7 +74,8 @@ extern int report_summary(void);
>  #define assert(cond)							\
>  do {									\
>  	if (!(cond))							\
> -		printf("%s:%d: assert failed\n", __FILE__, __LINE__),	\
> +		printf("%s:%d: assert failed: %s\n",			\
> +		       __FILE__, __LINE__, #cond),			\
>  		abort();						\
>  } while (0)
>  
> -- 
> 2.7.0.rc3.207.g0ac5344
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests 2/5] lib: backtrace printing

[Peter Feiner]

Functions to walk stack and print backtrace. The stack's unadorned as

	STACK: addr addr addr ...

A follow-up patch post-processes the output to pretty-print the stack.

Stack walker is just a stub on arm and ppc.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 Makefile                 |  2 +-
 arm/Makefile.common      |  1 +
 lib/arm/dump_stack.c     |  6 ++++++
 lib/libcflat.h           |  4 ++++
 lib/powerpc/dump_stack.c |  6 ++++++
 lib/printf.c             | 37 +++++++++++++++++++++++++++++++++++++
 lib/x86/dump_stack.c     | 24 ++++++++++++++++++++++++
 powerpc/Makefile.common  |  1 +
 x86/Makefile.common      |  1 +
 9 files changed, 81 insertions(+), 1 deletion(-)
 create mode 100644 lib/arm/dump_stack.c
 create mode 100644 lib/powerpc/dump_stack.c
 create mode 100644 lib/x86/dump_stack.c

diff --git a/Makefile b/Makefile
index ddba941..0ffa5e7 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,7 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
 
 CFLAGS += -g
 CFLAGS += $(autodepend-flags) -Wall
-CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
+CFLAGS += $(call cc-option, -fno-omit-frame-pointer, "")
 CFLAGS += $(call cc-option, -fno-stack-protector, "")
 CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
 
diff --git a/arm/Makefile.common b/arm/Makefile.common
index dd3a0ca..054bdee 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
 cflatobjs += lib/arm/bitops.o
 cflatobjs += lib/arm/psci.o
 cflatobjs += lib/arm/smp.o
+cflatobjs += lib/arm/dump_stack.o
 
 libeabi = lib/arm/libeabi.a
 eabiobjs = lib/arm/eabi_compat.o
diff --git a/lib/arm/dump_stack.c b/lib/arm/dump_stack.c
new file mode 100644
index 0000000..528ba63
--- /dev/null
+++ b/lib/arm/dump_stack.c
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	return 0;
+}
diff --git a/lib/libcflat.h b/lib/libcflat.h
index 1f0049c..42c94df 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -65,6 +65,10 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth);
+void dump_stack(unsigned long ip, unsigned long bp);
+void dump_current_stack(void);
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\
diff --git a/lib/powerpc/dump_stack.c b/lib/powerpc/dump_stack.c
new file mode 100644
index 0000000..528ba63
--- /dev/null
+++ b/lib/powerpc/dump_stack.c
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	return 0;
+}
diff --git a/lib/printf.c b/lib/printf.c
index 2aec59a..e97fca9 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -259,3 +259,40 @@ int printf(const char *fmt, ...)
     puts(buf);
     return r;
 }
+
+static void print_stack(unsigned long *stack, int depth,
+			bool top_is_return_address)
+{
+	int i;
+
+	printf("\tSTACK: " );
+	for (i = 0; i < depth; i++) {
+		int offset = -1;
+		if (i == 0 && !top_is_return_address)
+			offset = 0;
+		printf(" %lx", stack[i] + offset);
+	}
+	printf("\n");
+}
+
+#define MAX_DEPTH 10
+
+void dump_stack(unsigned long ip, unsigned long bp)
+{
+	unsigned long stack[MAX_DEPTH];
+	int depth;
+
+	stack[0] = ip;
+	depth = walk_stack(bp, &stack[1], MAX_DEPTH - 1);
+	print_stack(stack, depth + 1, false);
+}
+
+void dump_current_stack(void)
+{
+	unsigned long stack[MAX_DEPTH];
+	int depth;
+
+	depth = walk_stack((unsigned long)__builtin_frame_address(1), stack,
+			   MAX_DEPTH);
+	print_stack(stack, depth, true);
+}
diff --git a/lib/x86/dump_stack.c b/lib/x86/dump_stack.c
new file mode 100644
index 0000000..6e9d126
--- /dev/null
+++ b/lib/x86/dump_stack.c
@@ -0,0 +1,24 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	unsigned long *frame = (unsigned long *) bp;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	for (depth = 0; depth < max_depth; depth++) {
+		stack[depth] = frame[1];
+		if (stack[depth] == 0)
+			break;
+		frame = (unsigned long *) frame[0];
+	}
+
+	walking = 0;
+	return depth;
+}
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index cc27ac8..97f5ddc 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -30,6 +30,7 @@ cflatobjs += lib/powerpc/io.o
 cflatobjs += lib/powerpc/hcall.o
 cflatobjs += lib/powerpc/setup.o
 cflatobjs += lib/powerpc/rtas.o
+cflatobjs += lib/powerpc/dump_stack.o
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: LDFLAGS = $(arch_LDFLAGS) -nostdlib -pie
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 3a14fea..250d95d 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
 cflatobjs += lib/x86/desc.o
 cflatobjs += lib/x86/isr.o
 cflatobjs += lib/x86/acpi.o
+cflatobjs += lib/x86/dump_stack.o
 
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I lib
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests 2/5] lib: backtrace printing

[Peter Feiner]

On Tue, Mar 1, 2016 at 1:27 PM, Peter Feiner <pfeiner@google.com> wrote:
> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
> +CFLAGS += $(call cc-option, -fno-omit-frame-pointer, "")

Please hold off on this series for now. -fno-omit-frame-pointer seems
to break x86/emulator.flat. I'll look into it and reply when I've
figured things out.

Re: [kvm-unit-tests 2/5] lib: backtrace printing

[Peter Feiner]

On Tue, Mar 1, 2016 at 2:58 PM, Peter Feiner <pfeiner@google.com> wrote:
> On Tue, Mar 1, 2016 at 1:27 PM, Peter Feiner <pfeiner@google.com> wrote:
>> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
>> +CFLAGS += $(call cc-option, -fno-omit-frame-pointer, "")
>
> Please hold off on this series for now. -fno-omit-frame-pointer seems
> to break x86/emulator.flat. I'll look into it and reply when I've
> figured things out.

Apparently x86/emulator.flat is very fickle:

   -O0 causes #PF
   -fno-omit-frame-pointer causes triple fault
   -fno-omit-frame-pointer and -O0 causes #GP

Oye!

[kvm-unit-tests 3/5] x86: lib: debug dump on unhandled exceptions

[Peter Feiner]

Prints the stack trace and registers.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/x86/desc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 1a80c01..59c34b7 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -38,6 +38,67 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
+static const char* exception_mnemonic(int vector)
+{
+	switch(vector) {
+	case 0: return "#DE";
+	case 1: return "#DB";
+	case 2: return "#NMI";
+	case 3: return "#BP";
+	case 4: return "#OF";
+	case 5: return "#BR";
+	case 6: return "#UD";
+	case 7: return "#NM";
+	case 8: return "#DF";
+	case 10: return "#TS";
+	case 11: return "#NP";
+	case 12: return "#SS";
+	case 13: return "#GP";
+	case 14: return "#PF";
+	case 16: return "#MF";
+	case 17: return "#AC";
+	case 18: return "#MC";
+	case 19: return "#XM";
+	default: return "#??";
+	}
+}
+
+static void unhandled_exception(struct ex_regs *regs, bool cpu)
+{
+	printf("Unhandled %sexception %ld %s at ip %016lx\n",
+	       cpu ? "cpu " : "", regs->vector,
+	       exception_mnemonic(regs->vector), regs->rip);
+	if (regs->vector == 14)
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
+
+	printf("error_code=%04lx      rflags=%08lx      cs=%08lx\n"
+	       "rax=%016lx rcx=%016lx rdx=%016lx rbx=%016lx\n"
+	       "rbp=%016lx rsi=%016lx rdi=%016lx\n"
+#ifdef __x86_64__
+	       " r8=%016lx  r9=%016lx r10=%016lx r11=%016lx\n"
+	       "r12=%016lx r13=%016lx r14=%016lx r15=%016lx\n"
+#endif
+	       "cr0=%016lx cr2=%016lx cr3=%016lx cr4=%016lx\n"
+#ifdef __x86_64__
+	       "cr8=%016lx\n"
+#endif
+	       ,
+	       regs->error_code, regs->rflags, regs->cs,
+	       regs->rax, regs->rcx, regs->rdx, regs->rbx,
+	       regs->rbp, regs->rsi, regs->rdi,
+#ifdef __x86_64__
+	       regs->r8, regs->r9, regs->r10, regs->r11,
+	       regs->r12, regs->r13, regs->r14, regs->r15,
+#endif
+	       read_cr0(), read_cr2(), read_cr3(), read_cr4()
+#ifdef __x86_64__
+	       , read_cr8()
+#endif
+	);
+	dump_stack(regs->rip, regs->rbp);
+	exit(7);
+}
+
 static void check_exception_table(struct ex_regs *regs)
 {
     struct ex_record *ex;
@@ -53,8 +114,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %d\n", regs->vector);
-    exit(7);
+    unhandled_exception(regs, false);
 }
 
 static void (*exception_handlers[32])(struct ex_regs *regs);
@@ -75,10 +135,7 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %d\n", regs->vector);
-	if (regs->vector == 14)
-		printf("PF at %p addr %p\n", regs->rip, read_cr2());
-	exit(7);
+	unhandled_exception(regs, true);
 }
 
 #define EX(NAME, N) extern char NAME##_fault;	\
-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests 4/5] lib: dump stack on abort()

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 42c94df..1798716 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -77,10 +77,12 @@ void dump_current_stack(void);
 
 #define assert(cond)							\
 do {									\
-	if (!(cond))							\
+	if (!(cond)) {							\
 		printf("%s:%d: assert failed: %s\n",			\
-		       __FILE__, __LINE__, #cond),			\
+		       __FILE__, __LINE__, #cond);			\
+		dump_current_stack();					\
 		abort();						\
+	}								\
 } while (0)
 
 #endif
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests 4/5] lib: dump stack on abort()

[Peter Feiner]

subject should be "[kvm-unit-tests 4/5] lib: dump stack on failed assert()"

On Tue, Mar 1, 2016 at 1:27 PM, Peter Feiner <pfeiner@google.com> wrote:
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  lib/libcflat.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 42c94df..1798716 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -77,10 +77,12 @@ void dump_current_stack(void);
>
>  #define assert(cond)                                                   \
>  do {                                                                   \
> -       if (!(cond))                                                    \
> +       if (!(cond)) {                                                  \
>                 printf("%s:%d: assert failed: %s\n",                    \
> -                      __FILE__, __LINE__, #cond),                      \
> +                      __FILE__, __LINE__, #cond);                      \
> +               dump_current_stack();                                   \
>                 abort();                                                \
> +       }                                                               \
>  } while (0)
>
>  #endif
> --
> 2.7.0.rc3.207.g0ac5344
>

[kvm-unit-tests 5/5] scripts: pretty print stack traces

[Peter Feiner]

Optionally pretty print stacks in test.log with 'run_tests.sh -p'.

The script runs addresses through addr2line to get file names and line
numbers.  Also prints out source code lines if files are available.
For example

0x004013f5: ac_test_exec at x86/access.c:818
            }
      >     ac_test_setup_pte(at, pool);
            r = ac_test_do_access(at);
0x004014dd: ac_test_run at x86/access.c:852
                ++tests;
      >         successes += ac_test_exec(&at, &pool);
            } while (ac_test_bump(&at));
0x0040157d: main at x86/access.c:873
            printf("starting test\n\n");
      >     r = ac_test_run();
            return r ? 0 : 1;
0x0040028e: start64 at x86/cstart64.S:206
                lea __argv(%rip), %rsi
      >         call main
                mov %eax, %edi

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 run_tests.sh                   | 10 +++++--
 scripts/pretty_print_stacks.py | 61 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100755 scripts/pretty_print_stacks.py

diff --git a/run_tests.sh b/run_tests.sh
index 2c8af36..13bc089 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 
 verbose="no"
+pretty="no"
 
 if [ ! -f config.mak ]; then
     echo "run ./configure && make first. See ./configure -h"
@@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
     -g: Only execute tests in the given group
     -h: Output this help text
     -v: Enables verbose mode
+    -p: Pretty print stack traces in test.log
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
 specify the appropriate qemu binary for ARCH-run.
@@ -27,8 +29,9 @@ EOF
 
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
+log_redir=">> test.log"
 
-while getopts "g:hv" opt; do
+while getopts "g:hvp" opt; do
     case $opt in
         g)
             only_group=$OPTARG
@@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
         v)
             verbose="yes"
             ;;
+        p)
+           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
+           ;;
         *)
             exit
             ;;
     esac
 done
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 echo > test.log
diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
new file mode 100755
index 0000000..49564bd
--- /dev/null
+++ b/scripts/pretty_print_stacks.py
@@ -0,0 +1,61 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+import sys
+
+if len(sys.argv) != 2:
+    sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
+    sys.exit(1)
+
+binary = sys.argv[1]
+
+# Subvert output buffering.
+def puts(string):
+    sys.stdout.write(string)
+    sys.stdout.flush()
+
+try:
+    while True:
+        # Subvert input buffering.
+        line = sys.stdin.readline()
+        if line == '':
+            break
+
+        if not line.strip().startswith('STACK:'):
+            puts(line)
+            continue
+
+        addrs = line.split()[1:]
+        # Output like this:
+        #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
+        #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
+        cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+        cmd.extend(addrs)
+
+        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        out, err = p.communicate()
+        if p.returncode != 0:
+            puts(line)
+            continue
+
+        for line in out.splitlines():
+            m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
+            if m is None:
+                puts('%s\n' % line)
+                continue
+
+            head, path, line, tail = m.groups()
+            line = int(line)
+            puts('%s at %s:%d%s\n' % (head, path, line, tail))
+            try:
+                lines = open(path).readlines()
+            except IOError:
+                continue
+            if line > 1:
+                puts('        %s\n' % lines[line - 2].rstrip())
+            puts('      > %s\n' % lines[line - 1].rstrip())
+            if line < len(lines):
+                puts('        %s\n' % lines[line].rstrip())
+except:
+    sys.exit(1)
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests 5/5] scripts: pretty print stack traces

[Paolo Bonzini]

On 01/03/2016 22:27, Peter Feiner wrote:
> Optionally pretty print stacks in test.log with 'run_tests.sh -p'.
> 
> The script runs addresses through addr2line to get file names and line
> numbers.  Also prints out source code lines if files are available.
> For example
> 
> 0x004013f5: ac_test_exec at x86/access.c:818
>             }
>       >     ac_test_setup_pte(at, pool);
>             r = ac_test_do_access(at);
> 0x004014dd: ac_test_run at x86/access.c:852
>                 ++tests;
>       >         successes += ac_test_exec(&at, &pool);
>             } while (ac_test_bump(&at));
> 0x0040157d: main at x86/access.c:873
>             printf("starting test\n\n");
>       >     r = ac_test_run();
>             return r ? 0 : 1;
> 0x0040028e: start64 at x86/cstart64.S:206
>                 lea __argv(%rip), %rsi
>       >         call main
>                 mov %eax, %edi
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  run_tests.sh                   | 10 +++++--
>  scripts/pretty_print_stacks.py | 61 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 2 deletions(-)
>  create mode 100755 scripts/pretty_print_stacks.py
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 2c8af36..13bc089 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -1,6 +1,7 @@
>  #!/bin/bash
>  
>  verbose="no"
> +pretty="no"
>  
>  if [ ! -f config.mak ]; then
>      echo "run ./configure && make first. See ./configure -h"
> @@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
>      -g: Only execute tests in the given group
>      -h: Output this help text
>      -v: Enables verbose mode
> +    -p: Pretty print stack traces in test.log
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>  specify the appropriate qemu binary for ARCH-run.
> @@ -27,8 +29,9 @@ EOF
>  
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
> +log_redir=">> test.log"
>  
> -while getopts "g:hv" opt; do
> +while getopts "g:hvp" opt; do
>      case $opt in
>          g)
>              only_group=$OPTARG
> @@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
>          v)
>              verbose="yes"
>              ;;
> +        p)
> +           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> +           ;;
>          *)
>              exit
>              ;;
>      esac
>  done
>  
> -RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> +RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
>  config=$TEST_DIR/unittests.cfg
>  rm -f test.log
>  echo > test.log
> diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
> new file mode 100755
> index 0000000..49564bd
> --- /dev/null
> +++ b/scripts/pretty_print_stacks.py
> @@ -0,0 +1,61 @@
> +#!/usr/bin/env python
> +
> +import re
> +import subprocess
> +import sys
> +
> +if len(sys.argv) != 2:
> +    sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
> +    sys.exit(1)
> +
> +binary = sys.argv[1]
> +
> +# Subvert output buffering.
> +def puts(string):
> +    sys.stdout.write(string)
> +    sys.stdout.flush()
> +
> +try:
> +    while True:
> +        # Subvert input buffering.
> +        line = sys.stdin.readline()
> +        if line == '':
> +            break
> +
> +        if not line.strip().startswith('STACK:'):
> +            puts(line)
> +            continue
> +
> +        addrs = line.split()[1:]
> +        # Output like this:
> +        #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
> +        #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
> +        cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
> +        cmd.extend(addrs)
> +
> +        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> +        out, err = p.communicate()
> +        if p.returncode != 0:
> +            puts(line)
> +            continue
> +
> +        for line in out.splitlines():
> +            m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
> +            if m is None:
> +                puts('%s\n' % line)
> +                continue
> +
> +            head, path, line, tail = m.groups()
> +            line = int(line)
> +            puts('%s at %s:%d%s\n' % (head, path, line, tail))
> +            try:
> +                lines = open(path).readlines()
> +            except IOError:
> +                continue
> +            if line > 1:
> +                puts('        %s\n' % lines[line - 2].rstrip())
> +            puts('      > %s\n' % lines[line - 1].rstrip())
> +            if line < len(lines):
> +                puts('        %s\n' % lines[line].rstrip())
> +except:
> +    sys.exit(1)
> 

For now I've committed only the script; I'd like Drew to take a look at
run_tests.sh and see if it's possible/worthwhile to include the pretty
printing script in mkstandalone.sh.  But even without integration in
run_tests.sh the scripts is nice and useful.

Queued, will push tomorrow together with the rest.

Paolo

Re: [kvm-unit-tests 5/5] scripts: pretty print stack traces

[Andrew Jones]

On Tue, Mar 01, 2016 at 10:34:53PM +0100, Paolo Bonzini wrote:
> 
> 
> On 01/03/2016 22:27, Peter Feiner wrote:
> > Optionally pretty print stacks in test.log with 'run_tests.sh -p'.
> > 
> > The script runs addresses through addr2line to get file names and line
> > numbers.  Also prints out source code lines if files are available.
> > For example
> > 
> > 0x004013f5: ac_test_exec at x86/access.c:818
> >             }
> >       >     ac_test_setup_pte(at, pool);
> >             r = ac_test_do_access(at);
> > 0x004014dd: ac_test_run at x86/access.c:852
> >                 ++tests;
> >       >         successes += ac_test_exec(&at, &pool);
> >             } while (ac_test_bump(&at));
> > 0x0040157d: main at x86/access.c:873
> >             printf("starting test\n\n");
> >       >     r = ac_test_run();
> >             return r ? 0 : 1;
> > 0x0040028e: start64 at x86/cstart64.S:206
> >                 lea __argv(%rip), %rsi
> >       >         call main
> >                 mov %eax, %edi
> > 
> > Signed-off-by: Peter Feiner <pfeiner@google.com>
> > ---
> >  run_tests.sh                   | 10 +++++--
> >  scripts/pretty_print_stacks.py | 61 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 69 insertions(+), 2 deletions(-)
> >  create mode 100755 scripts/pretty_print_stacks.py
> > 
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 2c8af36..13bc089 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -1,6 +1,7 @@
> >  #!/bin/bash
> >  
> >  verbose="no"
> > +pretty="no"
> >  
> >  if [ ! -f config.mak ]; then
> >      echo "run ./configure && make first. See ./configure -h"
> > @@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
> >      -g: Only execute tests in the given group
> >      -h: Output this help text
> >      -v: Enables verbose mode
> > +    -p: Pretty print stack traces in test.log
> >  
> >  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> >  specify the appropriate qemu binary for ARCH-run.
> > @@ -27,8 +29,9 @@ EOF
> >  
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> > +log_redir=">> test.log"
> >  
> > -while getopts "g:hv" opt; do
> > +while getopts "g:hvp" opt; do
> >      case $opt in
> >          g)
> >              only_group=$OPTARG
> > @@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
> >          v)
> >              verbose="yes"
> >              ;;
> > +        p)
> > +           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> > +           ;;
> >          *)
> >              exit
> >              ;;
> >      esac
> >  done
> >  
> > -RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> > +RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
> >  config=$TEST_DIR/unittests.cfg
> >  rm -f test.log
> >  echo > test.log
> > diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
> > new file mode 100755
> > index 0000000..49564bd
> > --- /dev/null
> > +++ b/scripts/pretty_print_stacks.py
> > @@ -0,0 +1,61 @@
> > +#!/usr/bin/env python
> > +
> > +import re
> > +import subprocess
> > +import sys
> > +
> > +if len(sys.argv) != 2:
> > +    sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
> > +    sys.exit(1)
> > +
> > +binary = sys.argv[1]
> > +
> > +# Subvert output buffering.
> > +def puts(string):
> > +    sys.stdout.write(string)
> > +    sys.stdout.flush()
> > +
> > +try:
> > +    while True:
> > +        # Subvert input buffering.
> > +        line = sys.stdin.readline()
> > +        if line == '':
> > +            break
> > +
> > +        if not line.strip().startswith('STACK:'):
> > +            puts(line)
> > +            continue
> > +
> > +        addrs = line.split()[1:]
> > +        # Output like this:
> > +        #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
> > +        #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
> > +        cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
> > +        cmd.extend(addrs)
> > +
> > +        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> > +        out, err = p.communicate()
> > +        if p.returncode != 0:
> > +            puts(line)
> > +            continue
> > +
> > +        for line in out.splitlines():
> > +            m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
> > +            if m is None:
> > +                puts('%s\n' % line)
> > +                continue
> > +
> > +            head, path, line, tail = m.groups()
> > +            line = int(line)
> > +            puts('%s at %s:%d%s\n' % (head, path, line, tail))
> > +            try:
> > +                lines = open(path).readlines()
> > +            except IOError:
> > +                continue
> > +            if line > 1:
> > +                puts('        %s\n' % lines[line - 2].rstrip())
> > +            puts('      > %s\n' % lines[line - 1].rstrip())
> > +            if line < len(lines):
> > +                puts('        %s\n' % lines[line].rstrip())
> > +except:
> > +    sys.exit(1)
> > 
> 
> For now I've committed only the script; I'd like Drew to take a look at
> run_tests.sh and see if it's possible/worthwhile to include the pretty
> printing script in mkstandalone.sh.  But even without integration in
> run_tests.sh the scripts is nice and useful.
> 
> Queued, will push tomorrow together with the rest.

It should be possible, just another "temp_file", I think. But then we
expect the standalone test to be executed somewhere with pyhon (likely)
and addr2line (not as likely). Also, without the source available, then
it probably isn't all the useful to the user executing the test, and
anybody they send the results to can copy+paste the backtrace line
directly into addr2line themselves.

Long story short, I vote we don't add it to mkstandalone.

Now, one addition that may make sense, wrt mkstandalone, is that we output
what HEAD was when mkstandalone was used to build the tests. That would be
useful just by itself to know what code is running, and extra useful when
using addr2line.

drew

Re: [kvm-unit-tests 5/5] scripts: pretty print stack traces

[Paolo Bonzini]

On 03/03/2016 10:35, Andrew Jones wrote:
> wrt to enabling/disabling it though, how about we create a new configure
> command line option instead. One that defaults to on, if the architecture
> supports pretty-printing, but can be switched off. 

Which architectures don't?  But I like a
--enable/disable-pretty-print-stacks option.

> It should be possible, just another "temp_file", I think. But then we
> expect the standalone test to be executed somewhere with pyhon (likely)
> and addr2line (not as likely). Also, without the source available, then
> it probably isn't all the useful to the user executing the test, and
> anybody they send the results to can copy+paste the backtrace line
> directly into addr2line themselves.
> 
> Long story short, I vote we don't add it to mkstandalone.

I agree.  Something like

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index c7e78f8..d6607b8 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -41,6 +41,10 @@ generate_test ()
 	echo "#!/bin/bash"
 	echo "export STANDALONE=yes"
 	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e
's/arm.*/arm/')"
+
+	# do not require addr2line on the test machine
+	echo "export PRETTY_PRINT_STACKS=no"
+
 	config_export ARCH
 	config_export ARCH_NAME
 	config_export PROCESSOR

if we go with your suggestion of configurable pretty printing.

> Now, one addition that may make sense, wrt mkstandalone, is that we output
> what HEAD was when mkstandalone was used to build the tests. That would be
> useful just by itself to know what code is running, and extra useful when
> using addr2line.

That makes sense.  Can you send a patch (Drew)?

Paolo

Re: [kvm-unit-tests 5/5] scripts: pretty print stack traces

[Andrew Jones]

On Thu, Mar 03, 2016 at 01:57:37PM +0100, Paolo Bonzini wrote:
> 
> 
> On 03/03/2016 10:35, Andrew Jones wrote:
> > wrt to enabling/disabling it though, how about we create a new configure
> > command line option instead. One that defaults to on, if the architecture
> > supports pretty-printing, but can be switched off. 
> 
> Which architectures don't?  But I like a
> --enable/disable-pretty-print-stacks option.

Eventually, probably none.

> 
> > It should be possible, just another "temp_file", I think. But then we
> > expect the standalone test to be executed somewhere with pyhon (likely)
> > and addr2line (not as likely). Also, without the source available, then
> > it probably isn't all the useful to the user executing the test, and
> > anybody they send the results to can copy+paste the backtrace line
> > directly into addr2line themselves.
> > 
> > Long story short, I vote we don't add it to mkstandalone.
> 
> I agree.  Something like
> 
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index c7e78f8..d6607b8 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -41,6 +41,10 @@ generate_test ()
>  	echo "#!/bin/bash"
>  	echo "export STANDALONE=yes"
>  	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e
> 's/arm.*/arm/')"
> +
> +	# do not require addr2line on the test machine
> +	echo "export PRETTY_PRINT_STACKS=no"
> +
>  	config_export ARCH
>  	config_export ARCH_NAME
>  	config_export PROCESSOR
> 
> if we go with your suggestion of configurable pretty printing.

yeah, exactly.

> 
> > Now, one addition that may make sense, wrt mkstandalone, is that we output
> > what HEAD was when mkstandalone was used to build the tests. That would be
> > useful just by itself to know what code is running, and extra useful when
> > using addr2line.
> 
> That makes sense.  Can you send a patch (Drew)?

Yup, got it on the ol' TODO already :-)

drew

[kvm-unit-tests v2 0/8] Debugging aids

[Peter Feiner]

Some aids for debugging: stack dumping, register dumping, and printing
failing assertion conditions.

N.B. I haven't tried compiling on arm or ppc.

v2:
	* Fixed a bunch tests on x86 that broke with
	  -fno-omit-frame-pointer. 
	* Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
	  will have similar problems that I fixed for x86.
	

Peter Feiner (8):
  x86: emulator: asm fixes
  x86: emulator: disable test_lldt
  x86: realmode: fix test_sgdt_sidt overflow
  x86: eventinj: make test work with -O0
  lib: backtrace printing
  x86: lib: debug dump on unhandled exceptions
  lib: dump stack on abort()
  scripts: pretty print stack traces

 Makefile                       |  3 +-
 arm/Makefile.common            |  1 +
 lib/arm/dump_stack.c           |  6 ++++
 lib/libcflat.h                 | 10 ++++--
 lib/powerpc/dump_stack.c       |  6 ++++
 lib/printf.c                   | 37 +++++++++++++++++++++
 lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++++++++++----
 lib/x86/dump_stack.c           | 24 ++++++++++++++
 powerpc/Makefile.common        |  1 +
 run_tests.sh                   | 10 ++++--
 scripts/pretty_print_stacks.py | 61 ++++++++++++++++++++++++++++++++++
 x86/Makefile.common            |  4 +++
 x86/emulator.c                 | 74 +++++++++++++++++++++++++++---------------
 x86/eventinj.c                 |  3 +-
 x86/realmode.c                 | 23 ++++++-------
 15 files changed, 281 insertions(+), 51 deletions(-)
 create mode 100644 lib/arm/dump_stack.c
 create mode 100644 lib/powerpc/dump_stack.c
 create mode 100644 lib/x86/dump_stack.c
 create mode 100755 scripts/pretty_print_stacks.py

-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v2 1/8] x86: emulator: asm fixes

[Peter Feiner]

Found several problems in the emulator test when trying to compile
with various combinations of -fno-omit-frame-pointer and -O{0,1}.

Fixed register clobbering in several tests. It's not good enough to
save & restore a register with I/O operands because the compiler can
use the register to effect the I/O! Fixing %rbp was tricky because it
can't be listed in the clobber list when -fno-omit-frame-pointer is in
effect; thus %rbp has to be saved manually using a scratch register
that's declared as clobbered.

The length of the jmp instruction in test_jmp_noncanonical was 2, not
3. This patch makes the exception handler more robust by using the
computed instruction length.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/emulator.c | 60 +++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/x86/emulator.c b/x86/emulator.c
index e5c1c6b..d990961 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -260,24 +260,30 @@ void test_pop(void *mem)
 	report("ret", 1);
 
 	stack_top[-1] = 0x778899;
-	asm volatile("mov %%rsp, %[tmp] \n\t"
-		     "mov %%rbp, %[tmp3] \n\t"
-		     "mov %[stack_top], %%rbp \n\t"
+	asm volatile("mov %[stack_top], %%r8 \n\t"
+		     "mov %%rsp, %%r9 \n\t"
+		     "xchg %%rbp, %%r8 \n\t"
 		     "leave \n\t"
-		     "xchg %%rsp, %[tmp] \n\t"
-		     "xchg %%rbp, %[tmp3]"
+		     "xchg %%rsp, %%r9 \n\t"
+		     "xchg %%rbp, %%r8 \n\t"
+		     "mov %%r9, %[tmp] \n\t"
+		     "mov %%r8, %[tmp3]"
 		     : [tmp]"=&r"(tmp), [tmp3]"=&r"(tmp3) : [stack_top]"r"(stack_top-1)
-		     : "memory");
+		     : "memory", "r8", "r9");
 	report("leave", tmp == (ulong)stack_top && tmp3 == 0x778899);
 
 	rbp = 0xaa55aa55bb66bb66ULL;
 	rsp = (unsigned long)stack_top;
-	asm volatile("xchg %%rsp, %[rsp] \n\t"
-		     "xchg %%rbp, %[rbp] \n\t"
+	asm volatile("mov %[rsp], %%r8 \n\t"
+		     "mov %[rbp], %%r9 \n\t"
+		     "xchg %%rsp, %%r8 \n\t"
+		     "xchg %%rbp, %%r9 \n\t"
 		     "enter $0x1238, $0 \n\t"
-		     "xchg %%rsp, %[rsp] \n\t"
-		     "xchg %%rbp, %[rbp]"
-		     : [rsp]"+a"(rsp), [rbp]"+b"(rbp) : : "memory");
+		     "xchg %%rsp, %%r8 \n\t"
+		     "xchg %%rbp, %%r9 \n\t"
+		     "xchg %%r8, %[rsp] \n\t"
+		     "xchg %%r9, %[rbp]"
+		     : [rsp]"+a"(rsp), [rbp]"+b"(rbp) : : "memory", "r8", "r9");
 	report("enter",
 	       rsp == (unsigned long)stack_top - 8 - 0x1238
 	       && rbp == (unsigned long)stack_top - 8
@@ -405,7 +411,7 @@ void test_xchg(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xchg reg, r/m (1)",
 	       rax == 0xfedcba98765432ef && *memq == 0x123456789abcd10);
 
@@ -416,7 +422,7 @@ void test_xchg(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xchg reg, r/m (2)",
 	       rax == 0xfedcba987654cdef && *memq == 0x123456789ab3210);
 
@@ -427,7 +433,7 @@ void test_xchg(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xchg reg, r/m (3)",
 	       rax == 0x89abcdef && *memq == 0x123456776543210);
 
@@ -438,7 +444,7 @@ void test_xchg(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xchg reg, r/m (4)",
 	       rax == 0x123456789abcdef && *memq == 0xfedcba9876543210);
 }
@@ -455,7 +461,7 @@ void test_xadd(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xadd reg, r/m (1)",
 	       rax == 0xfedcba98765432ef && *memq == 0x123456789abcdff);
 
@@ -466,7 +472,7 @@ void test_xadd(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xadd reg, r/m (2)",
 	       rax == 0xfedcba987654cdef && *memq == 0x123456789abffff);
 
@@ -477,7 +483,7 @@ void test_xadd(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xadd reg, r/m (3)",
 	       rax == 0x89abcdef && *memq == 0x1234567ffffffff);
 
@@ -488,7 +494,7 @@ void test_xadd(void *mem)
 		     "mov %%rax, %[rax]\n\t"
 		     : [rax]"=r"(rax)
 		     : [memq]"r"(memq)
-		     : "memory");
+		     : "memory", "rax");
 	report("xadd reg, r/m (4)",
 	       rax == 0x123456789abcdef && *memq == 0xffffffffffffffff);
 }
@@ -804,10 +810,12 @@ static void trap_emulator(uint64_t *mem, void *alt_insn_page,
 	outregs = save;
 }
 
-static void advance_rip_by_3_and_note_exception(struct ex_regs *regs)
+static unsigned long rip_advance;
+
+static void advance_rip_and_note_exception(struct ex_regs *regs)
 {
     ++exceptions;
-    regs->rip += 3;
+    regs->rip += rip_advance;
 }
 
 static void test_mmx_movq_mf(uint64_t *mem, uint8_t *insn_page,
@@ -819,11 +827,12 @@ static void test_mmx_movq_mf(uint64_t *mem, uint8_t *insn_page,
 
     write_cr0(read_cr0() & ~6);  /* TS, EM */
     exceptions = 0;
-    handle_exception(MF_VECTOR, advance_rip_by_3_and_note_exception);
+    handle_exception(MF_VECTOR, advance_rip_and_note_exception);
     asm volatile("fninit; fldcw %0" : : "m"(fcw));
     asm volatile("fldz; fldz; fdivp"); /* generate exception */
 
     MK_INSN(mmx_movq_mf, "movq %mm0, (%rax) \n\t");
+    rip_advance = insn_mmx_movq_mf.len;
     inregs = (struct regs){ .rsp=(u64)stack+1024 };
     trap_emulator(mem, alt_insn_page, &insn_mmx_movq_mf);
     /* exit MMX mode */
@@ -834,11 +843,14 @@ static void test_mmx_movq_mf(uint64_t *mem, uint8_t *insn_page,
 
 static void test_jmp_noncanonical(uint64_t *mem)
 {
+	extern char nc_jmp_start, nc_jmp_end;
+
 	*mem = 0x1111111111111111ul;
 
 	exceptions = 0;
-	handle_exception(GP_VECTOR, advance_rip_by_3_and_note_exception);
-	asm volatile ("jmp *%0" : : "m"(*mem));
+	rip_advance = &nc_jmp_end - &nc_jmp_start;
+	handle_exception(GP_VECTOR, advance_rip_and_note_exception);
+	asm volatile ("nc_jmp_start: jmp *%0; nc_jmp_end:" : : "m"(*mem));
 	report("jump to non-canonical address", exceptions == 1);
 	handle_exception(GP_VECTOR, 0);
 }
-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v2 2/8] x86: emulator: disable test_lldt

[Peter Feiner]

When compiled with -fno-omit-frame-pointer, test_lldt causes a triple
fault, which insta-reboots the guest. I couldn't figure out why the
triple-fault was happening, so I disabled the test outright. Knowing
that the other emulator.c tests pass is more useful than a VM stuck in
a reboot loop.

On x86-64, this test was adding little value since it was broken to
begin with: an LDT descriptor is 16 bytes on x86-64. The GDT limit
was also bogus. I've fixed these problems for posterity.

N.B. I only tested this on Linux 4.3.5. Perhaps lldt emulation has
been fixed since then.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/emulator.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/x86/emulator.c b/x86/emulator.c
index d990961..3730721 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -1006,10 +1006,17 @@ static void test_sreg(volatile uint16_t *mem)
     write_ss(ss);
 }
 
+/* Broken emulation causes triple fault, which skips the other tests. */
+#if 0
 static void test_lldt(volatile uint16_t *mem)
 {
-    u64 gdt[] = { 0, 0x0000f82000000ffffull /* ldt descriptor */ };
-    struct descriptor_table_ptr gdt_ptr = { .limit = 0xffff, .base = (ulong)&gdt };
+    u64 gdt[] = { 0, /* null descriptor */
+#ifdef __X86_64__
+		  0, /* ldt descriptor is 16 bytes in long mode */
+#endif
+		  0x0000f82000000ffffull /* ldt descriptor */ };
+    struct descriptor_table_ptr gdt_ptr = { .limit = sizeof(gdt) - 1,
+					    .base = (ulong)&gdt };
     struct descriptor_table_ptr orig_gdt;
 
     cli();
@@ -1021,6 +1028,7 @@ static void test_lldt(volatile uint16_t *mem)
     sti();
     report("lldt", sldt() == *mem);
 }
+#endif
 
 static void test_ltr(volatile uint16_t *mem)
 {
@@ -1139,7 +1147,7 @@ int main()
 	test_shld_shrd(mem);
 	//test_lgdt_lidt(mem);
 	test_sreg(mem);
-	test_lldt(mem);
+	//test_lldt(mem);
 	test_ltr(mem);
 	test_cmov(mem);
 
-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v2 3/8] x86: realmode: fix test_sgdt_sidt overflow

[Peter Feiner]

In real mode, both sgdt and sidt write 6 bytes to the given memory
address: 2 byte limit, 3 byte address, 1 zero byte. However, the test
was only allocating 4 bytes. Given an inopportune stack layout, the
output was being overwritten and the assertion failed.

I discovered this problem when compiling with -fno-omit-stack-pointer.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/realmode.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/x86/realmode.c b/x86/realmode.c
index 09e6aa7..6411654 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -116,16 +116,18 @@ struct regs {
 	u32 eip, eflags;
 };
 
+struct table_descr {
+	u16 limit;
+	void *base;
+} __attribute__((packed));
+
 static u64 gdt[] = {
 	0,
 	0x00cf9b000000ffffull, // flat 32-bit code segment
 	0x00cf93000000ffffull, // flat 32-bit data segment
 };
 
-static struct {
-	u16 limit;
-	void *base;
-} __attribute__((packed)) gdt_descr = {
+static struct table_descr gdt_descr = {
 	sizeof(gdt) - 1,
 	gdt,
 };
@@ -1417,21 +1419,23 @@ static void test_ss_base_for_esp_ebp(void)
     report("ss relative addressing (2)", R_AX | R_BX, outregs.ebx == 0x87654321);
 }
 
+extern unsigned long long r_gdt[];
+
 static void test_sgdt_sidt(void)
 {
     MK_INSN(sgdt, "sgdtw (%eax)");
     MK_INSN(sidt, "sidtw (%eax)");
-    unsigned x, y;
+    struct table_descr x, y;
 
     inregs.eax = (unsigned)&y;
     asm volatile("sgdtw %0" : "=m"(x));
     exec_in_big_real_mode(&insn_sgdt);
-    report("sgdt", 0, x == y);
+    report("sgdt", 0, x.limit == y.limit && x.base == y.base);
 
     inregs.eax = (unsigned)&y;
     asm volatile("sidtw %0" : "=m"(x));
     exec_in_big_real_mode(&insn_sidt);
-    report("sidt", 0, x == y);
+    report("sidt", 0, x.limit == y.limit && x.base == y.base);
 }
 
 static void test_sahf(void)
@@ -1734,10 +1738,7 @@ void realmode_start(void)
 
 unsigned long long r_gdt[] = { 0, 0x9b000000ffff, 0x93000000ffff };
 
-struct __attribute__((packed)) {
-	unsigned short limit;
-	void *base;
-} r_gdt_descr = { sizeof(r_gdt) - 1, &r_gdt };
+struct table_descr r_gdt_descr = { sizeof(r_gdt) - 1, &r_gdt };
 
 asm(
 	".section .init \n\t"
-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v2 4/8] x86: eventinj: make test work with -O0

[Peter Feiner]

With optimizations disabled, the compiler generated a call for
irq_enable() -- rather than just inlining an sti instruction as
expected. Thus the irq_enable ret would run in the sti shadow and the
vectors would fire in the wrong order.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/eventinj.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/x86/eventinj.c b/x86/eventinj.c
index bddedce..202ac19 100644
--- a/x86/eventinj.c
+++ b/x86/eventinj.c
@@ -296,8 +296,7 @@ int main()
 	apic_self_ipi(32);
 	flush_stack();
 	io_delay();
-	irq_enable();
-	asm volatile ("int $33");
+	asm volatile ("sti; int $33");
 	irq_disable();
 	printf("After vec 32 and int $33\n");
 	report("vec 32/int $33", test_count == 2);
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v2 4/8] x86: eventinj: make test work with -O0

[Paolo Bonzini]

On 03/03/2016 02:09, Peter Feiner wrote:
> With optimizations disabled, the compiler generated a call for
> irq_enable() -- rather than just inlining an sti instruction as
> expected. Thus the irq_enable ret would run in the sti shadow and the
> vectors would fire in the wrong order.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  x86/eventinj.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/x86/eventinj.c b/x86/eventinj.c
> index bddedce..202ac19 100644
> --- a/x86/eventinj.c
> +++ b/x86/eventinj.c
> @@ -296,8 +296,7 @@ int main()
>  	apic_self_ipi(32);
>  	flush_stack();
>  	io_delay();
> -	irq_enable();
> -	asm volatile ("int $33");
> +	asm volatile ("sti; int $33");
>  	irq_disable();
>  	printf("After vec 32 and int $33\n");
>  	report("vec 32/int $33", test_count == 2);
> 

I've added a comment to irq_enable too:

/* Note that irq_enable() does not ensure an interrupt shadow due
 * to the vagaries of compiler optimizations.  If you need the
 * shadow, use a single asm with "sti" and the instruction after it.
 */


Paolo

[kvm-unit-tests v2 5/8] lib: backtrace printing

[Peter Feiner]

Functions to walk stack and print backtrace. The stack's unadorned as

	STACK: addr addr addr ...

A follow-up patch post-processes the output to pretty-print the stack.

Stack walker is just a stub on arm and ppc.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 Makefile                 |  3 ++-
 arm/Makefile.common      |  1 +
 lib/arm/dump_stack.c     |  6 ++++++
 lib/libcflat.h           |  4 ++++
 lib/powerpc/dump_stack.c |  6 ++++++
 lib/printf.c             | 37 +++++++++++++++++++++++++++++++++++++
 lib/x86/dump_stack.c     | 24 ++++++++++++++++++++++++
 powerpc/Makefile.common  |  1 +
 x86/Makefile.common      |  4 ++++
 9 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 lib/arm/dump_stack.c
 create mode 100644 lib/powerpc/dump_stack.c
 create mode 100644 lib/x86/dump_stack.c

diff --git a/Makefile b/Makefile
index ddba941..40ea4ec 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
 
 CFLAGS += -g
 CFLAGS += $(autodepend-flags) -Wall
-CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
+frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
+CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
 CFLAGS += $(call cc-option, -fno-stack-protector, "")
 CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
 
diff --git a/arm/Makefile.common b/arm/Makefile.common
index dd3a0ca..054bdee 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
 cflatobjs += lib/arm/bitops.o
 cflatobjs += lib/arm/psci.o
 cflatobjs += lib/arm/smp.o
+cflatobjs += lib/arm/dump_stack.o
 
 libeabi = lib/arm/libeabi.a
 eabiobjs = lib/arm/eabi_compat.o
diff --git a/lib/arm/dump_stack.c b/lib/arm/dump_stack.c
new file mode 100644
index 0000000..528ba63
--- /dev/null
+++ b/lib/arm/dump_stack.c
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	return 0;
+}
diff --git a/lib/libcflat.h b/lib/libcflat.h
index 1f0049c..42c94df 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -65,6 +65,10 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth);
+void dump_stack(unsigned long ip, unsigned long bp);
+void dump_current_stack(void);
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\
diff --git a/lib/powerpc/dump_stack.c b/lib/powerpc/dump_stack.c
new file mode 100644
index 0000000..528ba63
--- /dev/null
+++ b/lib/powerpc/dump_stack.c
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	return 0;
+}
diff --git a/lib/printf.c b/lib/printf.c
index 2aec59a..e97fca9 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -259,3 +259,40 @@ int printf(const char *fmt, ...)
     puts(buf);
     return r;
 }
+
+static void print_stack(unsigned long *stack, int depth,
+			bool top_is_return_address)
+{
+	int i;
+
+	printf("\tSTACK: " );
+	for (i = 0; i < depth; i++) {
+		int offset = -1;
+		if (i == 0 && !top_is_return_address)
+			offset = 0;
+		printf(" %lx", stack[i] + offset);
+	}
+	printf("\n");
+}
+
+#define MAX_DEPTH 10
+
+void dump_stack(unsigned long ip, unsigned long bp)
+{
+	unsigned long stack[MAX_DEPTH];
+	int depth;
+
+	stack[0] = ip;
+	depth = walk_stack(bp, &stack[1], MAX_DEPTH - 1);
+	print_stack(stack, depth + 1, false);
+}
+
+void dump_current_stack(void)
+{
+	unsigned long stack[MAX_DEPTH];
+	int depth;
+
+	depth = walk_stack((unsigned long)__builtin_frame_address(1), stack,
+			   MAX_DEPTH);
+	print_stack(stack, depth, true);
+}
diff --git a/lib/x86/dump_stack.c b/lib/x86/dump_stack.c
new file mode 100644
index 0000000..6e9d126
--- /dev/null
+++ b/lib/x86/dump_stack.c
@@ -0,0 +1,24 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	unsigned long *frame = (unsigned long *) bp;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	for (depth = 0; depth < max_depth; depth++) {
+		stack[depth] = frame[1];
+		if (stack[depth] == 0)
+			break;
+		frame = (unsigned long *) frame[0];
+	}
+
+	walking = 0;
+	return depth;
+}
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 2ce6494..694dea0 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -30,6 +30,7 @@ cflatobjs += lib/powerpc/io.o
 cflatobjs += lib/powerpc/hcall.o
 cflatobjs += lib/powerpc/setup.o
 cflatobjs += lib/powerpc/rtas.o
+cflatobjs += lib/powerpc/dump_stack.o
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 3a14fea..d7c7eab 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
 cflatobjs += lib/x86/desc.o
 cflatobjs += lib/x86/isr.o
 cflatobjs += lib/x86/acpi.o
+cflatobjs += lib/x86/dump_stack.o
 
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I lib
@@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
 CFLAGS += -m$(bits)
 CFLAGS += -O1
 
+# dump_stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
 libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
 
 FLATLIBS = lib/libcflat.a $(libgcc)
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v2 5/8] lib: backtrace printing

[Andrew Jones]

On Wed, Mar 02, 2016 at 05:09:35PM -0800, Peter Feiner wrote:
> Functions to walk stack and print backtrace. The stack's unadorned as
> 
> 	STACK: addr addr addr ...
> 
> A follow-up patch post-processes the output to pretty-print the stack.
> 
> Stack walker is just a stub on arm and ppc.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  Makefile                 |  3 ++-
>  arm/Makefile.common      |  1 +
>  lib/arm/dump_stack.c     |  6 ++++++
>  lib/libcflat.h           |  4 ++++
>  lib/powerpc/dump_stack.c |  6 ++++++
>  lib/printf.c             | 37 +++++++++++++++++++++++++++++++++++++
>  lib/x86/dump_stack.c     | 24 ++++++++++++++++++++++++
>  powerpc/Makefile.common  |  1 +
>  x86/Makefile.common      |  4 ++++
>  9 files changed, 85 insertions(+), 1 deletion(-)
>  create mode 100644 lib/arm/dump_stack.c
>  create mode 100644 lib/powerpc/dump_stack.c
>  create mode 100644 lib/x86/dump_stack.c
> 
> diff --git a/Makefile b/Makefile
> index ddba941..40ea4ec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
>  
>  CFLAGS += -g
>  CFLAGS += $(autodepend-flags) -Wall
> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
> +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> +CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
>  CFLAGS += $(call cc-option, -fno-stack-protector, "")
>  CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
>  
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index dd3a0ca..054bdee 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
>  cflatobjs += lib/arm/bitops.o
>  cflatobjs += lib/arm/psci.o
>  cflatobjs += lib/arm/smp.o
> +cflatobjs += lib/arm/dump_stack.o
>  
>  libeabi = lib/arm/libeabi.a
>  eabiobjs = lib/arm/eabi_compat.o
> diff --git a/lib/arm/dump_stack.c b/lib/arm/dump_stack.c
> new file mode 100644
> index 0000000..528ba63
> --- /dev/null
> +++ b/lib/arm/dump_stack.c
> @@ -0,0 +1,6 @@
> +#include "libcflat.h"
> +
> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
> +{
> +	return 0;
> +}
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 1f0049c..42c94df 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -65,6 +65,10 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
>  extern void report_abort(const char *msg_fmt, ...);
>  extern int report_summary(void);
>  
> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth);
> +void dump_stack(unsigned long ip, unsigned long bp);
> +void dump_current_stack(void);

The inputs are a bit x86ish. On ARM, for example, we call them pc and
fp. Acutally, how about we implement backtrace(3) instead? And then wrap
print_current_stack(void) around that.

Also, instead of building on a single arch function, walk_stack, how about
two

  void *arch_frame_address(unsigned int level);
  void *arch_return_address(void *frame, unsigned int level);

The architecture may be able to easily implement these with gcc builtins,
or not. I've added *frame to arch_return_address' parameters in case
__builtin_return_address(level) doesn't work. 

And, let's just require the architectures to have the functions, not that
they have a dump_stack.c file. If we include lib/asm/<ARCH>/stack.h, or
whatever you want to call it, then the arch could just supply some inlines
there.

> +
>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
>  
>  #define container_of(ptr, type, member) ({				\
> diff --git a/lib/powerpc/dump_stack.c b/lib/powerpc/dump_stack.c
> new file mode 100644
> index 0000000..528ba63
> --- /dev/null
> +++ b/lib/powerpc/dump_stack.c
> @@ -0,0 +1,6 @@
> +#include "libcflat.h"
> +
> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
> +{
> +	return 0;
> +}
> diff --git a/lib/printf.c b/lib/printf.c
> index 2aec59a..e97fca9 100644
> --- a/lib/printf.c
> +++ b/lib/printf.c
> @@ -259,3 +259,40 @@ int printf(const char *fmt, ...)
>      puts(buf);
>      return r;
>  }
> +
> +static void print_stack(unsigned long *stack, int depth,
> +			bool top_is_return_address)
> +{
> +	int i;
> +
> +	printf("\tSTACK: " );
> +	for (i = 0; i < depth; i++) {
> +		int offset = -1;
> +		if (i == 0 && !top_is_return_address)
> +			offset = 0;
> +		printf(" %lx", stack[i] + offset);

I'm afraid I don't understand the use of offset here. I fear it may
be x86-ness leaked into the arch-neutral print_stack().

> +	}
> +	printf("\n");
> +}
> +
> +#define MAX_DEPTH 10
> +
> +void dump_stack(unsigned long ip, unsigned long bp)
> +{
> +	unsigned long stack[MAX_DEPTH];
> +	int depth;
> +
> +	stack[0] = ip;
> +	depth = walk_stack(bp, &stack[1], MAX_DEPTH - 1);
> +	print_stack(stack, depth + 1, false);
> +}

When would dump_stack() be useful, as opposed to dump_current_stack?

> +
> +void dump_current_stack(void)
> +{
> +	unsigned long stack[MAX_DEPTH];
> +	int depth;
> +
> +	depth = walk_stack((unsigned long)__builtin_frame_address(1), stack,
> +			   MAX_DEPTH);
> +	print_stack(stack, depth, true);
> +}
> diff --git a/lib/x86/dump_stack.c b/lib/x86/dump_stack.c
> new file mode 100644
> index 0000000..6e9d126
> --- /dev/null
> +++ b/lib/x86/dump_stack.c
> @@ -0,0 +1,24 @@
> +#include "libcflat.h"
> +
> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
> +{
> +	static int walking;
> +	int depth = 0;
> +	unsigned long *frame = (unsigned long *) bp;
> +
> +	if (walking) {
> +		printf("RECURSIVE STACK WALK!!!\n");
> +		return 0;
> +	}
> +	walking = 1;
> +
> +	for (depth = 0; depth < max_depth; depth++) {
> +		stack[depth] = frame[1];
> +		if (stack[depth] == 0)
> +			break;
> +		frame = (unsigned long *) frame[0];
> +	}
> +
> +	walking = 0;
> +	return depth;
> +}
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 2ce6494..694dea0 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -30,6 +30,7 @@ cflatobjs += lib/powerpc/io.o
>  cflatobjs += lib/powerpc/hcall.o
>  cflatobjs += lib/powerpc/setup.o
>  cflatobjs += lib/powerpc/rtas.o
> +cflatobjs += lib/powerpc/dump_stack.o
>  
>  FLATLIBS = $(libcflat) $(LIBFDT_archive)
>  %.elf: CFLAGS += $(arch_CFLAGS)
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 3a14fea..d7c7eab 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
>  cflatobjs += lib/x86/desc.o
>  cflatobjs += lib/x86/isr.o
>  cflatobjs += lib/x86/acpi.o
> +cflatobjs += lib/x86/dump_stack.o
>  
>  $(libcflat): LDFLAGS += -nostdlib
>  $(libcflat): CFLAGS += -ffreestanding -I lib
> @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
>  CFLAGS += -m$(bits)
>  CFLAGS += -O1
>  
> +# dump_stack.o relies on frame pointers.
> +KEEP_FRAME_POINTER := y
> +
>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>  
>  FLATLIBS = lib/libcflat.a $(libgcc)
> -- 
> 2.7.0.rc3.207.g0ac5344

Thanks,
drew

Re: [kvm-unit-tests v2 5/8] lib: backtrace printing

[Peter Feiner]

On Thu, Mar 3, 2016 at 1:17 AM, Andrew Jones <drjones@redhat.com> wrote:
> On Wed, Mar 02, 2016 at 05:09:35PM -0800, Peter Feiner wrote:
>> Functions to walk stack and print backtrace. The stack's unadorned as
>>
>>       STACK: addr addr addr ...
>>
>> A follow-up patch post-processes the output to pretty-print the stack.
>>
>> Stack walker is just a stub on arm and ppc.
>>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> ---
>>  Makefile                 |  3 ++-
>>  arm/Makefile.common      |  1 +
>>  lib/arm/dump_stack.c     |  6 ++++++
>>  lib/libcflat.h           |  4 ++++
>>  lib/powerpc/dump_stack.c |  6 ++++++
>>  lib/printf.c             | 37 +++++++++++++++++++++++++++++++++++++
>>  lib/x86/dump_stack.c     | 24 ++++++++++++++++++++++++
>>  powerpc/Makefile.common  |  1 +
>>  x86/Makefile.common      |  4 ++++
>>  9 files changed, 85 insertions(+), 1 deletion(-)
>>  create mode 100644 lib/arm/dump_stack.c
>>  create mode 100644 lib/powerpc/dump_stack.c
>>  create mode 100644 lib/x86/dump_stack.c
>>
>> diff --git a/Makefile b/Makefile
>> index ddba941..40ea4ec 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
>>
>>  CFLAGS += -g
>>  CFLAGS += $(autodepend-flags) -Wall
>> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
>> +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
>> +CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
>>  CFLAGS += $(call cc-option, -fno-stack-protector, "")
>>  CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
>>
>> diff --git a/arm/Makefile.common b/arm/Makefile.common
>> index dd3a0ca..054bdee 100644
>> --- a/arm/Makefile.common
>> +++ b/arm/Makefile.common
>> @@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
>>  cflatobjs += lib/arm/bitops.o
>>  cflatobjs += lib/arm/psci.o
>>  cflatobjs += lib/arm/smp.o
>> +cflatobjs += lib/arm/dump_stack.o
>>
>>  libeabi = lib/arm/libeabi.a
>>  eabiobjs = lib/arm/eabi_compat.o
>> diff --git a/lib/arm/dump_stack.c b/lib/arm/dump_stack.c
>> new file mode 100644
>> index 0000000..528ba63
>> --- /dev/null
>> +++ b/lib/arm/dump_stack.c
>> @@ -0,0 +1,6 @@
>> +#include "libcflat.h"
>> +
>> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
>> +{
>> +     return 0;
>> +}
>> diff --git a/lib/libcflat.h b/lib/libcflat.h
>> index 1f0049c..42c94df 100644
>> --- a/lib/libcflat.h
>> +++ b/lib/libcflat.h
>> @@ -65,6 +65,10 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
>>  extern void report_abort(const char *msg_fmt, ...);
>>  extern int report_summary(void);
>>
>> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth);
>> +void dump_stack(unsigned long ip, unsigned long bp);
>> +void dump_current_stack(void);
>
> The inputs are a bit x86ish. On ARM, for example, we call them pc and
> fp. Acutally, how about we implement backtrace(3) instead? And then wrap
> print_current_stack(void) around that.

I'm indifferent w.r.t. input names. I can change them to void
*instruction and void *frame.

I didn't go with backtrace() because I wanted to give the frame
pointer as an argument. The frame pointer argument is essential for
dumping stacks of unhandled exceptions. See the 6th patch in the
series, "
x86: lib: debug dump on unhandled exceptions".

> Also, instead of building on a single arch function, walk_stack, how about
> two
>
>   void *arch_frame_address(unsigned int level);
>   void *arch_return_address(void *frame, unsigned int level);

This would entail O(level^2) frame pointer traversals to print a stack
trace. The extra CPU overhead doesn't bother me *too much*, but the
inevitable debugging burden does. Every once and a while, I have to
stick a printf() in walk_stack to see where a frame pointer is causing
a #PF :-) So I'd like to stick with a function that returns an array
of pointers.

I can call it

int arch_return_addresses(const void *frame, void **addresses, int max_depth);

(is the arch_ prefix necessary given that there's no non-arch_ variant?)

Here's the best of both worlds:

/* return addresses starting from the given frame. needs arch-specific code. */
int backtrace_frame(const void *frame, void **addresses, int max_depth);

/* return addresses from the current frame. Can use repeated calls to
__builtin_return_address. */
int backtrace(void **addresses, int max_depth);

I'll use backtrace() in dump_current_stack() and backtrace_frame in
dump_stack(). I'll also rename dump_current_stack to dump_stack and
dump_stack to dump_frame_stack for some consistency.

Wow, I should've just written the patch instead of writing all of this
crap out :-)

> The architecture may be able to easily implement these with gcc builtins,
> or not. I've added *frame to arch_return_address' parameters in case
> __builtin_return_address(level) doesn't work.
>
> And, let's just require the architectures to have the functions, not that
> they have a dump_stack.c file. If we include lib/asm/<ARCH>/stack.h, or
> whatever you want to call it, then the arch could just supply some inlines
> there.

That's a good suggestion. I'll change things.

>> +
>>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
>>
>>  #define container_of(ptr, type, member) ({                           \
>> diff --git a/lib/powerpc/dump_stack.c b/lib/powerpc/dump_stack.c
>> new file mode 100644
>> index 0000000..528ba63
>> --- /dev/null
>> +++ b/lib/powerpc/dump_stack.c
>> @@ -0,0 +1,6 @@
>> +#include "libcflat.h"
>> +
>> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
>> +{
>> +     return 0;
>> +}
>> diff --git a/lib/printf.c b/lib/printf.c
>> index 2aec59a..e97fca9 100644
>> --- a/lib/printf.c
>> +++ b/lib/printf.c
>> @@ -259,3 +259,40 @@ int printf(const char *fmt, ...)
>>      puts(buf);
>>      return r;
>>  }
>> +
>> +static void print_stack(unsigned long *stack, int depth,
>> +                     bool top_is_return_address)
>> +{
>> +     int i;
>> +
>> +     printf("\tSTACK: " );
>> +     for (i = 0; i < depth; i++) {
>> +             int offset = -1;
>> +             if (i == 0 && !top_is_return_address)
>> +                     offset = 0;
>> +             printf(" %lx", stack[i] + offset);
>
> I'm afraid I don't understand the use of offset here. I fear it may
> be x86-ness leaked into the arch-neutral print_stack().

It's not x86-ness, but it's certainly not self explanatory :-) Return
addresses are stored on the stack. Thus walk_stack returns an array of
pointers to instructions that we'll *return to* when the stack
unwinds. If you give those return addresses to addr2line, you'll get
the line numbers of the code following function calls. For example,

1: void foo(void) {
2:   dump_current_stack();
3:   return;
4: }
5: int main(void) {
6:   foo();
7:   return 0;
8: }

addr2line would print lines 3 & 7, i.e., the return addresses! With
offset = -1, the addresses of some code within call instructions (or
some other code generated to effect the function calls) are printed,
and add2line prints lines 6 & 2.

The special case of offset = 0 is for when the first address isn't the
return address but the address of the faulting instruction, which is
the case for unhandled exceptions. I.e., iret re-tries the interrupted
/ faulting instruction. I suppose I could've just done stack[0] = ip +
1 in dump_stack then unconditionally subtracted 1 in print_stack and
gotten rid of the top_is_return_address argument. Yes, I'll do that
since it'll be cleaner on all accounts. I'll also change the name of
the argument from "stack" to "return_addrs".

One disadvantage of all of this address twiddling is that the stack
will look like nonsense for people not using the pretty printer
because the'll see addresses in the middle of call instructions. I
could do the subtraction in the pretty printer, but then I'd have to
annotate the stack[0] = ip case somehow. Perhaps something like:

    STACK: @ip ret ret ret

That's pretty simple and most useful everybody. Let me know what you think Drew.

>
>> +     }
>> +     printf("\n");
>> +}
>> +
>> +#define MAX_DEPTH 10
>> +
>> +void dump_stack(unsigned long ip, unsigned long bp)
>> +{
>> +     unsigned long stack[MAX_DEPTH];
>> +     int depth;
>> +
>> +     stack[0] = ip;
>> +     depth = walk_stack(bp, &stack[1], MAX_DEPTH - 1);
>> +     print_stack(stack, depth + 1, false);
>> +}
>
> When would dump_stack() be useful, as opposed to dump_current_stack?

Exception handler.

>
>> +
>> +void dump_current_stack(void)
>> +{
>> +     unsigned long stack[MAX_DEPTH];
>> +     int depth;
>> +
>> +     depth = walk_stack((unsigned long)__builtin_frame_address(1), stack,
>> +                        MAX_DEPTH);
>> +     print_stack(stack, depth, true);
>> +}
>> diff --git a/lib/x86/dump_stack.c b/lib/x86/dump_stack.c
>> new file mode 100644
>> index 0000000..6e9d126
>> --- /dev/null
>> +++ b/lib/x86/dump_stack.c
>> @@ -0,0 +1,24 @@
>> +#include "libcflat.h"
>> +
>> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
>> +{
>> +     static int walking;
>> +     int depth = 0;
>> +     unsigned long *frame = (unsigned long *) bp;
>> +
>> +     if (walking) {
>> +             printf("RECURSIVE STACK WALK!!!\n");
>> +             return 0;
>> +     }
>> +     walking = 1;
>> +
>> +     for (depth = 0; depth < max_depth; depth++) {
>> +             stack[depth] = frame[1];
>> +             if (stack[depth] == 0)
>> +                     break;
>> +             frame = (unsigned long *) frame[0];
>> +     }
>> +
>> +     walking = 0;
>> +     return depth;
>> +}
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index 2ce6494..694dea0 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -30,6 +30,7 @@ cflatobjs += lib/powerpc/io.o
>>  cflatobjs += lib/powerpc/hcall.o
>>  cflatobjs += lib/powerpc/setup.o
>>  cflatobjs += lib/powerpc/rtas.o
>> +cflatobjs += lib/powerpc/dump_stack.o
>>
>>  FLATLIBS = $(libcflat) $(LIBFDT_archive)
>>  %.elf: CFLAGS += $(arch_CFLAGS)
>> diff --git a/x86/Makefile.common b/x86/Makefile.common
>> index 3a14fea..d7c7eab 100644
>> --- a/x86/Makefile.common
>> +++ b/x86/Makefile.common
>> @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
>>  cflatobjs += lib/x86/desc.o
>>  cflatobjs += lib/x86/isr.o
>>  cflatobjs += lib/x86/acpi.o
>> +cflatobjs += lib/x86/dump_stack.o
>>
>>  $(libcflat): LDFLAGS += -nostdlib
>>  $(libcflat): CFLAGS += -ffreestanding -I lib
>> @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
>>  CFLAGS += -m$(bits)
>>  CFLAGS += -O1
>>
>> +# dump_stack.o relies on frame pointers.
>> +KEEP_FRAME_POINTER := y
>> +
>>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>>
>>  FLATLIBS = lib/libcflat.a $(libgcc)
>> --
>> 2.7.0.rc3.207.g0ac5344
>
> Thanks,
> drew

Thanks for the review!

Re: [kvm-unit-tests v2 5/8] lib: backtrace printing

[Andrew Jones]

On Thu, Mar 03, 2016 at 09:01:03AM -0800, Peter Feiner wrote:
> On Thu, Mar 3, 2016 at 1:17 AM, Andrew Jones <drjones@redhat.com> wrote:
> > On Wed, Mar 02, 2016 at 05:09:35PM -0800, Peter Feiner wrote:
> >> Functions to walk stack and print backtrace. The stack's unadorned as
> >>
> >>       STACK: addr addr addr ...
> >>
> >> A follow-up patch post-processes the output to pretty-print the stack.
> >>
> >> Stack walker is just a stub on arm and ppc.
> >>
> >> Signed-off-by: Peter Feiner <pfeiner@google.com>
> >> ---
> >>  Makefile                 |  3 ++-
> >>  arm/Makefile.common      |  1 +
> >>  lib/arm/dump_stack.c     |  6 ++++++
> >>  lib/libcflat.h           |  4 ++++
> >>  lib/powerpc/dump_stack.c |  6 ++++++
> >>  lib/printf.c             | 37 +++++++++++++++++++++++++++++++++++++
> >>  lib/x86/dump_stack.c     | 24 ++++++++++++++++++++++++
> >>  powerpc/Makefile.common  |  1 +
> >>  x86/Makefile.common      |  4 ++++
> >>  9 files changed, 85 insertions(+), 1 deletion(-)
> >>  create mode 100644 lib/arm/dump_stack.c
> >>  create mode 100644 lib/powerpc/dump_stack.c
> >>  create mode 100644 lib/x86/dump_stack.c
> >>
> >> diff --git a/Makefile b/Makefile
> >> index ddba941..40ea4ec 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> >>
> >>  CFLAGS += -g
> >>  CFLAGS += $(autodepend-flags) -Wall
> >> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
> >> +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> >> +CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
> >>  CFLAGS += $(call cc-option, -fno-stack-protector, "")
> >>  CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
> >>
> >> diff --git a/arm/Makefile.common b/arm/Makefile.common
> >> index dd3a0ca..054bdee 100644
> >> --- a/arm/Makefile.common
> >> +++ b/arm/Makefile.common
> >> @@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
> >>  cflatobjs += lib/arm/bitops.o
> >>  cflatobjs += lib/arm/psci.o
> >>  cflatobjs += lib/arm/smp.o
> >> +cflatobjs += lib/arm/dump_stack.o
> >>
> >>  libeabi = lib/arm/libeabi.a
> >>  eabiobjs = lib/arm/eabi_compat.o
> >> diff --git a/lib/arm/dump_stack.c b/lib/arm/dump_stack.c
> >> new file mode 100644
> >> index 0000000..528ba63
> >> --- /dev/null
> >> +++ b/lib/arm/dump_stack.c
> >> @@ -0,0 +1,6 @@
> >> +#include "libcflat.h"
> >> +
> >> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
> >> +{
> >> +     return 0;
> >> +}
> >> diff --git a/lib/libcflat.h b/lib/libcflat.h
> >> index 1f0049c..42c94df 100644
> >> --- a/lib/libcflat.h
> >> +++ b/lib/libcflat.h
> >> @@ -65,6 +65,10 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> >>  extern void report_abort(const char *msg_fmt, ...);
> >>  extern int report_summary(void);
> >>
> >> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth);
> >> +void dump_stack(unsigned long ip, unsigned long bp);
> >> +void dump_current_stack(void);
> >
> > The inputs are a bit x86ish. On ARM, for example, we call them pc and
> > fp. Acutally, how about we implement backtrace(3) instead? And then wrap
> > print_current_stack(void) around that.
> 
> I'm indifferent w.r.t. input names. I can change them to void
> *instruction and void *frame.
> 
> I didn't go with backtrace() because I wanted to give the frame
> pointer as an argument. The frame pointer argument is essential for
> dumping stacks of unhandled exceptions. See the 6th patch in the
> series, "
> x86: lib: debug dump on unhandled exceptions".
> 
> > Also, instead of building on a single arch function, walk_stack, how about
> > two
> >
> >   void *arch_frame_address(unsigned int level);
> >   void *arch_return_address(void *frame, unsigned int level);
> 
> This would entail O(level^2) frame pointer traversals to print a stack
> trace. The extra CPU overhead doesn't bother me *too much*, but the
> inevitable debugging burden does. Every once and a while, I have to
> stick a printf() in walk_stack to see where a frame pointer is causing
> a #PF :-) So I'd like to stick with a function that returns an array
> of pointers.
> 
> I can call it
> 
> int arch_return_addresses(const void *frame, void **addresses, int max_depth);
> 
> (is the arch_ prefix necessary given that there's no non-arch_ variant?)

yeah, you're right, arch_ isn't really necessary here.

> 
> Here's the best of both worlds:
> 
> /* return addresses starting from the given frame. needs arch-specific code. */
> int backtrace_frame(const void *frame, void **addresses, int max_depth);
> 
> /* return addresses from the current frame. Can use repeated calls to
> __builtin_return_address. */
> int backtrace(void **addresses, int max_depth);
> 
> I'll use backtrace() in dump_current_stack() and backtrace_frame in
> dump_stack(). I'll also rename dump_current_stack to dump_stack and
> dump_stack to dump_frame_stack for some consistency.
> 
> Wow, I should've just written the patch instead of writing all of this
> crap out :-)

:-)

> 
> > The architecture may be able to easily implement these with gcc builtins,
> > or not. I've added *frame to arch_return_address' parameters in case
> > __builtin_return_address(level) doesn't work.
> >
> > And, let's just require the architectures to have the functions, not that
> > they have a dump_stack.c file. If we include lib/asm/<ARCH>/stack.h, or
> > whatever you want to call it, then the arch could just supply some inlines
> > there.
> 
> That's a good suggestion. I'll change things.
> 
> >> +
> >>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> >>
> >>  #define container_of(ptr, type, member) ({                           \
> >> diff --git a/lib/powerpc/dump_stack.c b/lib/powerpc/dump_stack.c
> >> new file mode 100644
> >> index 0000000..528ba63
> >> --- /dev/null
> >> +++ b/lib/powerpc/dump_stack.c
> >> @@ -0,0 +1,6 @@
> >> +#include "libcflat.h"
> >> +
> >> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
> >> +{
> >> +     return 0;
> >> +}
> >> diff --git a/lib/printf.c b/lib/printf.c
> >> index 2aec59a..e97fca9 100644
> >> --- a/lib/printf.c
> >> +++ b/lib/printf.c
> >> @@ -259,3 +259,40 @@ int printf(const char *fmt, ...)
> >>      puts(buf);
> >>      return r;
> >>  }
> >> +
> >> +static void print_stack(unsigned long *stack, int depth,
> >> +                     bool top_is_return_address)
> >> +{
> >> +     int i;
> >> +
> >> +     printf("\tSTACK: " );
> >> +     for (i = 0; i < depth; i++) {
> >> +             int offset = -1;
> >> +             if (i == 0 && !top_is_return_address)
> >> +                     offset = 0;
> >> +             printf(" %lx", stack[i] + offset);
> >
> > I'm afraid I don't understand the use of offset here. I fear it may
> > be x86-ness leaked into the arch-neutral print_stack().
> 
> It's not x86-ness, but it's certainly not self explanatory :-) Return
> addresses are stored on the stack. Thus walk_stack returns an array of
> pointers to instructions that we'll *return to* when the stack
> unwinds. If you give those return addresses to addr2line, you'll get
> the line numbers of the code following function calls. For example,
> 
> 1: void foo(void) {
> 2:   dump_current_stack();
> 3:   return;
> 4: }
> 5: int main(void) {
> 6:   foo();
> 7:   return 0;
> 8: }
> 
> addr2line would print lines 3 & 7, i.e., the return addresses! With
> offset = -1, the addresses of some code within call instructions (or
> some other code generated to effect the function calls) are printed,
> and add2line prints lines 6 & 2.
> 
> The special case of offset = 0 is for when the first address isn't the
> return address but the address of the faulting instruction, which is
> the case for unhandled exceptions. I.e., iret re-tries the interrupted
> / faulting instruction. I suppose I could've just done stack[0] = ip +
> 1 in dump_stack then unconditionally subtracted 1 in print_stack and
> gotten rid of the top_is_return_address argument. Yes, I'll do that
> since it'll be cleaner on all accounts. I'll also change the name of
> the argument from "stack" to "return_addrs".
> 
> One disadvantage of all of this address twiddling is that the stack
> will look like nonsense for people not using the pretty printer
> because the'll see addresses in the middle of call instructions. I
> could do the subtraction in the pretty printer, but then I'd have to
> annotate the stack[0] = ip case somehow. Perhaps something like:
> 
>     STACK: @ip ret ret ret
> 
> That's pretty simple and most useful everybody. Let me know what you think Drew.

Sounds good to me. Thanks!

> 
> >
> >> +     }
> >> +     printf("\n");
> >> +}
> >> +
> >> +#define MAX_DEPTH 10
> >> +
> >> +void dump_stack(unsigned long ip, unsigned long bp)
> >> +{
> >> +     unsigned long stack[MAX_DEPTH];
> >> +     int depth;
> >> +
> >> +     stack[0] = ip;
> >> +     depth = walk_stack(bp, &stack[1], MAX_DEPTH - 1);
> >> +     print_stack(stack, depth + 1, false);
> >> +}
> >
> > When would dump_stack() be useful, as opposed to dump_current_stack?
> 
> Exception handler.
> 
> >
> >> +
> >> +void dump_current_stack(void)
> >> +{
> >> +     unsigned long stack[MAX_DEPTH];
> >> +     int depth;
> >> +
> >> +     depth = walk_stack((unsigned long)__builtin_frame_address(1), stack,
> >> +                        MAX_DEPTH);
> >> +     print_stack(stack, depth, true);
> >> +}
> >> diff --git a/lib/x86/dump_stack.c b/lib/x86/dump_stack.c
> >> new file mode 100644
> >> index 0000000..6e9d126
> >> --- /dev/null
> >> +++ b/lib/x86/dump_stack.c
> >> @@ -0,0 +1,24 @@
> >> +#include "libcflat.h"
> >> +
> >> +int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
> >> +{
> >> +     static int walking;
> >> +     int depth = 0;
> >> +     unsigned long *frame = (unsigned long *) bp;
> >> +
> >> +     if (walking) {
> >> +             printf("RECURSIVE STACK WALK!!!\n");
> >> +             return 0;
> >> +     }
> >> +     walking = 1;
> >> +
> >> +     for (depth = 0; depth < max_depth; depth++) {
> >> +             stack[depth] = frame[1];
> >> +             if (stack[depth] == 0)
> >> +                     break;
> >> +             frame = (unsigned long *) frame[0];
> >> +     }
> >> +
> >> +     walking = 0;
> >> +     return depth;
> >> +}
> >> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> >> index 2ce6494..694dea0 100644
> >> --- a/powerpc/Makefile.common
> >> +++ b/powerpc/Makefile.common
> >> @@ -30,6 +30,7 @@ cflatobjs += lib/powerpc/io.o
> >>  cflatobjs += lib/powerpc/hcall.o
> >>  cflatobjs += lib/powerpc/setup.o
> >>  cflatobjs += lib/powerpc/rtas.o
> >> +cflatobjs += lib/powerpc/dump_stack.o
> >>
> >>  FLATLIBS = $(libcflat) $(LIBFDT_archive)
> >>  %.elf: CFLAGS += $(arch_CFLAGS)
> >> diff --git a/x86/Makefile.common b/x86/Makefile.common
> >> index 3a14fea..d7c7eab 100644
> >> --- a/x86/Makefile.common
> >> +++ b/x86/Makefile.common
> >> @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
> >>  cflatobjs += lib/x86/desc.o
> >>  cflatobjs += lib/x86/isr.o
> >>  cflatobjs += lib/x86/acpi.o
> >> +cflatobjs += lib/x86/dump_stack.o
> >>
> >>  $(libcflat): LDFLAGS += -nostdlib
> >>  $(libcflat): CFLAGS += -ffreestanding -I lib
> >> @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
> >>  CFLAGS += -m$(bits)
> >>  CFLAGS += -O1
> >>
> >> +# dump_stack.o relies on frame pointers.
> >> +KEEP_FRAME_POINTER := y
> >> +
> >>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
> >>
> >>  FLATLIBS = lib/libcflat.a $(libgcc)
> >> --
> >> 2.7.0.rc3.207.g0ac5344
> >
> > Thanks,
> > drew
> 
> Thanks for the review!
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests v2 6/8] x86: lib: debug dump on unhandled exceptions

[Peter Feiner]

Prints the stack trace and registers.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/x86/desc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 1a80c01..59c34b7 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -38,6 +38,67 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
+static const char* exception_mnemonic(int vector)
+{
+	switch(vector) {
+	case 0: return "#DE";
+	case 1: return "#DB";
+	case 2: return "#NMI";
+	case 3: return "#BP";
+	case 4: return "#OF";
+	case 5: return "#BR";
+	case 6: return "#UD";
+	case 7: return "#NM";
+	case 8: return "#DF";
+	case 10: return "#TS";
+	case 11: return "#NP";
+	case 12: return "#SS";
+	case 13: return "#GP";
+	case 14: return "#PF";
+	case 16: return "#MF";
+	case 17: return "#AC";
+	case 18: return "#MC";
+	case 19: return "#XM";
+	default: return "#??";
+	}
+}
+
+static void unhandled_exception(struct ex_regs *regs, bool cpu)
+{
+	printf("Unhandled %sexception %ld %s at ip %016lx\n",
+	       cpu ? "cpu " : "", regs->vector,
+	       exception_mnemonic(regs->vector), regs->rip);
+	if (regs->vector == 14)
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
+
+	printf("error_code=%04lx      rflags=%08lx      cs=%08lx\n"
+	       "rax=%016lx rcx=%016lx rdx=%016lx rbx=%016lx\n"
+	       "rbp=%016lx rsi=%016lx rdi=%016lx\n"
+#ifdef __x86_64__
+	       " r8=%016lx  r9=%016lx r10=%016lx r11=%016lx\n"
+	       "r12=%016lx r13=%016lx r14=%016lx r15=%016lx\n"
+#endif
+	       "cr0=%016lx cr2=%016lx cr3=%016lx cr4=%016lx\n"
+#ifdef __x86_64__
+	       "cr8=%016lx\n"
+#endif
+	       ,
+	       regs->error_code, regs->rflags, regs->cs,
+	       regs->rax, regs->rcx, regs->rdx, regs->rbx,
+	       regs->rbp, regs->rsi, regs->rdi,
+#ifdef __x86_64__
+	       regs->r8, regs->r9, regs->r10, regs->r11,
+	       regs->r12, regs->r13, regs->r14, regs->r15,
+#endif
+	       read_cr0(), read_cr2(), read_cr3(), read_cr4()
+#ifdef __x86_64__
+	       , read_cr8()
+#endif
+	);
+	dump_stack(regs->rip, regs->rbp);
+	exit(7);
+}
+
 static void check_exception_table(struct ex_regs *regs)
 {
     struct ex_record *ex;
@@ -53,8 +114,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %d\n", regs->vector);
-    exit(7);
+    unhandled_exception(regs, false);
 }
 
 static void (*exception_handlers[32])(struct ex_regs *regs);
@@ -75,10 +135,7 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %d\n", regs->vector);
-	if (regs->vector == 14)
-		printf("PF at %p addr %p\n", regs->rip, read_cr2());
-	exit(7);
+	unhandled_exception(regs, true);
 }
 
 #define EX(NAME, N) extern char NAME##_fault;	\
-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v2 7/8] lib: dump stack on abort()

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 42c94df..1798716 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -77,10 +77,12 @@ void dump_current_stack(void);
 
 #define assert(cond)							\
 do {									\
-	if (!(cond))							\
+	if (!(cond)) {							\
 		printf("%s:%d: assert failed: %s\n",			\
-		       __FILE__, __LINE__, #cond),			\
+		       __FILE__, __LINE__, #cond);			\
+		dump_current_stack();					\
 		abort();						\
+	}								\
 } while (0)
 
 #endif
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v2 7/8] lib: dump stack on abort()

[Andrew Jones]

Looks like you forgot to update $SUBJECT; s/abort/assert/

drew

On Wed, Mar 02, 2016 at 05:09:37PM -0800, Peter Feiner wrote:
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  lib/libcflat.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 42c94df..1798716 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -77,10 +77,12 @@ void dump_current_stack(void);
>  
>  #define assert(cond)							\
>  do {									\
> -	if (!(cond))							\
> +	if (!(cond)) {							\
>  		printf("%s:%d: assert failed: %s\n",			\
> -		       __FILE__, __LINE__, #cond),			\
> +		       __FILE__, __LINE__, #cond);			\
> +		dump_current_stack();					\
>  		abort();						\
> +	}								\
>  } while (0)
>  
>  #endif
> -- 
> 2.7.0.rc3.207.g0ac5344
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests v2 8/8] scripts: pretty print stack traces

[Peter Feiner]

Optionally pretty print stacks in test.log with 'run_tests.sh -p'.

The script runs addresses through addr2line to get file names and line
numbers.  Also prints out source code lines if files are available.
For example

0x004013f5: ac_test_exec at x86/access.c:818
            }
      >     ac_test_setup_pte(at, pool);
            r = ac_test_do_access(at);
0x004014dd: ac_test_run at x86/access.c:852
                ++tests;
      >         successes += ac_test_exec(&at, &pool);
            } while (ac_test_bump(&at));
0x0040157d: main at x86/access.c:873
            printf("starting test\n\n");
      >     r = ac_test_run();
            return r ? 0 : 1;
0x0040028e: start64 at x86/cstart64.S:206
                lea __argv(%rip), %rsi
      >         call main
                mov %eax, %edi

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 run_tests.sh                   | 10 +++++--
 scripts/pretty_print_stacks.py | 61 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100755 scripts/pretty_print_stacks.py

diff --git a/run_tests.sh b/run_tests.sh
index 2c8af36..13bc089 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 
 verbose="no"
+pretty="no"
 
 if [ ! -f config.mak ]; then
     echo "run ./configure && make first. See ./configure -h"
@@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
     -g: Only execute tests in the given group
     -h: Output this help text
     -v: Enables verbose mode
+    -p: Pretty print stack traces in test.log
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
 specify the appropriate qemu binary for ARCH-run.
@@ -27,8 +29,9 @@ EOF
 
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
+log_redir=">> test.log"
 
-while getopts "g:hv" opt; do
+while getopts "g:hvp" opt; do
     case $opt in
         g)
             only_group=$OPTARG
@@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
         v)
             verbose="yes"
             ;;
+        p)
+           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
+           ;;
         *)
             exit
             ;;
     esac
 done
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 echo > test.log
diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
new file mode 100755
index 0000000..49564bd
--- /dev/null
+++ b/scripts/pretty_print_stacks.py
@@ -0,0 +1,61 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+import sys
+
+if len(sys.argv) != 2:
+    sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
+    sys.exit(1)
+
+binary = sys.argv[1]
+
+# Subvert output buffering.
+def puts(string):
+    sys.stdout.write(string)
+    sys.stdout.flush()
+
+try:
+    while True:
+        # Subvert input buffering.
+        line = sys.stdin.readline()
+        if line == '':
+            break
+
+        if not line.strip().startswith('STACK:'):
+            puts(line)
+            continue
+
+        addrs = line.split()[1:]
+        # Output like this:
+        #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
+        #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
+        cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+        cmd.extend(addrs)
+
+        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        out, err = p.communicate()
+        if p.returncode != 0:
+            puts(line)
+            continue
+
+        for line in out.splitlines():
+            m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
+            if m is None:
+                puts('%s\n' % line)
+                continue
+
+            head, path, line, tail = m.groups()
+            line = int(line)
+            puts('%s at %s:%d%s\n' % (head, path, line, tail))
+            try:
+                lines = open(path).readlines()
+            except IOError:
+                continue
+            if line > 1:
+                puts('        %s\n' % lines[line - 2].rstrip())
+            puts('      > %s\n' % lines[line - 1].rstrip())
+            if line < len(lines):
+                puts('        %s\n' % lines[line].rstrip())
+except:
+    sys.exit(1)
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v2 8/8] scripts: pretty print stack traces

[Andrew Jones]

On Wed, Mar 02, 2016 at 05:09:38PM -0800, Peter Feiner wrote:
> Optionally pretty print stacks in test.log with 'run_tests.sh -p'.
> 
> The script runs addresses through addr2line to get file names and line
> numbers.  Also prints out source code lines if files are available.
> For example
> 
> 0x004013f5: ac_test_exec at x86/access.c:818
>             }
>       >     ac_test_setup_pte(at, pool);
>             r = ac_test_do_access(at);
> 0x004014dd: ac_test_run at x86/access.c:852
>                 ++tests;
>       >         successes += ac_test_exec(&at, &pool);
>             } while (ac_test_bump(&at));
> 0x0040157d: main at x86/access.c:873
>             printf("starting test\n\n");
>       >     r = ac_test_run();
>             return r ? 0 : 1;
> 0x0040028e: start64 at x86/cstart64.S:206
>                 lea __argv(%rip), %rsi
>       >         call main
>                 mov %eax, %edi
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  run_tests.sh                   | 10 +++++--
>  scripts/pretty_print_stacks.py | 61 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 2 deletions(-)
>  create mode 100755 scripts/pretty_print_stacks.py
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 2c8af36..13bc089 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -1,6 +1,7 @@
>  #!/bin/bash
>  
>  verbose="no"
> +pretty="no"
>  
>  if [ ! -f config.mak ]; then
>      echo "run ./configure && make first. See ./configure -h"
> @@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
>      -g: Only execute tests in the given group
>      -h: Output this help text
>      -v: Enables verbose mode
> +    -p: Pretty print stack traces in test.log
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>  specify the appropriate qemu binary for ARCH-run.
> @@ -27,8 +29,9 @@ EOF
>  
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
> +log_redir=">> test.log"
>  
> -while getopts "g:hv" opt; do
> +while getopts "g:hvp" opt; do
>      case $opt in
>          g)
>              only_group=$OPTARG
> @@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
>          v)
>              verbose="yes"
>              ;;
> +        p)
> +           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> +           ;;
>          *)
>              exit
>              ;;
>      esac
>  done
>  
> -RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> +RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
>  config=$TEST_DIR/unittests.cfg
>  rm -f test.log
>  echo > test.log
> diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
> new file mode 100755
> index 0000000..49564bd
> --- /dev/null
> +++ b/scripts/pretty_print_stacks.py
> @@ -0,0 +1,61 @@
> +#!/usr/bin/env python
> +
> +import re
> +import subprocess
> +import sys
> +
> +if len(sys.argv) != 2:
> +    sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
> +    sys.exit(1)
> +
> +binary = sys.argv[1]
> +
> +# Subvert output buffering.
> +def puts(string):
> +    sys.stdout.write(string)
> +    sys.stdout.flush()
> +
> +try:
> +    while True:
> +        # Subvert input buffering.
> +        line = sys.stdin.readline()
> +        if line == '':
> +            break
> +
> +        if not line.strip().startswith('STACK:'):
> +            puts(line)
> +            continue
> +
> +        addrs = line.split()[1:]
> +        # Output like this:
> +        #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
> +        #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
> +        cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
> +        cmd.extend(addrs)
> +
> +        p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> +        out, err = p.communicate()
> +        if p.returncode != 0:
> +            puts(line)
> +            continue
> +
> +        for line in out.splitlines():
> +            m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
> +            if m is None:
> +                puts('%s\n' % line)
> +                continue
> +
> +            head, path, line, tail = m.groups()
> +            line = int(line)
> +            puts('%s at %s:%d%s\n' % (head, path, line, tail))
> +            try:
> +                lines = open(path).readlines()
> +            except IOError:
> +                continue
> +            if line > 1:
> +                puts('        %s\n' % lines[line - 2].rstrip())
> +            puts('      > %s\n' % lines[line - 1].rstrip())
> +            if line < len(lines):
> +                puts('        %s\n' % lines[line].rstrip())
> +except:
> +    sys.exit(1)
> -- 
> 2.7.0.rc3.207.g0ac5344

I think this probably could have been written in bash, but I'm not
overly opposed to adding python scripts, as I think we're pretty safe
adding that dependency, particularly when its use can be disabled.

wrt to enabling/disabling it though, how about we create a new configure
command line option instead. One that defaults to on, if the architecture
supports pretty-printing, but can be switched off. Then, run_tests.sh,
which sources config.mak, can just turn pretty printing on by default
when it's available, rather than introducing the -p option, which I
think we'd always want to use when we can.

Thanks,
drew

Re: [kvm-unit-tests v2 0/8] Debugging aids

[Paolo Bonzini]

On 03/03/2016 02:09, Peter Feiner wrote:
> Some aids for debugging: stack dumping, register dumping, and printing
> failing assertion conditions.
> 
> N.B. I haven't tried compiling on arm or ppc.
> 
> v2:
> 	* Fixed a bunch tests on x86 that broke with
> 	  -fno-omit-frame-pointer. 
> 	* Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
> 	  will have similar problems that I fixed for x86.

ARM and PPC have fewer tests and even less inline assembly, so hopefully
they're safe.  But, I agree with adding support one architecture at a time.

Drew had comments so for now I'm applying patches 1-4.  Thanks a lot for
them!

Paolo

[kvm-unit-tests v3 0/4] Debugging aids

[Peter Feiner]

Some aids for debugging: stack dumping, register dumping, and printing
failing assertion conditions.

I _have_ compiled on ppc64 and arm64 :-)

v3:
	* Renamed functions to match standard backtrace functions
	* Added asm-generic stack walker that uses __builtin_return_address.
	* Moved offset out of libcflat and into pretty printer
	* Made pretty printer more robust w.r.t. errors: now it just
	  continues printing.

v2:
        * Fixed a bunch tests on x86 that broke with
          -fno-omit-frame-pointer.
        * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
          will have similar problems that I fixed for x86.

Peter Feiner (4):
  lib: backtrace printing
  x86: lib: debug dump on unhandled exceptions
  lib: dump stack on failed assert()
  scripts: pretty print stack traces

 Makefile                       |  3 +-
 arm/Makefile.common            |  1 +
 lib/arm/asm/stack.h            |  1 +
 lib/arm64/asm/stack.h          |  1 +
 lib/asm-generic/stack.c        | 53 +++++++++++++++++++++++++
 lib/asm-generic/stack.h        | 14 +++++++
 lib/libcflat.h                 |  9 ++++-
 lib/powerpc/asm/stack.h        |  1 +
 lib/ppc64/asm/stack.h          |  1 +
 lib/printf.c                   | 42 ++++++++++++++++++++
 lib/x86/asm/stack.c            | 25 ++++++++++++
 lib/x86/asm/stack.h            | 12 ++++++
 lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++++---
 powerpc/Makefile.common        |  1 +
 run_tests.sh                   | 10 ++++-
 scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
 x86/Makefile.common            |  4 ++
 17 files changed, 325 insertions(+), 11 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/asm-generic/stack.c
 create mode 100644 lib/asm-generic/stack.h
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/x86/asm/stack.c
 create mode 100644 lib/x86/asm/stack.h
 create mode 100755 scripts/pretty_print_stacks.py

-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v3 1/4] lib: backtrace printing

[Peter Feiner]

Functions to walk stack and print backtrace. The stack's unadorned as

	STACK: [@]addr addr addr ...

where the optional @ indicates that addr isn't a return address.

A follow-up patch post-processes the output to pretty-print the stack.

Frame stack walker is just a stub on arm and ppc.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 Makefile                |  3 ++-
 arm/Makefile.common     |  1 +
 lib/arm/asm/stack.h     |  1 +
 lib/arm64/asm/stack.h   |  1 +
 lib/asm-generic/stack.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/asm-generic/stack.h | 14 +++++++++++++
 lib/libcflat.h          |  3 +++
 lib/powerpc/asm/stack.h |  1 +
 lib/ppc64/asm/stack.h   |  1 +
 lib/printf.c            | 42 +++++++++++++++++++++++++++++++++++++++
 lib/x86/asm/stack.c     | 25 +++++++++++++++++++++++
 lib/x86/asm/stack.h     | 12 +++++++++++
 powerpc/Makefile.common |  1 +
 x86/Makefile.common     |  4 ++++
 14 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/asm-generic/stack.c
 create mode 100644 lib/asm-generic/stack.h
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/x86/asm/stack.c
 create mode 100644 lib/x86/asm/stack.h

diff --git a/Makefile b/Makefile
index 72e6711..536f2e6 100644
--- a/Makefile
+++ b/Makefile
@@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
 
 CFLAGS += -g
 CFLAGS += $(autodepend-flags) -Wall
-CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
+frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
+CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
 CFLAGS += $(call cc-option, -fno-stack-protector, "")
 CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
 
diff --git a/arm/Makefile.common b/arm/Makefile.common
index dd3a0ca..ede8851 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
 cflatobjs += lib/arm/bitops.o
 cflatobjs += lib/arm/psci.o
 cflatobjs += lib/arm/smp.o
+cflatobjs += lib/asm-generic/stack.o
 
 libeabi = lib/arm/libeabi.a
 eabiobjs = lib/arm/eabi_compat.o
diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
new file mode 100644
index 0000000..f101d03
--- /dev/null
+++ b/lib/arm/asm/stack.h
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>
diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
new file mode 100644
index 0000000..f101d03
--- /dev/null
+++ b/lib/arm64/asm/stack.h
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>
diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c
new file mode 100644
index 0000000..93219cb
--- /dev/null
+++ b/lib/asm-generic/stack.c
@@ -0,0 +1,53 @@
+#include <libcflat.h>
+#include <asm-generic/stack.h>
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	void *addr;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	/* __builtin_return_address requires a compile-time constant argument */
+#define GET_RETURN_ADDRESS(i)						\
+	if (max_depth == i)						\
+		goto done;						\
+	addr = __builtin_return_address(i);				\
+	if (!addr)							\
+		goto done;						\
+	return_addrs[i] = __builtin_extract_return_addr(addr);		\
+	depth = i + 1;							\
+
+	GET_RETURN_ADDRESS(0)
+	GET_RETURN_ADDRESS(1)
+	GET_RETURN_ADDRESS(2)
+	GET_RETURN_ADDRESS(3)
+	GET_RETURN_ADDRESS(4)
+	GET_RETURN_ADDRESS(5)
+	GET_RETURN_ADDRESS(6)
+	GET_RETURN_ADDRESS(7)
+	GET_RETURN_ADDRESS(8)
+	GET_RETURN_ADDRESS(9)
+	GET_RETURN_ADDRESS(10)
+	GET_RETURN_ADDRESS(11)
+	GET_RETURN_ADDRESS(12)
+	GET_RETURN_ADDRESS(13)
+	GET_RETURN_ADDRESS(14)
+	GET_RETURN_ADDRESS(15)
+	GET_RETURN_ADDRESS(16)
+	GET_RETURN_ADDRESS(17)
+	GET_RETURN_ADDRESS(18)
+	GET_RETURN_ADDRESS(19)
+	GET_RETURN_ADDRESS(20)
+
+#undef GET_RETURN_ADDRESS
+
+done:
+	walking = 0;
+	return depth;
+}
diff --git a/lib/asm-generic/stack.h b/lib/asm-generic/stack.h
new file mode 100644
index 0000000..381fce4
--- /dev/null
+++ b/lib/asm-generic/stack.h
@@ -0,0 +1,14 @@
+#ifndef _ASMGENERIC_STACK_H_
+#define _ASMGENERIC_STACK_H_
+
+static inline int
+backtrace_frame(const void *frame __attribute__((unused)),
+		const void **return_addrs __attribute__((unused)),
+		int max_depth __attribute__((unused)))
+{
+	return 0;
+}
+
+int backtrace(const void **return_addrs, int max_depth);
+
+#endif
diff --git a/lib/libcflat.h b/lib/libcflat.h
index b58a8a1..55bddca 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+extern void dump_stack(void);
+extern void dump_frame_stack(const void *instruction, const void *frame);
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\
diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
new file mode 100644
index 0000000..f101d03
--- /dev/null
+++ b/lib/powerpc/asm/stack.h
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>
diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
new file mode 100644
index 0000000..f101d03
--- /dev/null
+++ b/lib/ppc64/asm/stack.h
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>
diff --git a/lib/printf.c b/lib/printf.c
index 2aec59a..096e175 100644
--- a/lib/printf.c
+++ b/lib/printf.c
@@ -1,4 +1,5 @@
 #include "libcflat.h"
+#include "asm/stack.h"
 
 #define BUFSZ 2000
 
@@ -259,3 +260,44 @@ int printf(const char *fmt, ...)
     puts(buf);
     return r;
 }
+
+static void print_stack(const void **return_addrs, int depth,
+			bool top_is_return_address)
+{
+	int i = 0;
+
+	printf("\tSTACK:");
+
+	/* @addr indicates a non-return address, as expected by the stack
+	 * pretty printer script. */
+	if (depth > 0 && !top_is_return_address) {
+		printf(" @%lx", (unsigned long) return_addrs[0]);
+		i++;
+	}
+
+	for (; i < depth; i++) {
+		printf(" %lx", (unsigned long) return_addrs[i]);
+	}
+	printf("\n");
+}
+
+#define MAX_DEPTH 10
+
+void dump_stack(void)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	depth = backtrace(return_addrs, MAX_DEPTH);
+	print_stack(return_addrs, depth, true);
+}
+
+void dump_frame_stack(const void *instruction, const void *frame)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	return_addrs[0] = instruction;
+	depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1);
+	print_stack(return_addrs, depth + 1, false);
+}
diff --git a/lib/x86/asm/stack.c b/lib/x86/asm/stack.c
new file mode 100644
index 0000000..b30b3b1
--- /dev/null
+++ b/lib/x86/asm/stack.c
@@ -0,0 +1,25 @@
+#include <libcflat.h>
+#include <asm/stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	const unsigned long *bp = (unsigned long *) frame;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	for (depth = 0; depth < max_depth; depth++) {
+		return_addrs[depth] = (void *) bp[1];
+		if (return_addrs[depth] == 0)
+			break;
+		bp = (unsigned long *) bp[0];
+	}
+
+	walking = 0;
+	return depth;
+}
diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
new file mode 100644
index 0000000..c43bb96
--- /dev/null
+++ b/lib/x86/asm/stack.h
@@ -0,0 +1,12 @@
+#ifndef _X86ASM_STACK_H_
+#define _X86ASM_STACK_H_
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
+
+static inline int backtrace(const void **return_addrs, int max_depth)
+{
+	return backtrace_frame(__builtin_frame_address(0), return_addrs,
+			       max_depth);
+}
+
+#endif
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 424983e..ea8f669 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
 cflatobjs += lib/powerpc/hcall.o
 cflatobjs += lib/powerpc/setup.o
 cflatobjs += lib/powerpc/rtas.o
+cflatobjs += lib/asm-generic/stack.o
 
 FLATLIBS = $(libcflat) $(LIBFDT_archive)
 %.elf: CFLAGS += $(arch_CFLAGS)
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 3a14fea..c4750f0 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
 cflatobjs += lib/x86/desc.o
 cflatobjs += lib/x86/isr.o
 cflatobjs += lib/x86/acpi.o
+cflatobjs += lib/x86/asm/stack.o
 
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I lib
@@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
 CFLAGS += -m$(bits)
 CFLAGS += -O1
 
+# dump_stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
 libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
 
 FLATLIBS = lib/libcflat.a $(libgcc)
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v3 1/4] lib: backtrace printing

[Andrew Jones]

On Thu, Mar 03, 2016 at 12:48:33PM -0800, Peter Feiner wrote:
> Functions to walk stack and print backtrace. The stack's unadorned as
> 
> 	STACK: [@]addr addr addr ...
> 
> where the optional @ indicates that addr isn't a return address.
> 
> A follow-up patch post-processes the output to pretty-print the stack.
> 
> Frame stack walker is just a stub on arm and ppc.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  Makefile                |  3 ++-
>  arm/Makefile.common     |  1 +
>  lib/arm/asm/stack.h     |  1 +
>  lib/arm64/asm/stack.h   |  1 +
>  lib/asm-generic/stack.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/asm-generic/stack.h | 14 +++++++++++++
>  lib/libcflat.h          |  3 +++
>  lib/powerpc/asm/stack.h |  1 +
>  lib/ppc64/asm/stack.h   |  1 +
>  lib/printf.c            | 42 +++++++++++++++++++++++++++++++++++++++
>  lib/x86/asm/stack.c     | 25 +++++++++++++++++++++++
>  lib/x86/asm/stack.h     | 12 +++++++++++
>  powerpc/Makefile.common |  1 +
>  x86/Makefile.common     |  4 ++++
>  14 files changed, 161 insertions(+), 1 deletion(-)
>  create mode 100644 lib/arm/asm/stack.h
>  create mode 100644 lib/arm64/asm/stack.h
>  create mode 100644 lib/asm-generic/stack.c
>  create mode 100644 lib/asm-generic/stack.h
>  create mode 100644 lib/powerpc/asm/stack.h
>  create mode 100644 lib/ppc64/asm/stack.h
>  create mode 100644 lib/x86/asm/stack.c
>  create mode 100644 lib/x86/asm/stack.h

It's not usual to put C files in the asm directories, at least Linux
doesn't do that. So I'd prefer

lib/stack.c: the common backtrace()
lib/<ARCH>/stack.c: arch specific backtrace* functions

I'm tempted to suggest that print_stack() and the dump*stack
functions also go in lib/stack.c, instead of lib/printf.c, but
then we need some

#ifndef backtrace
#define backtrace backtrace
int backtrace(...)
{
}
#endif

ugliness to allow the common backtrace() to be overridden. Although,
it might still be better to move those functions out of printf,
as I don't feel they really fit there.

> 
> diff --git a/Makefile b/Makefile
> index 72e6711..536f2e6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -42,7 +42,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
>  
>  CFLAGS += -g
>  CFLAGS += $(autodepend-flags) -Wall
> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
> +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> +CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
>  CFLAGS += $(call cc-option, -fno-stack-protector, "")
>  CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
>  
> diff --git a/arm/Makefile.common b/arm/Makefile.common
> index dd3a0ca..ede8851 100644
> --- a/arm/Makefile.common
> +++ b/arm/Makefile.common
> @@ -39,6 +39,7 @@ cflatobjs += lib/arm/mmu.o
>  cflatobjs += lib/arm/bitops.o
>  cflatobjs += lib/arm/psci.o
>  cflatobjs += lib/arm/smp.o
> +cflatobjs += lib/asm-generic/stack.o
>  
>  libeabi = lib/arm/libeabi.a
>  eabiobjs = lib/arm/eabi_compat.o
> diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
> new file mode 100644
> index 0000000..f101d03
> --- /dev/null
> +++ b/lib/arm/asm/stack.h
> @@ -0,0 +1 @@
> +#include <asm-generic/stack.h>
> diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
> new file mode 100644
> index 0000000..f101d03
> --- /dev/null
> +++ b/lib/arm64/asm/stack.h
> @@ -0,0 +1 @@
> +#include <asm-generic/stack.h>
> diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c
> new file mode 100644
> index 0000000..93219cb
> --- /dev/null
> +++ b/lib/asm-generic/stack.c
> @@ -0,0 +1,53 @@
> +#include <libcflat.h>
> +#include <asm-generic/stack.h>

nit: including asm-generic/stack.h doesn't appear to be necessary.

Actually, if there was something needed from stack.h, then we should
probably get it from asm/stack.h, in case the arch wanted to override
it.

> +
> +int backtrace(const void **return_addrs, int max_depth)
> +{
> +	static int walking;
> +	int depth = 0;
> +	void *addr;
> +
> +	if (walking) {
> +		printf("RECURSIVE STACK WALK!!!\n");
> +		return 0;
> +	}
> +	walking = 1;
> +
> +	/* __builtin_return_address requires a compile-time constant argument */
> +#define GET_RETURN_ADDRESS(i)						\
> +	if (max_depth == i)						\
> +		goto done;						\
> +	addr = __builtin_return_address(i);				\
> +	if (!addr)							\
> +		goto done;						\
> +	return_addrs[i] = __builtin_extract_return_addr(addr);		\
> +	depth = i + 1;							\
> +
> +	GET_RETURN_ADDRESS(0)
> +	GET_RETURN_ADDRESS(1)
> +	GET_RETURN_ADDRESS(2)
> +	GET_RETURN_ADDRESS(3)
> +	GET_RETURN_ADDRESS(4)
> +	GET_RETURN_ADDRESS(5)
> +	GET_RETURN_ADDRESS(6)
> +	GET_RETURN_ADDRESS(7)
> +	GET_RETURN_ADDRESS(8)
> +	GET_RETURN_ADDRESS(9)
> +	GET_RETURN_ADDRESS(10)
> +	GET_RETURN_ADDRESS(11)
> +	GET_RETURN_ADDRESS(12)
> +	GET_RETURN_ADDRESS(13)
> +	GET_RETURN_ADDRESS(14)
> +	GET_RETURN_ADDRESS(15)
> +	GET_RETURN_ADDRESS(16)
> +	GET_RETURN_ADDRESS(17)
> +	GET_RETURN_ADDRESS(18)
> +	GET_RETURN_ADDRESS(19)
> +	GET_RETURN_ADDRESS(20)
> +
> +#undef GET_RETURN_ADDRESS
> +
> +done:
> +	walking = 0;
> +	return depth;
> +}
> diff --git a/lib/asm-generic/stack.h b/lib/asm-generic/stack.h
> new file mode 100644
> index 0000000..381fce4
> --- /dev/null
> +++ b/lib/asm-generic/stack.h
> @@ -0,0 +1,14 @@
> +#ifndef _ASMGENERIC_STACK_H_
> +#define _ASMGENERIC_STACK_H_
> +
> +static inline int
> +backtrace_frame(const void *frame __attribute__((unused)),
> +		const void **return_addrs __attribute__((unused)),
> +		int max_depth __attribute__((unused)))
> +{
> +	return 0;
> +}
> +
> +int backtrace(const void **return_addrs, int max_depth);
> +
> +#endif
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index b58a8a1..55bddca 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
>  extern void report_abort(const char *msg_fmt, ...);
>  extern int report_summary(void);
>  
> +extern void dump_stack(void);
> +extern void dump_frame_stack(const void *instruction, const void *frame);
> +
>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
>  
>  #define container_of(ptr, type, member) ({				\
> diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
> new file mode 100644
> index 0000000..f101d03
> --- /dev/null
> +++ b/lib/powerpc/asm/stack.h
> @@ -0,0 +1 @@
> +#include <asm-generic/stack.h>
> diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
> new file mode 100644
> index 0000000..f101d03
> --- /dev/null
> +++ b/lib/ppc64/asm/stack.h
> @@ -0,0 +1 @@
> +#include <asm-generic/stack.h>
> diff --git a/lib/printf.c b/lib/printf.c
> index 2aec59a..096e175 100644
> --- a/lib/printf.c
> +++ b/lib/printf.c
> @@ -1,4 +1,5 @@
>  #include "libcflat.h"
> +#include "asm/stack.h"
>  
>  #define BUFSZ 2000
>  
> @@ -259,3 +260,44 @@ int printf(const char *fmt, ...)
>      puts(buf);
>      return r;
>  }
> +
> +static void print_stack(const void **return_addrs, int depth,
> +			bool top_is_return_address)
> +{
> +	int i = 0;
> +
> +	printf("\tSTACK:");
> +
> +	/* @addr indicates a non-return address, as expected by the stack
> +	 * pretty printer script. */
> +	if (depth > 0 && !top_is_return_address) {
> +		printf(" @%lx", (unsigned long) return_addrs[0]);
> +		i++;
> +	}
> +
> +	for (; i < depth; i++) {
> +		printf(" %lx", (unsigned long) return_addrs[i]);
> +	}
> +	printf("\n");
> +}
> +
> +#define MAX_DEPTH 10

How about 20, like the common backtrace() supports?

> +
> +void dump_stack(void)
> +{
> +	const void *return_addrs[MAX_DEPTH];
> +	int depth;
> +
> +	depth = backtrace(return_addrs, MAX_DEPTH);
> +	print_stack(return_addrs, depth, true);
> +}
> +
> +void dump_frame_stack(const void *instruction, const void *frame)
> +{
> +	const void *return_addrs[MAX_DEPTH];
> +	int depth;
> +
> +	return_addrs[0] = instruction;
> +	depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1);
> +	print_stack(return_addrs, depth + 1, false);
> +}
> diff --git a/lib/x86/asm/stack.c b/lib/x86/asm/stack.c
> new file mode 100644
> index 0000000..b30b3b1
> --- /dev/null
> +++ b/lib/x86/asm/stack.c
> @@ -0,0 +1,25 @@
> +#include <libcflat.h>
> +#include <asm/stack.h>
> +
> +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
> +{
> +	static int walking;
> +	int depth = 0;
> +	const unsigned long *bp = (unsigned long *) frame;
> +
> +	if (walking) {
> +		printf("RECURSIVE STACK WALK!!!\n");
> +		return 0;
> +	}
> +	walking = 1;
> +
> +	for (depth = 0; depth < max_depth; depth++) {
> +		return_addrs[depth] = (void *) bp[1];
> +		if (return_addrs[depth] == 0)
> +			break;
> +		bp = (unsigned long *) bp[0];
> +	}
> +
> +	walking = 0;
> +	return depth;
> +}
> diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
> new file mode 100644
> index 0000000..c43bb96
> --- /dev/null
> +++ b/lib/x86/asm/stack.h
> @@ -0,0 +1,12 @@
> +#ifndef _X86ASM_STACK_H_
> +#define _X86ASM_STACK_H_
> +
> +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
> +
> +static inline int backtrace(const void **return_addrs, int max_depth)
> +{
> +	return backtrace_frame(__builtin_frame_address(0), return_addrs,
> +			       max_depth);
> +}
> +
> +#endif
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 424983e..ea8f669 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -31,6 +31,7 @@ cflatobjs += lib/powerpc/io.o
>  cflatobjs += lib/powerpc/hcall.o
>  cflatobjs += lib/powerpc/setup.o
>  cflatobjs += lib/powerpc/rtas.o
> +cflatobjs += lib/asm-generic/stack.o
>  
>  FLATLIBS = $(libcflat) $(LIBFDT_archive)
>  %.elf: CFLAGS += $(arch_CFLAGS)
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 3a14fea..c4750f0 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
>  cflatobjs += lib/x86/desc.o
>  cflatobjs += lib/x86/isr.o
>  cflatobjs += lib/x86/acpi.o
> +cflatobjs += lib/x86/asm/stack.o
>  
>  $(libcflat): LDFLAGS += -nostdlib
>  $(libcflat): CFLAGS += -ffreestanding -I lib
> @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
>  CFLAGS += -m$(bits)
>  CFLAGS += -O1
>  
> +# dump_stack.o relies on frame pointers.
> +KEEP_FRAME_POINTER := y
> +
>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>  
>  FLATLIBS = lib/libcflat.a $(libgcc)
> -- 
> 2.7.0.rc3.207.g0ac5344

Other than my desire to keep C out of asm directories, then this
looks good to me.

Thanks,
drew

[kvm-unit-tests v3 2/4] x86: lib: debug dump on unhandled exceptions

[Peter Feiner]

Prints the stack trace and registers.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/x86/desc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 16cbba5..75765ef 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -38,6 +38,67 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
+static const char* exception_mnemonic(int vector)
+{
+	switch(vector) {
+	case 0: return "#DE";
+	case 1: return "#DB";
+	case 2: return "#NMI";
+	case 3: return "#BP";
+	case 4: return "#OF";
+	case 5: return "#BR";
+	case 6: return "#UD";
+	case 7: return "#NM";
+	case 8: return "#DF";
+	case 10: return "#TS";
+	case 11: return "#NP";
+	case 12: return "#SS";
+	case 13: return "#GP";
+	case 14: return "#PF";
+	case 16: return "#MF";
+	case 17: return "#AC";
+	case 18: return "#MC";
+	case 19: return "#XM";
+	default: return "#??";
+	}
+}
+
+static void unhandled_exception(struct ex_regs *regs, bool cpu)
+{
+	printf("Unhandled %sexception %ld %s at ip %016lx\n",
+	       cpu ? "cpu " : "", regs->vector,
+	       exception_mnemonic(regs->vector), regs->rip);
+	if (regs->vector == 14)
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
+
+	printf("error_code=%04lx      rflags=%08lx      cs=%08lx\n"
+	       "rax=%016lx rcx=%016lx rdx=%016lx rbx=%016lx\n"
+	       "rbp=%016lx rsi=%016lx rdi=%016lx\n"
+#ifdef __x86_64__
+	       " r8=%016lx  r9=%016lx r10=%016lx r11=%016lx\n"
+	       "r12=%016lx r13=%016lx r14=%016lx r15=%016lx\n"
+#endif
+	       "cr0=%016lx cr2=%016lx cr3=%016lx cr4=%016lx\n"
+#ifdef __x86_64__
+	       "cr8=%016lx\n"
+#endif
+	       ,
+	       regs->error_code, regs->rflags, regs->cs,
+	       regs->rax, regs->rcx, regs->rdx, regs->rbx,
+	       regs->rbp, regs->rsi, regs->rdi,
+#ifdef __x86_64__
+	       regs->r8, regs->r9, regs->r10, regs->r11,
+	       regs->r12, regs->r13, regs->r14, regs->r15,
+#endif
+	       read_cr0(), read_cr2(), read_cr3(), read_cr4()
+#ifdef __x86_64__
+	       , read_cr8()
+#endif
+	);
+	dump_frame_stack((void*) regs->rip, (void*) regs->rbp);
+	exit(7);
+}
+
 static void check_exception_table(struct ex_regs *regs)
 {
     struct ex_record *ex;
@@ -53,8 +114,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %lu\n", regs->vector);
-    abort();
+    unhandled_exception(regs, false);
 }
 
 static void (*exception_handlers[32])(struct ex_regs *regs);
@@ -75,10 +135,7 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %lu\n", regs->vector);
-	if (regs->vector == 14)
-		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
-	abort();
+	unhandled_exception(regs, true);
 }
 
 #define EX(NAME, N) extern char NAME##_fault;	\
-- 
2.7.0.rc3.207.g0ac5344

[kvm-unit-tests v3 3/4] lib: dump stack on failed assert()

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 55bddca..df50615 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -92,10 +92,12 @@ extern void dump_frame_stack(const void *instruction, const void *frame);
 
 #define assert(cond)							\
 do {									\
-	if (!(cond))							\
+	if (!(cond)) {							\
 		printf("%s:%d: assert failed: %s\n",			\
-		       __FILE__, __LINE__, #cond),			\
+		       __FILE__, __LINE__, #cond);			\
+		dump_stack();						\
 		abort();						\
+	}								\
 } while (0)
 
 #endif
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v3 3/4] lib: dump stack on failed assert()

[Andrew Jones]

On Thu, Mar 03, 2016 at 12:48:35PM -0800, Peter Feiner wrote:
> Signed-off-by: Peter Feiner <pfeiner@google.com>

Reviewed-by: Andrew Jones <drjones@redhat.com>

[kvm-unit-tests v3 4/4] scripts: pretty print stack traces

[Peter Feiner]

Optionally pretty print stacks in test.log with 'run_tests.sh -p'.

The script runs addresses through addr2line to get file names and line
numbers.  Also prints out source code lines if files are available.
For example

0x004013f5: ac_test_exec at x86/access.c:818
            }
      >     ac_test_setup_pte(at, pool);
            r = ac_test_do_access(at);
0x004014dd: ac_test_run at x86/access.c:852
                ++tests;
      >         successes += ac_test_exec(&at, &pool);
            } while (ac_test_bump(&at));
0x0040157d: main at x86/access.c:873
            printf("starting test\n\n");
      >     r = ac_test_run();
            return r ? 0 : 1;
0x0040028e: start64 at x86/cstart64.S:206
                lea __argv(%rip), %rsi
      >         call main
                mov %eax, %edi

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 run_tests.sh                   | 10 ++++-
 scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100755 scripts/pretty_print_stacks.py

diff --git a/run_tests.sh b/run_tests.sh
index 89e8f84..b6f2003 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -1,6 +1,7 @@
 #!/bin/bash
 
 verbose="no"
+pretty="no"
 
 if [ ! -f config.mak ]; then
     echo "run ./configure && make first. See ./configure -h"
@@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
     -g: Only execute tests in the given group
     -h: Output this help text
     -v: Enables verbose mode
+    -p: Pretty print stack traces in test.log
 
 Set the environment variable QEMU=/path/to/qemu-system-ARCH to
 specify the appropriate qemu binary for ARCH-run.
@@ -27,8 +29,9 @@ EOF
 
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
+log_redir=">> test.log"
 
-while getopts "g:hv" opt; do
+while getopts "g:hvp" opt; do
     case $opt in
         g)
             only_group=$OPTARG
@@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
         v)
             verbose="yes"
             ;;
+        p)
+           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
+           ;;
         *)
             exit 1
             ;;
     esac
 done
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
new file mode 100755
index 0000000..ee5a52e
--- /dev/null
+++ b/scripts/pretty_print_stacks.py
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+import sys
+import traceback
+
+# Subvert output buffering.
+def puts(string):
+    sys.stdout.write(string)
+    sys.stdout.flush()
+
+def pretty_print_stack(binary, line):
+    addrs = line.split()[1:]
+    # Addresses are return addresses unless preceded by a '@'. We want the
+    # caller address so line numbers are more intuitive. Thus we subtract 1
+    # from the address to get the call code.
+    for i in range(len(addrs)):
+        addr = addrs[i]
+        if addr.startswith('@'):
+            addrs[i] = addr[1:]
+        else:
+            addrs[i] = '%lx' % (int(addrs[i], 16) - 1)
+
+    # Output like this:
+    #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
+    #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
+    cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+    cmd.extend(addrs)
+
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+    out, err = p.communicate()
+    if p.returncode != 0:
+        puts(line)
+        return
+
+    for line in out.splitlines():
+        m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
+        if m is None:
+            puts('%s\n' % line)
+            return
+
+        head, path, line, tail = m.groups()
+        line = int(line)
+        puts('%s at %s:%d%s\n' % (head, path, line, tail))
+        try:
+            lines = open(path).readlines()
+        except IOError:
+            continue
+        if line > 1:
+            puts('        %s\n' % lines[line - 2].rstrip())
+        puts('      > %s\n' % lines[line - 1].rstrip())
+        if line < len(lines):
+            puts('        %s\n' % lines[line].rstrip())
+
+def main():
+    if len(sys.argv) != 2:
+        sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
+        sys.exit(1)
+
+    binary = sys.argv[1]
+
+    try:
+        while True:
+            # Subvert input buffering.
+            line = sys.stdin.readline()
+            if line == '':
+                break
+
+            if not line.strip().startswith('STACK:'):
+                puts(line)
+                continue
+
+            try:
+                pretty_print_stack(binary, line)
+            except Exception:
+                puts('Error pretty printing stack:\n')
+                puts(traceback.format_exc())
+                puts('Continuing without pretty printing...\n')
+                while True:
+                    puts(line)
+                    line = sys.stdin.readline()
+                    if line == '':
+                        break
+    except:
+        sys.exit(1)
+
+if __name__ == '__main__':
+    main()
-- 
2.7.0.rc3.207.g0ac5344

Re: [kvm-unit-tests v3 4/4] scripts: pretty print stack traces

[Andrew Jones]

On Thu, Mar 03, 2016 at 12:48:36PM -0800, Peter Feiner wrote:
> Optionally pretty print stacks in test.log with 'run_tests.sh -p'.
> 
> The script runs addresses through addr2line to get file names and line
> numbers.  Also prints out source code lines if files are available.
> For example
> 
> 0x004013f5: ac_test_exec at x86/access.c:818
>             }
>       >     ac_test_setup_pte(at, pool);
>             r = ac_test_do_access(at);
> 0x004014dd: ac_test_run at x86/access.c:852
>                 ++tests;
>       >         successes += ac_test_exec(&at, &pool);
>             } while (ac_test_bump(&at));
> 0x0040157d: main at x86/access.c:873
>             printf("starting test\n\n");
>       >     r = ac_test_run();
>             return r ? 0 : 1;
> 0x0040028e: start64 at x86/cstart64.S:206
>                 lea __argv(%rip), %rsi
>       >         call main
>                 mov %eax, %edi
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  run_tests.sh                   | 10 ++++-
>  scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+), 2 deletions(-)
>  create mode 100755 scripts/pretty_print_stacks.py
> 
> diff --git a/run_tests.sh b/run_tests.sh
> index 89e8f84..b6f2003 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -1,6 +1,7 @@
>  #!/bin/bash
>  
>  verbose="no"
> +pretty="no"
>  
>  if [ ! -f config.mak ]; then
>      echo "run ./configure && make first. See ./configure -h"
> @@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
>      -g: Only execute tests in the given group
>      -h: Output this help text
>      -v: Enables verbose mode
> +    -p: Pretty print stack traces in test.log
>  
>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>  specify the appropriate qemu binary for ARCH-run.
> @@ -27,8 +29,9 @@ EOF
>  
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
> +log_redir=">> test.log"
>  
> -while getopts "g:hv" opt; do
> +while getopts "g:hvp" opt; do
>      case $opt in
>          g)
>              only_group=$OPTARG
> @@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
>          v)
>              verbose="yes"
>              ;;
> +        p)
> +           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> +           ;;

What about the idea of turning this on by default, and only turning it
off if it was configured off?

drew

Re: [kvm-unit-tests v3 4/4] scripts: pretty print stack traces

[Peter Feiner]

On Fri, Mar 4, 2016 at 2:24 AM, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Mar 03, 2016 at 12:48:36PM -0800, Peter Feiner wrote:
>> Optionally pretty print stacks in test.log with 'run_tests.sh -p'.
>>
>> The script runs addresses through addr2line to get file names and line
>> numbers.  Also prints out source code lines if files are available.
>> For example
>>
>> 0x004013f5: ac_test_exec at x86/access.c:818
>>             }
>>       >     ac_test_setup_pte(at, pool);
>>             r = ac_test_do_access(at);
>> 0x004014dd: ac_test_run at x86/access.c:852
>>                 ++tests;
>>       >         successes += ac_test_exec(&at, &pool);
>>             } while (ac_test_bump(&at));
>> 0x0040157d: main at x86/access.c:873
>>             printf("starting test\n\n");
>>       >     r = ac_test_run();
>>             return r ? 0 : 1;
>> 0x0040028e: start64 at x86/cstart64.S:206
>>                 lea __argv(%rip), %rsi
>>       >         call main
>>                 mov %eax, %edi
>>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> ---
>>  run_tests.sh                   | 10 ++++-
>>  scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 97 insertions(+), 2 deletions(-)
>>  create mode 100755 scripts/pretty_print_stacks.py
>>
>> diff --git a/run_tests.sh b/run_tests.sh
>> index 89e8f84..b6f2003 100755
>> --- a/run_tests.sh
>> +++ b/run_tests.sh
>> @@ -1,6 +1,7 @@
>>  #!/bin/bash
>>
>>  verbose="no"
>> +pretty="no"
>>
>>  if [ ! -f config.mak ]; then
>>      echo "run ./configure && make first. See ./configure -h"
>> @@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
>>      -g: Only execute tests in the given group
>>      -h: Output this help text
>>      -v: Enables verbose mode
>> +    -p: Pretty print stack traces in test.log
>>
>>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
>>  specify the appropriate qemu binary for ARCH-run.
>> @@ -27,8 +29,9 @@ EOF
>>
>>  RUNTIME_arch_run="./$TEST_DIR/run"
>>  source scripts/runtime.bash
>> +log_redir=">> test.log"
>>
>> -while getopts "g:hv" opt; do
>> +while getopts "g:hvp" opt; do
>>      case $opt in
>>          g)
>>              only_group=$OPTARG
>> @@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
>>          v)
>>              verbose="yes"
>>              ;;
>> +        p)
>> +           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
>> +           ;;
>
> What about the idea of turning this on by default, and only turning it
> off if it was configured off?

I was waiting for the config option to be added. I had thought you
were going to send that patch. I'll add the config option to the
series and make it on-by-default.

Peter

Re: [kvm-unit-tests v3 4/4] scripts: pretty print stack traces

[Andrew Jones]

On Fri, Mar 04, 2016 at 08:55:59AM -0800, Peter Feiner wrote:
> On Fri, Mar 4, 2016 at 2:24 AM, Andrew Jones <drjones@redhat.com> wrote:
> > On Thu, Mar 03, 2016 at 12:48:36PM -0800, Peter Feiner wrote:
> >> Optionally pretty print stacks in test.log with 'run_tests.sh -p'.
> >>
> >> The script runs addresses through addr2line to get file names and line
> >> numbers.  Also prints out source code lines if files are available.
> >> For example
> >>
> >> 0x004013f5: ac_test_exec at x86/access.c:818
> >>             }
> >>       >     ac_test_setup_pte(at, pool);
> >>             r = ac_test_do_access(at);
> >> 0x004014dd: ac_test_run at x86/access.c:852
> >>                 ++tests;
> >>       >         successes += ac_test_exec(&at, &pool);
> >>             } while (ac_test_bump(&at));
> >> 0x0040157d: main at x86/access.c:873
> >>             printf("starting test\n\n");
> >>       >     r = ac_test_run();
> >>             return r ? 0 : 1;
> >> 0x0040028e: start64 at x86/cstart64.S:206
> >>                 lea __argv(%rip), %rsi
> >>       >         call main
> >>                 mov %eax, %edi
> >>
> >> Signed-off-by: Peter Feiner <pfeiner@google.com>
> >> ---
> >>  run_tests.sh                   | 10 ++++-
> >>  scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 97 insertions(+), 2 deletions(-)
> >>  create mode 100755 scripts/pretty_print_stacks.py
> >>
> >> diff --git a/run_tests.sh b/run_tests.sh
> >> index 89e8f84..b6f2003 100755
> >> --- a/run_tests.sh
> >> +++ b/run_tests.sh
> >> @@ -1,6 +1,7 @@
> >>  #!/bin/bash
> >>
> >>  verbose="no"
> >> +pretty="no"
> >>
> >>  if [ ! -f config.mak ]; then
> >>      echo "run ./configure && make first. See ./configure -h"
> >> @@ -18,6 +19,7 @@ Usage: $0 [-g group] [-h] [-v]
> >>      -g: Only execute tests in the given group
> >>      -h: Output this help text
> >>      -v: Enables verbose mode
> >> +    -p: Pretty print stack traces in test.log
> >>
> >>  Set the environment variable QEMU=/path/to/qemu-system-ARCH to
> >>  specify the appropriate qemu binary for ARCH-run.
> >> @@ -27,8 +29,9 @@ EOF
> >>
> >>  RUNTIME_arch_run="./$TEST_DIR/run"
> >>  source scripts/runtime.bash
> >> +log_redir=">> test.log"
> >>
> >> -while getopts "g:hv" opt; do
> >> +while getopts "g:hvp" opt; do
> >>      case $opt in
> >>          g)
> >>              only_group=$OPTARG
> >> @@ -40,13 +43,16 @@ while getopts "g:hv" opt; do
> >>          v)
> >>              verbose="yes"
> >>              ;;
> >> +        p)
> >> +           log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> >> +           ;;
> >
> > What about the idea of turning this on by default, and only turning it
> > off if it was configured off?
> 
> I was waiting for the config option to be added. I had thought you
> were going to send that patch. I'll add the config option to the
> series and make it on-by-default.

Ah, yeah, I was thinking you'd do it, since the '-p' option can just be
dropped, once we have it on by default, and thus no reason to commit this,
and then commit a patch taking it away.

Thanks,
drew


> 
> Peter
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH kvm-unit-tests v4 0/6] Debugging aids

[Peter Feiner]

Some aids for debugging: stack dumping, register dumping, and printing
failing assertion conditions.

Compiled on ppc64, arm64, i386, x86_64.

v4:
	* Moved around arch and asm files per Linux style (hopefully).
	* Added --[enable|disable]-pretty-print-stacks to ./configure.
	  This has obviated the -p command-line option in
	  ./run_tests.sh that I introduced in v1.
	* Fixed arch-neutral backtrace so backtrace itself isn't
	  included.

v3:
	* Renamed functions to match standard backtrace functions
	* Added asm-generic stack walker that uses __builtin_return_address.
	* Moved offset out of libcflat and into pretty printer
	* Made pretty printer more robust w.r.t. errors: now it just
	  continues printing.

v2:
        * Fixed a bunch tests on x86 that broke with
          -fno-omit-frame-pointer.
        * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
          will have similar problems that I fixed for x86.

Peter Feiner (5):
  lib: backtrace printing
  x86: lib: debug dump on unhandled exceptions
  lib: dump stack on failed assert()
  scripts: pretty print stack traces
  scripts: automatically pretty print stacks

 Makefile                       |  6 ++-
 configure                      | 10 +++++
 lib/arm/asm/stack.h            |  0
 lib/arm64/asm/stack.h          |  0
 lib/asm-generic/stack.c        |  3 ++
 lib/libcflat.h                 |  9 +++-
 lib/powerpc/asm/stack.h        |  0
 lib/ppc64/asm/stack.h          |  0
 lib/stack.c                    | 96 ++++++++++++++++++++++++++++++++++++++++++
 lib/stack.h                    | 20 +++++++++
 lib/x86/asm/stack.h            | 18 ++++++++
 lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++---
 lib/x86/stack.c                | 25 +++++++++++
 run_tests.sh                   | 10 ++++-
 scripts/mkstandalone.sh        |  4 ++
 scripts/pretty_print_stacks.py | 89 +++++++++++++++++++++++++++++++++++++++
 x86/Makefile.common            |  4 ++
 17 files changed, 351 insertions(+), 12 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/asm-generic/stack.c
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/stack.c
 create mode 100644 lib/stack.h
 create mode 100644 lib/x86/asm/stack.h
 create mode 100644 lib/x86/stack.c
 create mode 100755 scripts/pretty_print_stacks.py

-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v4 1/5] lib: backtrace printing

[Peter Feiner]

Functions to walk stack and print backtrace. The stack's unadorned as

	STACK: [@]addr addr addr ...

where the optional @ indicates that addr isn't a return address.

A follow-up patch post-processes the output to pretty-print the stack.

Frame stack walker is just a stub on arm and ppc.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 Makefile                |  6 ++--
 lib/arm/asm/stack.h     |  0
 lib/arm64/asm/stack.h   |  0
 lib/asm-generic/stack.c |  3 ++
 lib/libcflat.h          |  3 ++
 lib/powerpc/asm/stack.h |  0
 lib/ppc64/asm/stack.h   |  0
 lib/stack.c             | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/stack.h             | 20 +++++++++++
 lib/x86/asm/stack.h     | 18 ++++++++++
 lib/x86/stack.c         | 25 +++++++++++++
 x86/Makefile.common     |  4 +++
 12 files changed, 173 insertions(+), 2 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/asm-generic/stack.c
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/stack.c
 create mode 100644 lib/stack.h
 create mode 100644 lib/x86/asm/stack.h
 create mode 100644 lib/x86/stack.c

diff --git a/Makefile b/Makefile
index 72e6711..c53aff6 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,8 @@ cflatobjs := \
 	lib/printf.o \
 	lib/string.o \
 	lib/abort.o \
-	lib/report.o
+	lib/report.o \
+	lib/stack.o
 
 # libfdt paths
 LIBFDT_objdir = lib/libfdt
@@ -42,7 +43,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
 
 CFLAGS += -g
 CFLAGS += $(autodepend-flags) -Wall
-CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
+frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
+CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
 CFLAGS += $(call cc-option, -fno-stack-protector, "")
 CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
 
diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c
new file mode 100644
index 0000000..40c67c2
--- /dev/null
+++ b/lib/asm-generic/stack.c
@@ -0,0 +1,3 @@
+#include <libcflat.h>
+#include <asm-generic/stack.h>
+
diff --git a/lib/libcflat.h b/lib/libcflat.h
index b58a8a1..55bddca 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+extern void dump_stack(void);
+extern void dump_frame_stack(const void *instruction, const void *frame);
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\
diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/stack.c b/lib/stack.c
new file mode 100644
index 0000000..035fe27
--- /dev/null
+++ b/lib/stack.c
@@ -0,0 +1,96 @@
+#include <libcflat.h>
+#include <stack.h>
+
+#define MAX_DEPTH 20
+
+static void print_stack(const void **return_addrs, int depth,
+			bool top_is_return_address)
+{
+	int i = 0;
+
+	printf("\tSTACK:");
+
+	/* @addr indicates a non-return address, as expected by the stack
+	 * pretty printer script. */
+	if (depth > 0 && !top_is_return_address) {
+		printf(" @%lx", (unsigned long) return_addrs[0]);
+		i++;
+	}
+
+	for (; i < depth; i++) {
+		printf(" %lx", (unsigned long) return_addrs[i]);
+	}
+	printf("\n");
+}
+
+void dump_stack(void)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	depth = backtrace(return_addrs, MAX_DEPTH);
+	print_stack(return_addrs, depth, true);
+}
+
+void dump_frame_stack(const void *instruction, const void *frame)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	return_addrs[0] = instruction;
+	depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1);
+	print_stack(return_addrs, depth + 1, false);
+}
+
+#ifndef HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	void *addr;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	/* __builtin_return_address requires a compile-time constant argument */
+#define GET_RETURN_ADDRESS(i)						\
+	if (max_depth == i)						\
+		goto done;						\
+	addr = __builtin_return_address(i + 1);				\
+	if (!addr)							\
+		goto done;						\
+	return_addrs[i] = __builtin_extract_return_addr(addr);		\
+	depth = i + 1;							\
+
+	GET_RETURN_ADDRESS(0)
+	GET_RETURN_ADDRESS(1)
+	GET_RETURN_ADDRESS(2)
+	GET_RETURN_ADDRESS(3)
+	GET_RETURN_ADDRESS(4)
+	GET_RETURN_ADDRESS(5)
+	GET_RETURN_ADDRESS(6)
+	GET_RETURN_ADDRESS(7)
+	GET_RETURN_ADDRESS(8)
+	GET_RETURN_ADDRESS(9)
+	GET_RETURN_ADDRESS(10)
+	GET_RETURN_ADDRESS(11)
+	GET_RETURN_ADDRESS(12)
+	GET_RETURN_ADDRESS(13)
+	GET_RETURN_ADDRESS(14)
+	GET_RETURN_ADDRESS(15)
+	GET_RETURN_ADDRESS(16)
+	GET_RETURN_ADDRESS(17)
+	GET_RETURN_ADDRESS(18)
+	GET_RETURN_ADDRESS(19)
+	GET_RETURN_ADDRESS(20)
+
+#undef GET_RETURN_ADDRESS
+
+done:
+	walking = 0;
+	return depth;
+}
+#endif  /* HAVE_ARCH_BACKTRACE */
diff --git a/lib/stack.h b/lib/stack.h
new file mode 100644
index 0000000..bb6b9aa
--- /dev/null
+++ b/lib/stack.h
@@ -0,0 +1,20 @@
+#ifndef _STACK_H_
+#define _STACK_H_
+
+#include <libcflat.h>
+#include <asm/stack.h>
+
+#ifndef HAVE_ARCH_BACKTRACE_FRAME
+static inline int
+backtrace_frame(const void *frame __unused, const void **return_addrs __unused,
+		int max_depth __unused)
+{
+	return 0;
+}
+#endif
+
+#ifndef HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth);
+#endif
+
+#endif
diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
new file mode 100644
index 0000000..bb1c2c0
--- /dev/null
+++ b/lib/x86/asm/stack.h
@@ -0,0 +1,18 @@
+#ifndef _X86ASM_STACK_H_
+#define _X86ASM_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#define HAVE_ARCH_BACKTRACE_FRAME
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
+
+#define HAVE_ARCH_BACKTRACE
+static inline int backtrace(const void **return_addrs, int max_depth)
+{
+	return backtrace_frame(__builtin_frame_address(0), return_addrs,
+			       max_depth);
+}
+
+#endif
diff --git a/lib/x86/stack.c b/lib/x86/stack.c
new file mode 100644
index 0000000..3227800
--- /dev/null
+++ b/lib/x86/stack.c
@@ -0,0 +1,25 @@
+#include <libcflat.h>
+#include <stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	const unsigned long *bp = (unsigned long *) frame;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	for (depth = 0; depth < max_depth; depth++) {
+		return_addrs[depth] = (void *) bp[1];
+		if (return_addrs[depth] == 0)
+			break;
+		bp = (unsigned long *) bp[0];
+	}
+
+	walking = 0;
+	return depth;
+}
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 3a14fea..ca80367 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
 cflatobjs += lib/x86/desc.o
 cflatobjs += lib/x86/isr.o
 cflatobjs += lib/x86/acpi.o
+cflatobjs += lib/x86/stack.o
 
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I lib
@@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
 CFLAGS += -m$(bits)
 CFLAGS += -O1
 
+# dump_stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
 libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
 
 FLATLIBS = lib/libcflat.a $(libgcc)
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v4 2/5] x86: lib: debug dump on unhandled exceptions

[Peter Feiner]

Prints the stack trace and registers.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/x86/desc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 16cbba5..75765ef 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -38,6 +38,67 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
+static const char* exception_mnemonic(int vector)
+{
+	switch(vector) {
+	case 0: return "#DE";
+	case 1: return "#DB";
+	case 2: return "#NMI";
+	case 3: return "#BP";
+	case 4: return "#OF";
+	case 5: return "#BR";
+	case 6: return "#UD";
+	case 7: return "#NM";
+	case 8: return "#DF";
+	case 10: return "#TS";
+	case 11: return "#NP";
+	case 12: return "#SS";
+	case 13: return "#GP";
+	case 14: return "#PF";
+	case 16: return "#MF";
+	case 17: return "#AC";
+	case 18: return "#MC";
+	case 19: return "#XM";
+	default: return "#??";
+	}
+}
+
+static void unhandled_exception(struct ex_regs *regs, bool cpu)
+{
+	printf("Unhandled %sexception %ld %s at ip %016lx\n",
+	       cpu ? "cpu " : "", regs->vector,
+	       exception_mnemonic(regs->vector), regs->rip);
+	if (regs->vector == 14)
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
+
+	printf("error_code=%04lx      rflags=%08lx      cs=%08lx\n"
+	       "rax=%016lx rcx=%016lx rdx=%016lx rbx=%016lx\n"
+	       "rbp=%016lx rsi=%016lx rdi=%016lx\n"
+#ifdef __x86_64__
+	       " r8=%016lx  r9=%016lx r10=%016lx r11=%016lx\n"
+	       "r12=%016lx r13=%016lx r14=%016lx r15=%016lx\n"
+#endif
+	       "cr0=%016lx cr2=%016lx cr3=%016lx cr4=%016lx\n"
+#ifdef __x86_64__
+	       "cr8=%016lx\n"
+#endif
+	       ,
+	       regs->error_code, regs->rflags, regs->cs,
+	       regs->rax, regs->rcx, regs->rdx, regs->rbx,
+	       regs->rbp, regs->rsi, regs->rdi,
+#ifdef __x86_64__
+	       regs->r8, regs->r9, regs->r10, regs->r11,
+	       regs->r12, regs->r13, regs->r14, regs->r15,
+#endif
+	       read_cr0(), read_cr2(), read_cr3(), read_cr4()
+#ifdef __x86_64__
+	       , read_cr8()
+#endif
+	);
+	dump_frame_stack((void*) regs->rip, (void*) regs->rbp);
+	exit(7);
+}
+
 static void check_exception_table(struct ex_regs *regs)
 {
     struct ex_record *ex;
@@ -53,8 +114,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %lu\n", regs->vector);
-    abort();
+    unhandled_exception(regs, false);
 }
 
 static void (*exception_handlers[32])(struct ex_regs *regs);
@@ -75,10 +135,7 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %lu\n", regs->vector);
-	if (regs->vector == 14)
-		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
-	abort();
+	unhandled_exception(regs, true);
 }
 
 #define EX(NAME, N) extern char NAME##_fault;	\
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v4 3/5] lib: dump stack on failed assert()

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 55bddca..df50615 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -92,10 +92,12 @@ extern void dump_frame_stack(const void *instruction, const void *frame);
 
 #define assert(cond)							\
 do {									\
-	if (!(cond))							\
+	if (!(cond)) {							\
 		printf("%s:%d: assert failed: %s\n",			\
-		       __FILE__, __LINE__, #cond),			\
+		       __FILE__, __LINE__, #cond);			\
+		dump_stack();						\
 		abort();						\
+	}								\
 } while (0)
 
 #endif
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v4 4/5] scripts: pretty print stack traces

[Peter Feiner]

The script runs addresses through addr2line to get file names and line
numbers.  Also prints out source code lines if files are available.
For example

0x004013f5: ac_test_exec at x86/access.c:818
            }
      >     ac_test_setup_pte(at, pool);
            r = ac_test_do_access(at);
0x004014dd: ac_test_run at x86/access.c:852
                ++tests;
      >         successes += ac_test_exec(&at, &pool);
            } while (ac_test_bump(&at));
0x0040157d: main at x86/access.c:873
            printf("starting test\n\n");
      >     r = ac_test_run();
            return r ? 0 : 1;
0x0040028e: start64 at x86/cstart64.S:206
                lea __argv(%rip), %rsi
      >         call main
                mov %eax, %edi

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100755 scripts/pretty_print_stacks.py

diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
new file mode 100755
index 0000000..ee5a52e
--- /dev/null
+++ b/scripts/pretty_print_stacks.py
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+import sys
+import traceback
+
+# Subvert output buffering.
+def puts(string):
+    sys.stdout.write(string)
+    sys.stdout.flush()
+
+def pretty_print_stack(binary, line):
+    addrs = line.split()[1:]
+    # Addresses are return addresses unless preceded by a '@'. We want the
+    # caller address so line numbers are more intuitive. Thus we subtract 1
+    # from the address to get the call code.
+    for i in range(len(addrs)):
+        addr = addrs[i]
+        if addr.startswith('@'):
+            addrs[i] = addr[1:]
+        else:
+            addrs[i] = '%lx' % (int(addrs[i], 16) - 1)
+
+    # Output like this:
+    #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
+    #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
+    cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+    cmd.extend(addrs)
+
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+    out, err = p.communicate()
+    if p.returncode != 0:
+        puts(line)
+        return
+
+    for line in out.splitlines():
+        m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
+        if m is None:
+            puts('%s\n' % line)
+            return
+
+        head, path, line, tail = m.groups()
+        line = int(line)
+        puts('%s at %s:%d%s\n' % (head, path, line, tail))
+        try:
+            lines = open(path).readlines()
+        except IOError:
+            continue
+        if line > 1:
+            puts('        %s\n' % lines[line - 2].rstrip())
+        puts('      > %s\n' % lines[line - 1].rstrip())
+        if line < len(lines):
+            puts('        %s\n' % lines[line].rstrip())
+
+def main():
+    if len(sys.argv) != 2:
+        sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
+        sys.exit(1)
+
+    binary = sys.argv[1]
+
+    try:
+        while True:
+            # Subvert input buffering.
+            line = sys.stdin.readline()
+            if line == '':
+                break
+
+            if not line.strip().startswith('STACK:'):
+                puts(line)
+                continue
+
+            try:
+                pretty_print_stack(binary, line)
+            except Exception:
+                puts('Error pretty printing stack:\n')
+                puts(traceback.format_exc())
+                puts('Continuing without pretty printing...\n')
+                while True:
+                    puts(line)
+                    line = sys.stdin.readline()
+                    if line == '':
+                        break
+    except:
+        sys.exit(1)
+
+if __name__ == '__main__':
+    main()
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v4 5/5] scripts: automatically pretty print stacks

[Peter Feiner]

Now run_tests.sh automatically pipes test output through
scripts/pretty_print_stacks.py. Can be disabled with
./configure --disable-pretty-print-stacks.

Pretty printing disabled on standalone tests because there's no
binary.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 configure               | 10 ++++++++++
 run_tests.sh            | 10 ++++++++--
 scripts/mkstandalone.sh |  4 ++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 958321d..ba6c55b 100755
--- a/configure
+++ b/configure
@@ -11,6 +11,7 @@ arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
 host=$arch
 cross_prefix=
 endian=""
+pretty_print_stacks=yes
 
 usage() {
     cat <<-EOF
@@ -25,6 +26,8 @@ usage() {
 	    --prefix=PREFIX        where to install things ($prefix)
 	    --kerneldir=DIR        kernel build directory for kvm.h ($kerneldir)
 	    --endian=ENDIAN        endianness to compile for (little or big, ppc64 only)
+	    --[enable|disable]-pretty-print-stacks
+	                           enable or disable pretty stack printing (enabled by default)
 EOF
     exit 1
 }
@@ -61,6 +64,12 @@ while [[ "$1" = -* ]]; do
 	--ld)
 	    ld="$arg"
 	    ;;
+	--enable-pretty-print-stacks)
+	    pretty_print_stacks=yes
+	    ;;
+	--disable-pretty-print-stacks)
+	    pretty_print_stacks=no
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -149,4 +158,5 @@ API=$api
 TEST_DIR=$testdir
 FIRMWARE=$firmware
 ENDIAN=$endian
+PRETTY_PRINT_STACKS=$pretty_print_stacks
 EOF
diff --git a/run_tests.sh b/run_tests.sh
index 89e8f84..9589252 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -28,7 +28,7 @@ EOF
 RUNTIME_arch_run="./$TEST_DIR/run"
 source scripts/runtime.bash
 
-while getopts "g:hv" opt; do
+while getopts "g:hvp" opt; do
     case $opt in
         g)
             only_group=$OPTARG
@@ -46,7 +46,13 @@ while getopts "g:hv" opt; do
     esac
 done
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
+	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
+else
+	log_redir=">> test.log"
+fi
+
+RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 1d3a20d..368fa4a 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -41,6 +41,10 @@ generate_test ()
 	echo "#!/bin/bash"
 	echo "export STANDALONE=yes"
 	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/')"
+
+	# Can't pretty print without binary.
+	echo "export PRETTY_PRINT_STACKS=no"
+
 	config_export ARCH
 	config_export ARCH_NAME
 	config_export PROCESSOR
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH kvm-unit-tests v4 5/5] scripts: automatically pretty print stacks

[Andrew Jones]

----- Original Message -----
> Now run_tests.sh automatically pipes test output through
> scripts/pretty_print_stacks.py. Can be disabled with
> ./configure --disable-pretty-print-stacks.
> 
> Pretty printing disabled on standalone tests because there's no
> binary.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  configure               | 10 ++++++++++
>  run_tests.sh            | 10 ++++++++--
>  scripts/mkstandalone.sh |  4 ++++
>  3 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 958321d..ba6c55b 100755
> --- a/configure
> +++ b/configure
> @@ -11,6 +11,7 @@ arch=`uname -m | sed -e
> 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
>  host=$arch
>  cross_prefix=
>  endian=""
> +pretty_print_stacks=yes
>  
>  usage() {
>      cat <<-EOF
> @@ -25,6 +26,8 @@ usage() {
>  	    --prefix=PREFIX        where to install things ($prefix)
>  	    --kerneldir=DIR        kernel build directory for kvm.h ($kerneldir)
>  	    --endian=ENDIAN        endianness to compile for (little or big, ppc64
>  	    only)
> +	    --[enable|disable]-pretty-print-stacks
> +	                           enable or disable pretty stack printing (enabled
> by default)
>  EOF
>      exit 1
>  }
> @@ -61,6 +64,12 @@ while [[ "$1" = -* ]]; do
>  	--ld)
>  	    ld="$arg"
>  	    ;;
> +	--enable-pretty-print-stacks)
> +	    pretty_print_stacks=yes
> +	    ;;
> +	--disable-pretty-print-stacks)
> +	    pretty_print_stacks=no
> +	    ;;
>  	--help)
>  	    usage
>  	    ;;
> @@ -149,4 +158,5 @@ API=$api
>  TEST_DIR=$testdir
>  FIRMWARE=$firmware
>  ENDIAN=$endian
> +PRETTY_PRINT_STACKS=$pretty_print_stacks
>  EOF
> diff --git a/run_tests.sh b/run_tests.sh
> index 89e8f84..9589252 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -28,7 +28,7 @@ EOF
>  RUNTIME_arch_run="./$TEST_DIR/run"
>  source scripts/runtime.bash
>  
> -while getopts "g:hv" opt; do
> +while getopts "g:hvp" opt; do

Forgot to remove the 'p' here.

>      case $opt in
>          g)
>              only_group=$OPTARG
> @@ -46,7 +46,13 @@ while getopts "g:hv" opt; do
>      esac
>  done
>  
> -RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> +if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> +	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> +else
> +	log_redir=">> test.log"
> +fi
> +
> +RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
>  config=$TEST_DIR/unittests.cfg
>  rm -f test.log
>  printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> index 1d3a20d..368fa4a 100755
> --- a/scripts/mkstandalone.sh
> +++ b/scripts/mkstandalone.sh
> @@ -41,6 +41,10 @@ generate_test ()
>  	echo "#!/bin/bash"
>  	echo "export STANDALONE=yes"
>  	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e
>  	's/arm.*/arm/')"
> +
> +	# Can't pretty print without binary.
> +	echo "export PRETTY_PRINT_STACKS=no"

To be precise, "without the elf", because we do have the flat file.
(And, for powerpc we actually even have the elf, so I guess we should
 say "can't pretty print without the pretty print script" or something)

However, we don't even really need this right now, because the
PRETTY_PRINT_STACKS variable is only consumed by run_tests.sh, which
isn't used by standalone scripts. I don't mind adding the explicit =no
here though, just in case something changes.

> +
>  	config_export ARCH
>  	config_export ARCH_NAME
>  	config_export PROCESSOR
> --
> 2.7.0.rc3.207.g0ac5344
> 
> 

Re: [PATCH kvm-unit-tests v4 5/5] scripts: automatically pretty print stacks

[Peter Feiner]

On Sat, Mar 05, 2016 at 06:29:50AM -0500, Andrew Jones wrote:
> 
> 
> ----- Original Message -----
> > Now run_tests.sh automatically pipes test output through
> > scripts/pretty_print_stacks.py. Can be disabled with
> > ./configure --disable-pretty-print-stacks.
> > 
> > Pretty printing disabled on standalone tests because there's no
> > binary.
> > 
> > Signed-off-by: Peter Feiner <pfeiner@google.com>
> > ---
> >  configure               | 10 ++++++++++
> >  run_tests.sh            | 10 ++++++++--
> >  scripts/mkstandalone.sh |  4 ++++
> >  3 files changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 958321d..ba6c55b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -11,6 +11,7 @@ arch=`uname -m | sed -e
> > 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
> >  host=$arch
> >  cross_prefix=
> >  endian=""
> > +pretty_print_stacks=yes
> >  
> >  usage() {
> >      cat <<-EOF
> > @@ -25,6 +26,8 @@ usage() {
> >  	    --prefix=PREFIX        where to install things ($prefix)
> >  	    --kerneldir=DIR        kernel build directory for kvm.h ($kerneldir)
> >  	    --endian=ENDIAN        endianness to compile for (little or big, ppc64
> >  	    only)
> > +	    --[enable|disable]-pretty-print-stacks
> > +	                           enable or disable pretty stack printing (enabled
> > by default)
> >  EOF
> >      exit 1
> >  }
> > @@ -61,6 +64,12 @@ while [[ "$1" = -* ]]; do
> >  	--ld)
> >  	    ld="$arg"
> >  	    ;;
> > +	--enable-pretty-print-stacks)
> > +	    pretty_print_stacks=yes
> > +	    ;;
> > +	--disable-pretty-print-stacks)
> > +	    pretty_print_stacks=no
> > +	    ;;
> >  	--help)
> >  	    usage
> >  	    ;;
> > @@ -149,4 +158,5 @@ API=$api
> >  TEST_DIR=$testdir
> >  FIRMWARE=$firmware
> >  ENDIAN=$endian
> > +PRETTY_PRINT_STACKS=$pretty_print_stacks
> >  EOF
> > diff --git a/run_tests.sh b/run_tests.sh
> > index 89e8f84..9589252 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -28,7 +28,7 @@ EOF
> >  RUNTIME_arch_run="./$TEST_DIR/run"
> >  source scripts/runtime.bash
> >  
> > -while getopts "g:hv" opt; do
> > +while getopts "g:hvp" opt; do
> 
> Forgot to remove the 'p' here.
> 
> >      case $opt in
> >          g)
> >              only_group=$OPTARG
> > @@ -46,7 +46,13 @@ while getopts "g:hv" opt; do
> >      esac
> >  done
> >  
> > -RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
> > +if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
> > +	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
> > +else
> > +	log_redir=">> test.log"
> > +fi
> > +
> > +RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
> >  config=$TEST_DIR/unittests.cfg
> >  rm -f test.log
> >  printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
> > diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
> > index 1d3a20d..368fa4a 100755
> > --- a/scripts/mkstandalone.sh
> > +++ b/scripts/mkstandalone.sh
> > @@ -41,6 +41,10 @@ generate_test ()
> >  	echo "#!/bin/bash"
> >  	echo "export STANDALONE=yes"
> >  	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e
> >  	's/arm.*/arm/')"
> > +
> > +	# Can't pretty print without binary.
> > +	echo "export PRETTY_PRINT_STACKS=no"
> 
> To be precise, "without the elf", because we do have the flat file.
> (And, for powerpc we actually even have the elf, so I guess we should
>  say "can't pretty print without the pretty print script" or something)
> 
> However, we don't even really need this right now, because the
> PRETTY_PRINT_STACKS variable is only consumed by run_tests.sh, which
> isn't used by standalone scripts. I don't mind adding the explicit =no
> here though, just in case something changes.

All good points. I just removed the comment altogether :-)

Peter

Re: [PATCH kvm-unit-tests v4 0/6] Debugging aids

[Peter Feiner]

On Fri, Mar 04, 2016 at 11:33:56AM -0800, Peter Feiner wrote:
> Some aids for debugging: stack dumping, register dumping, and printing
> failing assertion conditions.
> 
> Compiled on ppc64, arm64, i386, x86_64.
> 
> v4:
> 	* Moved around arch and asm files per Linux style (hopefully).
> 	* Added --[enable|disable]-pretty-print-stacks to ./configure.
> 	  This has obviated the -p command-line option in
> 	  ./run_tests.sh that I introduced in v1.
> 	* Fixed arch-neutral backtrace so backtrace itself isn't
> 	  included.
> 
> v3:
> 	* Renamed functions to match standard backtrace functions
> 	* Added asm-generic stack walker that uses __builtin_return_address.
> 	* Moved offset out of libcflat and into pretty printer
> 	* Made pretty printer more robust w.r.t. errors: now it just
> 	  continues printing.
> 
> v2:
>         * Fixed a bunch tests on x86 that broke with
>           -fno-omit-frame-pointer.
>         * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
>           will have similar problems that I fixed for x86.
> 
> Peter Feiner (5):
>   lib: backtrace printing
>   x86: lib: debug dump on unhandled exceptions
>   lib: dump stack on failed assert()
>   scripts: pretty print stack traces
>   scripts: automatically pretty print stacks

There are indeed only 5 patches in v4. The subject on this first email, "[PATCH
kvm-unit-tests v4 0/6] Debugging aids", has a typo.

Peter

[PATCH kvm-unit-tests v5 0/5] Debugging aids

[Peter Feiner]

Some aids for debugging: stack dumping, register dumping, and printing
failing assertion conditions.

Compiled on ppc64, arm64, i386, x86_64.

v5:
	* Removed unused "b" flag from run_tests.sh
	* Removed misleading comment in mkstandalone.sh.

v4:
	* Moved around arch and asm files per Linux style (hopefully).
	* Added --[enable|disable]-pretty-print-stacks to ./configure.
	  This has obviated the -p command-line option in
	  ./run_tests.sh that I introduced in v1.
	* Fixed arch-neutral backtrace so backtrace itself isn't
	  included.

v3:
	* Renamed functions to match standard backtrace functions
	* Added asm-generic stack walker that uses __builtin_return_address.
	* Moved offset out of libcflat and into pretty printer
	* Made pretty printer more robust w.r.t. errors: now it just
	  continues printing.

v2:
        * Fixed a bunch tests on x86 that broke with
          -fno-omit-frame-pointer.
        * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
          will have similar problems that I fixed for x86.

Peter Feiner (5):
  lib: backtrace printing
  x86: lib: debug dump on unhandled exceptions
  lib: dump stack on failed assert()
  scripts: pretty print stack traces
  scripts: automatically pretty print stacks

 Makefile                       |  6 ++-
 configure                      | 10 +++++
 lib/arm/asm/stack.h            |  0
 lib/arm64/asm/stack.h          |  0
 lib/asm-generic/stack.c        |  3 ++
 lib/libcflat.h                 |  9 +++-
 lib/powerpc/asm/stack.h        |  0
 lib/ppc64/asm/stack.h          |  0
 lib/stack.c                    | 96 ++++++++++++++++++++++++++++++++++++++++++
 lib/stack.h                    | 20 +++++++++
 lib/x86/asm/stack.h            | 18 ++++++++
 lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++---
 lib/x86/stack.c                | 25 +++++++++++
 run_tests.sh                   |  8 +++-
 scripts/mkstandalone.sh        |  2 +
 scripts/pretty_print_stacks.py | 89 +++++++++++++++++++++++++++++++++++++++
 x86/Makefile.common            |  4 ++
 17 files changed, 348 insertions(+), 11 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/asm-generic/stack.c
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/stack.c
 create mode 100644 lib/stack.h
 create mode 100644 lib/x86/asm/stack.h
 create mode 100644 lib/x86/stack.c
 create mode 100755 scripts/pretty_print_stacks.py

-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v5 1/5] lib: backtrace printing

[Peter Feiner]

Functions to walk stack and print backtrace. The stack's unadorned as

	STACK: [@]addr addr addr ...

where the optional @ indicates that addr isn't a return address.

A follow-up patch post-processes the output to pretty-print the stack.

Frame stack walker is just a stub on arm and ppc.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 Makefile                |  6 ++--
 lib/arm/asm/stack.h     |  0
 lib/arm64/asm/stack.h   |  0
 lib/asm-generic/stack.c |  3 ++
 lib/libcflat.h          |  3 ++
 lib/powerpc/asm/stack.h |  0
 lib/ppc64/asm/stack.h   |  0
 lib/stack.c             | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/stack.h             | 20 +++++++++++
 lib/x86/asm/stack.h     | 18 ++++++++++
 lib/x86/stack.c         | 25 +++++++++++++
 x86/Makefile.common     |  4 +++
 12 files changed, 173 insertions(+), 2 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/asm-generic/stack.c
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/stack.c
 create mode 100644 lib/stack.h
 create mode 100644 lib/x86/asm/stack.h
 create mode 100644 lib/x86/stack.c

diff --git a/Makefile b/Makefile
index 72e6711..c53aff6 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,8 @@ cflatobjs := \
 	lib/printf.o \
 	lib/string.o \
 	lib/abort.o \
-	lib/report.o
+	lib/report.o \
+	lib/stack.o
 
 # libfdt paths
 LIBFDT_objdir = lib/libfdt
@@ -42,7 +43,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
 
 CFLAGS += -g
 CFLAGS += $(autodepend-flags) -Wall
-CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
+frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
+CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
 CFLAGS += $(call cc-option, -fno-stack-protector, "")
 CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
 
diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c
new file mode 100644
index 0000000..40c67c2
--- /dev/null
+++ b/lib/asm-generic/stack.c
@@ -0,0 +1,3 @@
+#include <libcflat.h>
+#include <asm-generic/stack.h>
+
diff --git a/lib/libcflat.h b/lib/libcflat.h
index b58a8a1..55bddca 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+extern void dump_stack(void);
+extern void dump_frame_stack(const void *instruction, const void *frame);
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\
diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/stack.c b/lib/stack.c
new file mode 100644
index 0000000..035fe27
--- /dev/null
+++ b/lib/stack.c
@@ -0,0 +1,96 @@
+#include <libcflat.h>
+#include <stack.h>
+
+#define MAX_DEPTH 20
+
+static void print_stack(const void **return_addrs, int depth,
+			bool top_is_return_address)
+{
+	int i = 0;
+
+	printf("\tSTACK:");
+
+	/* @addr indicates a non-return address, as expected by the stack
+	 * pretty printer script. */
+	if (depth > 0 && !top_is_return_address) {
+		printf(" @%lx", (unsigned long) return_addrs[0]);
+		i++;
+	}
+
+	for (; i < depth; i++) {
+		printf(" %lx", (unsigned long) return_addrs[i]);
+	}
+	printf("\n");
+}
+
+void dump_stack(void)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	depth = backtrace(return_addrs, MAX_DEPTH);
+	print_stack(return_addrs, depth, true);
+}
+
+void dump_frame_stack(const void *instruction, const void *frame)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	return_addrs[0] = instruction;
+	depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1);
+	print_stack(return_addrs, depth + 1, false);
+}
+
+#ifndef HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	void *addr;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	/* __builtin_return_address requires a compile-time constant argument */
+#define GET_RETURN_ADDRESS(i)						\
+	if (max_depth == i)						\
+		goto done;						\
+	addr = __builtin_return_address(i + 1);				\
+	if (!addr)							\
+		goto done;						\
+	return_addrs[i] = __builtin_extract_return_addr(addr);		\
+	depth = i + 1;							\
+
+	GET_RETURN_ADDRESS(0)
+	GET_RETURN_ADDRESS(1)
+	GET_RETURN_ADDRESS(2)
+	GET_RETURN_ADDRESS(3)
+	GET_RETURN_ADDRESS(4)
+	GET_RETURN_ADDRESS(5)
+	GET_RETURN_ADDRESS(6)
+	GET_RETURN_ADDRESS(7)
+	GET_RETURN_ADDRESS(8)
+	GET_RETURN_ADDRESS(9)
+	GET_RETURN_ADDRESS(10)
+	GET_RETURN_ADDRESS(11)
+	GET_RETURN_ADDRESS(12)
+	GET_RETURN_ADDRESS(13)
+	GET_RETURN_ADDRESS(14)
+	GET_RETURN_ADDRESS(15)
+	GET_RETURN_ADDRESS(16)
+	GET_RETURN_ADDRESS(17)
+	GET_RETURN_ADDRESS(18)
+	GET_RETURN_ADDRESS(19)
+	GET_RETURN_ADDRESS(20)
+
+#undef GET_RETURN_ADDRESS
+
+done:
+	walking = 0;
+	return depth;
+}
+#endif  /* HAVE_ARCH_BACKTRACE */
diff --git a/lib/stack.h b/lib/stack.h
new file mode 100644
index 0000000..bb6b9aa
--- /dev/null
+++ b/lib/stack.h
@@ -0,0 +1,20 @@
+#ifndef _STACK_H_
+#define _STACK_H_
+
+#include <libcflat.h>
+#include <asm/stack.h>
+
+#ifndef HAVE_ARCH_BACKTRACE_FRAME
+static inline int
+backtrace_frame(const void *frame __unused, const void **return_addrs __unused,
+		int max_depth __unused)
+{
+	return 0;
+}
+#endif
+
+#ifndef HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth);
+#endif
+
+#endif
diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
new file mode 100644
index 0000000..bb1c2c0
--- /dev/null
+++ b/lib/x86/asm/stack.h
@@ -0,0 +1,18 @@
+#ifndef _X86ASM_STACK_H_
+#define _X86ASM_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#define HAVE_ARCH_BACKTRACE_FRAME
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
+
+#define HAVE_ARCH_BACKTRACE
+static inline int backtrace(const void **return_addrs, int max_depth)
+{
+	return backtrace_frame(__builtin_frame_address(0), return_addrs,
+			       max_depth);
+}
+
+#endif
diff --git a/lib/x86/stack.c b/lib/x86/stack.c
new file mode 100644
index 0000000..3227800
--- /dev/null
+++ b/lib/x86/stack.c
@@ -0,0 +1,25 @@
+#include <libcflat.h>
+#include <stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	const unsigned long *bp = (unsigned long *) frame;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	for (depth = 0; depth < max_depth; depth++) {
+		return_addrs[depth] = (void *) bp[1];
+		if (return_addrs[depth] == 0)
+			break;
+		bp = (unsigned long *) bp[0];
+	}
+
+	walking = 0;
+	return depth;
+}
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 3a14fea..ca80367 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
 cflatobjs += lib/x86/desc.o
 cflatobjs += lib/x86/isr.o
 cflatobjs += lib/x86/acpi.o
+cflatobjs += lib/x86/stack.o
 
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I lib
@@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
 CFLAGS += -m$(bits)
 CFLAGS += -O1
 
+# dump_stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
 libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
 
 FLATLIBS = lib/libcflat.a $(libgcc)
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH kvm-unit-tests v5 1/5] lib: backtrace printing

[Andrew Jones]

On Mon, Mar 07, 2016 at 09:46:53AM -0800, Peter Feiner wrote:
> Functions to walk stack and print backtrace. The stack's unadorned as
> 
> 	STACK: [@]addr addr addr ...
> 
> where the optional @ indicates that addr isn't a return address.
> 
> A follow-up patch post-processes the output to pretty-print the stack.
> 
> Frame stack walker is just a stub on arm and ppc.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  Makefile                |  6 ++--
>  lib/arm/asm/stack.h     |  0
>  lib/arm64/asm/stack.h   |  0
>  lib/asm-generic/stack.c |  3 ++
>  lib/libcflat.h          |  3 ++
>  lib/powerpc/asm/stack.h |  0
>  lib/ppc64/asm/stack.h   |  0
>  lib/stack.c             | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/stack.h             | 20 +++++++++++
>  lib/x86/asm/stack.h     | 18 ++++++++++
>  lib/x86/stack.c         | 25 +++++++++++++
>  x86/Makefile.common     |  4 +++
>  12 files changed, 173 insertions(+), 2 deletions(-)
>  create mode 100644 lib/arm/asm/stack.h
>  create mode 100644 lib/arm64/asm/stack.h
>  create mode 100644 lib/asm-generic/stack.c
>  create mode 100644 lib/powerpc/asm/stack.h
>  create mode 100644 lib/ppc64/asm/stack.h
>  create mode 100644 lib/stack.c
>  create mode 100644 lib/stack.h
>  create mode 100644 lib/x86/asm/stack.h
>  create mode 100644 lib/x86/stack.c
> 
> diff --git a/Makefile b/Makefile
> index 72e6711..c53aff6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -22,7 +22,8 @@ cflatobjs := \
>  	lib/printf.o \
>  	lib/string.o \
>  	lib/abort.o \
> -	lib/report.o
> +	lib/report.o \
> +	lib/stack.o
>  
>  # libfdt paths
>  LIBFDT_objdir = lib/libfdt
> @@ -42,7 +43,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
>  
>  CFLAGS += -g
>  CFLAGS += $(autodepend-flags) -Wall
> -CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
> +frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> +CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
>  CFLAGS += $(call cc-option, -fno-stack-protector, "")
>  CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
>  
> diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
> new file mode 100644
> index 0000000..e69de29
> diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
> new file mode 100644
> index 0000000..e69de29
> diff --git a/lib/asm-generic/stack.c b/lib/asm-generic/stack.c
> new file mode 100644
> index 0000000..40c67c2
> --- /dev/null
> +++ b/lib/asm-generic/stack.c
> @@ -0,0 +1,3 @@
> +#include <libcflat.h>
> +#include <asm-generic/stack.h>
> +

Looks like you forgot to remove this file.

> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index b58a8a1..55bddca 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
>  extern void report_abort(const char *msg_fmt, ...);
>  extern int report_summary(void);
>  
> +extern void dump_stack(void);
> +extern void dump_frame_stack(const void *instruction, const void *frame);
> +
>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
>  
>  #define container_of(ptr, type, member) ({				\
> diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
> new file mode 100644
> index 0000000..e69de29
> diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
> new file mode 100644
> index 0000000..e69de29
> diff --git a/lib/stack.c b/lib/stack.c
> new file mode 100644
> index 0000000..035fe27
> --- /dev/null
> +++ b/lib/stack.c
> @@ -0,0 +1,96 @@
> +#include <libcflat.h>
> +#include <stack.h>
> +
> +#define MAX_DEPTH 20
> +
> +static void print_stack(const void **return_addrs, int depth,
> +			bool top_is_return_address)
> +{
> +	int i = 0;
> +
> +	printf("\tSTACK:");
> +
> +	/* @addr indicates a non-return address, as expected by the stack
> +	 * pretty printer script. */
> +	if (depth > 0 && !top_is_return_address) {
> +		printf(" @%lx", (unsigned long) return_addrs[0]);
> +		i++;
> +	}
> +
> +	for (; i < depth; i++) {
> +		printf(" %lx", (unsigned long) return_addrs[i]);
> +	}
> +	printf("\n");
> +}
> +
> +void dump_stack(void)
> +{
> +	const void *return_addrs[MAX_DEPTH];
> +	int depth;
> +
> +	depth = backtrace(return_addrs, MAX_DEPTH);
> +	print_stack(return_addrs, depth, true);
> +}
> +
> +void dump_frame_stack(const void *instruction, const void *frame)
> +{
> +	const void *return_addrs[MAX_DEPTH];
> +	int depth;
> +
> +	return_addrs[0] = instruction;
> +	depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1);
> +	print_stack(return_addrs, depth + 1, false);
> +}
> +
> +#ifndef HAVE_ARCH_BACKTRACE
> +int backtrace(const void **return_addrs, int max_depth)
> +{
> +	static int walking;
> +	int depth = 0;
> +	void *addr;
> +
> +	if (walking) {
> +		printf("RECURSIVE STACK WALK!!!\n");
> +		return 0;
> +	}
> +	walking = 1;
> +
> +	/* __builtin_return_address requires a compile-time constant argument */
> +#define GET_RETURN_ADDRESS(i)						\
> +	if (max_depth == i)						\
> +		goto done;						\
> +	addr = __builtin_return_address(i + 1);				\

Is the +1 to skip a level, which means addr is an address two levels up?
If we do that, then won't we skip the callers of backtrace, which may or
may not be dump_stack?


> +	if (!addr)							\
> +		goto done;						\
> +	return_addrs[i] = __builtin_extract_return_addr(addr);		\

So here we put the i+1th return address into return_addrs[i]. I find
that a bit confusing. Maybe we should leave it to the callers of
backtrace to do any level skipping they want, i.e. here we shouldn't
skip anything, but dump_stack could start printing at i=1.


> +	depth = i + 1;							\
> +
> +	GET_RETURN_ADDRESS(0)
> +	GET_RETURN_ADDRESS(1)
> +	GET_RETURN_ADDRESS(2)
> +	GET_RETURN_ADDRESS(3)
> +	GET_RETURN_ADDRESS(4)
> +	GET_RETURN_ADDRESS(5)
> +	GET_RETURN_ADDRESS(6)
> +	GET_RETURN_ADDRESS(7)
> +	GET_RETURN_ADDRESS(8)
> +	GET_RETURN_ADDRESS(9)
> +	GET_RETURN_ADDRESS(10)
> +	GET_RETURN_ADDRESS(11)
> +	GET_RETURN_ADDRESS(12)
> +	GET_RETURN_ADDRESS(13)
> +	GET_RETURN_ADDRESS(14)
> +	GET_RETURN_ADDRESS(15)
> +	GET_RETURN_ADDRESS(16)
> +	GET_RETURN_ADDRESS(17)
> +	GET_RETURN_ADDRESS(18)
> +	GET_RETURN_ADDRESS(19)
> +	GET_RETURN_ADDRESS(20)
> +
> +#undef GET_RETURN_ADDRESS
> +
> +done:
> +	walking = 0;
> +	return depth;
> +}
> +#endif  /* HAVE_ARCH_BACKTRACE */
> diff --git a/lib/stack.h b/lib/stack.h
> new file mode 100644
> index 0000000..bb6b9aa
> --- /dev/null
> +++ b/lib/stack.h
> @@ -0,0 +1,20 @@
> +#ifndef _STACK_H_
> +#define _STACK_H_
> +
> +#include <libcflat.h>
> +#include <asm/stack.h>
> +
> +#ifndef HAVE_ARCH_BACKTRACE_FRAME
> +static inline int
> +backtrace_frame(const void *frame __unused, const void **return_addrs __unused,
> +		int max_depth __unused)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#ifndef HAVE_ARCH_BACKTRACE
> +int backtrace(const void **return_addrs, int max_depth);
> +#endif
> +
> +#endif
> diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
> new file mode 100644
> index 0000000..bb1c2c0
> --- /dev/null
> +++ b/lib/x86/asm/stack.h
> @@ -0,0 +1,18 @@
> +#ifndef _X86ASM_STACK_H_
> +#define _X86ASM_STACK_H_
> +
> +#ifndef _STACK_H_
> +#error Do not directly include <asm/stack.h>. Just use <stack.h>.
> +#endif
> +
> +#define HAVE_ARCH_BACKTRACE_FRAME
> +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
> +
> +#define HAVE_ARCH_BACKTRACE
> +static inline int backtrace(const void **return_addrs, int max_depth)
> +{
> +	return backtrace_frame(__builtin_frame_address(0), return_addrs,
> +			       max_depth);
> +}
> +
> +#endif
> diff --git a/lib/x86/stack.c b/lib/x86/stack.c
> new file mode 100644
> index 0000000..3227800
> --- /dev/null
> +++ b/lib/x86/stack.c
> @@ -0,0 +1,25 @@
> +#include <libcflat.h>
> +#include <stack.h>
> +
> +int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
> +{
> +	static int walking;
> +	int depth = 0;
> +	const unsigned long *bp = (unsigned long *) frame;
> +
> +	if (walking) {
> +		printf("RECURSIVE STACK WALK!!!\n");
> +		return 0;
> +	}
> +	walking = 1;
> +
> +	for (depth = 0; depth < max_depth; depth++) {
> +		return_addrs[depth] = (void *) bp[1];
> +		if (return_addrs[depth] == 0)
> +			break;
> +		bp = (unsigned long *) bp[0];
> +	}
> +
> +	walking = 0;
> +	return depth;
> +}
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 3a14fea..ca80367 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
>  cflatobjs += lib/x86/desc.o
>  cflatobjs += lib/x86/isr.o
>  cflatobjs += lib/x86/acpi.o
> +cflatobjs += lib/x86/stack.o
>  
>  $(libcflat): LDFLAGS += -nostdlib
>  $(libcflat): CFLAGS += -ffreestanding -I lib
> @@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
>  CFLAGS += -m$(bits)
>  CFLAGS += -O1
>  
> +# dump_stack.o relies on frame pointers.
> +KEEP_FRAME_POINTER := y
> +
>  libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
>  
>  FLATLIBS = lib/libcflat.a $(libgcc)
> -- 
> 2.7.0.rc3.207.g0ac5344
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,
drew

Re: [PATCH kvm-unit-tests v5 1/5] lib: backtrace printing

[Peter Feiner]

On Tue, Mar 08, 2016 at 11:24:45AM +0700, Andrew Jones wrote:
> On Mon, Mar 07, 2016 at 09:46:53AM -0800, Peter Feiner wrote:
> > +#ifndef HAVE_ARCH_BACKTRACE
> > +int backtrace(const void **return_addrs, int max_depth)
> > +{
> > +	static int walking;
> > +	int depth = 0;
> > +	void *addr;
> > +
> > +	if (walking) {
> > +		printf("RECURSIVE STACK WALK!!!\n");
> > +		return 0;
> > +	}
> > +	walking = 1;
> > +
> > +	/* __builtin_return_address requires a compile-time constant argument */
> > +#define GET_RETURN_ADDRESS(i)						\
> > +	if (max_depth == i)						\
> > +		goto done;						\
> > +	addr = __builtin_return_address(i + 1);				\
> 
> Is the +1 to skip a level, which means addr is an address two levels up?
> If we do that, then won't we skip the callers of backtrace, which may or
> may not be dump_stack?

Right and right. In v6 dump_stack now does the skipping.


> 
> 
> > +	if (!addr)							\
> > +		goto done;						\
> > +	return_addrs[i] = __builtin_extract_return_addr(addr);		\
> 
> So here we put the i+1th return address into return_addrs[i]. I find
> that a bit confusing. Maybe we should leave it to the callers of
> backtrace to do any level skipping they want, i.e. here we shouldn't
> skip anything, but dump_stack could start printing at i=1.

Agreed. Changed in v6.

Peter

[PATCH kvm-unit-tests v5 2/5] x86: lib: debug dump on unhandled exceptions

[Peter Feiner]

Prints the stack trace and registers.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/x86/desc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 16cbba5..75765ef 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -38,6 +38,67 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
+static const char* exception_mnemonic(int vector)
+{
+	switch(vector) {
+	case 0: return "#DE";
+	case 1: return "#DB";
+	case 2: return "#NMI";
+	case 3: return "#BP";
+	case 4: return "#OF";
+	case 5: return "#BR";
+	case 6: return "#UD";
+	case 7: return "#NM";
+	case 8: return "#DF";
+	case 10: return "#TS";
+	case 11: return "#NP";
+	case 12: return "#SS";
+	case 13: return "#GP";
+	case 14: return "#PF";
+	case 16: return "#MF";
+	case 17: return "#AC";
+	case 18: return "#MC";
+	case 19: return "#XM";
+	default: return "#??";
+	}
+}
+
+static void unhandled_exception(struct ex_regs *regs, bool cpu)
+{
+	printf("Unhandled %sexception %ld %s at ip %016lx\n",
+	       cpu ? "cpu " : "", regs->vector,
+	       exception_mnemonic(regs->vector), regs->rip);
+	if (regs->vector == 14)
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
+
+	printf("error_code=%04lx      rflags=%08lx      cs=%08lx\n"
+	       "rax=%016lx rcx=%016lx rdx=%016lx rbx=%016lx\n"
+	       "rbp=%016lx rsi=%016lx rdi=%016lx\n"
+#ifdef __x86_64__
+	       " r8=%016lx  r9=%016lx r10=%016lx r11=%016lx\n"
+	       "r12=%016lx r13=%016lx r14=%016lx r15=%016lx\n"
+#endif
+	       "cr0=%016lx cr2=%016lx cr3=%016lx cr4=%016lx\n"
+#ifdef __x86_64__
+	       "cr8=%016lx\n"
+#endif
+	       ,
+	       regs->error_code, regs->rflags, regs->cs,
+	       regs->rax, regs->rcx, regs->rdx, regs->rbx,
+	       regs->rbp, regs->rsi, regs->rdi,
+#ifdef __x86_64__
+	       regs->r8, regs->r9, regs->r10, regs->r11,
+	       regs->r12, regs->r13, regs->r14, regs->r15,
+#endif
+	       read_cr0(), read_cr2(), read_cr3(), read_cr4()
+#ifdef __x86_64__
+	       , read_cr8()
+#endif
+	);
+	dump_frame_stack((void*) regs->rip, (void*) regs->rbp);
+	exit(7);
+}
+
 static void check_exception_table(struct ex_regs *regs)
 {
     struct ex_record *ex;
@@ -53,8 +114,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %lu\n", regs->vector);
-    abort();
+    unhandled_exception(regs, false);
 }
 
 static void (*exception_handlers[32])(struct ex_regs *regs);
@@ -75,10 +135,7 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %lu\n", regs->vector);
-	if (regs->vector == 14)
-		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
-	abort();
+	unhandled_exception(regs, true);
 }
 
 #define EX(NAME, N) extern char NAME##_fault;	\
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v5 3/5] lib: dump stack on failed assert()

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 55bddca..df50615 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -92,10 +92,12 @@ extern void dump_frame_stack(const void *instruction, const void *frame);
 
 #define assert(cond)							\
 do {									\
-	if (!(cond))							\
+	if (!(cond)) {							\
 		printf("%s:%d: assert failed: %s\n",			\
-		       __FILE__, __LINE__, #cond),			\
+		       __FILE__, __LINE__, #cond);			\
+		dump_stack();						\
 		abort();						\
+	}								\
 } while (0)
 
 #endif
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v5 4/5] scripts: pretty print stack traces

[Peter Feiner]

The script runs addresses through addr2line to get file names and line
numbers.  Also prints out source code lines if files are available.
For example

0x004013f5: ac_test_exec at x86/access.c:818
            }
      >     ac_test_setup_pte(at, pool);
            r = ac_test_do_access(at);
0x004014dd: ac_test_run at x86/access.c:852
                ++tests;
      >         successes += ac_test_exec(&at, &pool);
            } while (ac_test_bump(&at));
0x0040157d: main at x86/access.c:873
            printf("starting test\n\n");
      >     r = ac_test_run();
            return r ? 0 : 1;
0x0040028e: start64 at x86/cstart64.S:206
                lea __argv(%rip), %rsi
      >         call main
                mov %eax, %edi

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100755 scripts/pretty_print_stacks.py

diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
new file mode 100755
index 0000000..ee5a52e
--- /dev/null
+++ b/scripts/pretty_print_stacks.py
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+import sys
+import traceback
+
+# Subvert output buffering.
+def puts(string):
+    sys.stdout.write(string)
+    sys.stdout.flush()
+
+def pretty_print_stack(binary, line):
+    addrs = line.split()[1:]
+    # Addresses are return addresses unless preceded by a '@'. We want the
+    # caller address so line numbers are more intuitive. Thus we subtract 1
+    # from the address to get the call code.
+    for i in range(len(addrs)):
+        addr = addrs[i]
+        if addr.startswith('@'):
+            addrs[i] = addr[1:]
+        else:
+            addrs[i] = '%lx' % (int(addrs[i], 16) - 1)
+
+    # Output like this:
+    #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
+    #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
+    cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+    cmd.extend(addrs)
+
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+    out, err = p.communicate()
+    if p.returncode != 0:
+        puts(line)
+        return
+
+    for line in out.splitlines():
+        m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
+        if m is None:
+            puts('%s\n' % line)
+            return
+
+        head, path, line, tail = m.groups()
+        line = int(line)
+        puts('%s at %s:%d%s\n' % (head, path, line, tail))
+        try:
+            lines = open(path).readlines()
+        except IOError:
+            continue
+        if line > 1:
+            puts('        %s\n' % lines[line - 2].rstrip())
+        puts('      > %s\n' % lines[line - 1].rstrip())
+        if line < len(lines):
+            puts('        %s\n' % lines[line].rstrip())
+
+def main():
+    if len(sys.argv) != 2:
+        sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
+        sys.exit(1)
+
+    binary = sys.argv[1]
+
+    try:
+        while True:
+            # Subvert input buffering.
+            line = sys.stdin.readline()
+            if line == '':
+                break
+
+            if not line.strip().startswith('STACK:'):
+                puts(line)
+                continue
+
+            try:
+                pretty_print_stack(binary, line)
+            except Exception:
+                puts('Error pretty printing stack:\n')
+                puts(traceback.format_exc())
+                puts('Continuing without pretty printing...\n')
+                while True:
+                    puts(line)
+                    line = sys.stdin.readline()
+                    if line == '':
+                        break
+    except:
+        sys.exit(1)
+
+if __name__ == '__main__':
+    main()
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v5 5/5] scripts: automatically pretty print stacks

[Peter Feiner]

Now run_tests.sh automatically pipes test output through
scripts/pretty_print_stacks.py. Can be disabled with
./configure --disable-pretty-print-stacks.

Pretty printing disabled on standalone tests because there's no
binary.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 configure               | 10 ++++++++++
 run_tests.sh            |  8 +++++++-
 scripts/mkstandalone.sh |  2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 958321d..ba6c55b 100755
--- a/configure
+++ b/configure
@@ -11,6 +11,7 @@ arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
 host=$arch
 cross_prefix=
 endian=""
+pretty_print_stacks=yes
 
 usage() {
     cat <<-EOF
@@ -25,6 +26,8 @@ usage() {
 	    --prefix=PREFIX        where to install things ($prefix)
 	    --kerneldir=DIR        kernel build directory for kvm.h ($kerneldir)
 	    --endian=ENDIAN        endianness to compile for (little or big, ppc64 only)
+	    --[enable|disable]-pretty-print-stacks
+	                           enable or disable pretty stack printing (enabled by default)
 EOF
     exit 1
 }
@@ -61,6 +64,12 @@ while [[ "$1" = -* ]]; do
 	--ld)
 	    ld="$arg"
 	    ;;
+	--enable-pretty-print-stacks)
+	    pretty_print_stacks=yes
+	    ;;
+	--disable-pretty-print-stacks)
+	    pretty_print_stacks=no
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -149,4 +158,5 @@ API=$api
 TEST_DIR=$testdir
 FIRMWARE=$firmware
 ENDIAN=$endian
+PRETTY_PRINT_STACKS=$pretty_print_stacks
 EOF
diff --git a/run_tests.sh b/run_tests.sh
index 89e8f84..7e0237f 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -46,7 +46,13 @@ while getopts "g:hv" opt; do
     esac
 done
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
+	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
+else
+	log_redir=">> test.log"
+fi
+
+RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 1d3a20d..ef15bc8 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -41,6 +41,8 @@ generate_test ()
 	echo "#!/bin/bash"
 	echo "export STANDALONE=yes"
 	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/')"
+	echo "export PRETTY_PRINT_STACKS=no"
+
 	config_export ARCH
 	config_export ARCH_NAME
 	config_export PROCESSOR
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH kvm-unit-tests v5 0/5] Debugging aids

[Andrew Jones]

On Mon, Mar 07, 2016 at 09:46:52AM -0800, Peter Feiner wrote:
> Some aids for debugging: stack dumping, register dumping, and printing
> failing assertion conditions.
> 
> Compiled on ppc64, arm64, i386, x86_64.
> 
> v5:
> 	* Removed unused "b" flag from run_tests.sh
> 	* Removed misleading comment in mkstandalone.sh.
> 
> v4:
> 	* Moved around arch and asm files per Linux style (hopefully).
> 	* Added --[enable|disable]-pretty-print-stacks to ./configure.
> 	  This has obviated the -p command-line option in
> 	  ./run_tests.sh that I introduced in v1.
> 	* Fixed arch-neutral backtrace so backtrace itself isn't
> 	  included.
> 
> v3:
> 	* Renamed functions to match standard backtrace functions
> 	* Added asm-generic stack walker that uses __builtin_return_address.
> 	* Moved offset out of libcflat and into pretty printer
> 	* Made pretty printer more robust w.r.t. errors: now it just
> 	  continues printing.
> 
> v2:
>         * Fixed a bunch tests on x86 that broke with
>           -fno-omit-frame-pointer.
>         * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
>           will have similar problems that I fixed for x86.
> 
> Peter Feiner (5):
>   lib: backtrace printing
>   x86: lib: debug dump on unhandled exceptions
>   lib: dump stack on failed assert()
>   scripts: pretty print stack traces
>   scripts: automatically pretty print stacks
> 
>  Makefile                       |  6 ++-
>  configure                      | 10 +++++
>  lib/arm/asm/stack.h            |  0
>  lib/arm64/asm/stack.h          |  0
>  lib/asm-generic/stack.c        |  3 ++
>  lib/libcflat.h                 |  9 +++-
>  lib/powerpc/asm/stack.h        |  0
>  lib/ppc64/asm/stack.h          |  0
>  lib/stack.c                    | 96 ++++++++++++++++++++++++++++++++++++++++++
>  lib/stack.h                    | 20 +++++++++
>  lib/x86/asm/stack.h            | 18 ++++++++
>  lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++---
>  lib/x86/stack.c                | 25 +++++++++++
>  run_tests.sh                   |  8 +++-
>  scripts/mkstandalone.sh        |  2 +
>  scripts/pretty_print_stacks.py | 89 +++++++++++++++++++++++++++++++++++++++
>  x86/Makefile.common            |  4 ++
>  17 files changed, 348 insertions(+), 11 deletions(-)
>  create mode 100644 lib/arm/asm/stack.h
>  create mode 100644 lib/arm64/asm/stack.h
>  create mode 100644 lib/asm-generic/stack.c
>  create mode 100644 lib/powerpc/asm/stack.h
>  create mode 100644 lib/ppc64/asm/stack.h
>  create mode 100644 lib/stack.c
>  create mode 100644 lib/stack.h
>  create mode 100644 lib/x86/asm/stack.h
>  create mode 100644 lib/x86/stack.c
>  create mode 100755 scripts/pretty_print_stacks.py
> 
> -- 
> 2.7.0.rc3.207.g0ac5344

I had a couple comments on 1/5, otherwise the series looks good to
me.

Thanks,
drew

[PATCH kvm-unit-tests v6 0/5] Debugging aids

[Peter Feiner]

Some aids for debugging: stack dumping, register dumping, and printing
failing assertion conditions.

Compiled on ppc64, arm64, i386, x86_64.

v6:
	* Don't print dump_stack's call to backtrace() in the stack
	  trace rather than having backtrace() skip over the first
	  return address in the trace.
	* Actually removed lib/asm-generic/stack.c.

v5:
	* Removed unused "b" flag from run_tests.sh
	* Removed misleading comment in mkstandalone.sh.

v4:
	* Moved around arch and asm files per Linux style (hopefully).
	* Added --[enable|disable]-pretty-print-stacks to ./configure.
	  This has obviated the -p command-line option in
	  ./run_tests.sh that I introduced in v1.
	* Fixed arch-neutral backtrace so backtrace itself isn't
	  included.

v3:
	* Renamed functions to match standard backtrace functions
	* Added asm-generic stack walker that uses __builtin_return_address.
	* Moved offset out of libcflat and into pretty printer
	* Made pretty printer more robust w.r.t. errors: now it just
	  continues printing.

v2:
        * Fixed a bunch tests on x86 that broke with
          -fno-omit-frame-pointer.
        * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
          will have similar problems that I fixed for x86.



Peter Feiner (5):
  lib: backtrace printing
  x86: lib: debug dump on unhandled exceptions
  lib: dump stack on failed assert()
  scripts: pretty print stack traces
  scripts: automatically pretty print stacks

 Makefile                       |  6 ++-
 configure                      | 10 +++++
 lib/arm/asm/stack.h            |  0
 lib/arm64/asm/stack.h          |  0
 lib/libcflat.h                 |  9 +++-
 lib/powerpc/asm/stack.h        |  0
 lib/ppc64/asm/stack.h          |  0
 lib/stack.c                    | 96 ++++++++++++++++++++++++++++++++++++++++++
 lib/stack.h                    | 20 +++++++++
 lib/x86/asm/stack.h            | 14 ++++++
 lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++---
 lib/x86/stack.c                | 31 ++++++++++++++
 run_tests.sh                   |  8 +++-
 scripts/mkstandalone.sh        |  2 +
 scripts/pretty_print_stacks.py | 89 +++++++++++++++++++++++++++++++++++++++
 x86/Makefile.common            |  4 ++
 16 files changed, 347 insertions(+), 11 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/stack.c
 create mode 100644 lib/stack.h
 create mode 100644 lib/x86/asm/stack.h
 create mode 100644 lib/x86/stack.c
 create mode 100755 scripts/pretty_print_stacks.py

-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v6 1/5] lib: backtrace printing

[Peter Feiner]

Functions to walk stack and print backtrace. The stack's unadorned as

	STACK: [@]addr addr addr ...

where the optional @ indicates that addr isn't a return address.

A follow-up patch post-processes the output to pretty-print the stack.

Frame stack walker is just a stub on arm and ppc.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 Makefile                |  6 ++--
 lib/arm/asm/stack.h     |  0
 lib/arm64/asm/stack.h   |  0
 lib/libcflat.h          |  3 ++
 lib/powerpc/asm/stack.h |  0
 lib/ppc64/asm/stack.h   |  0
 lib/stack.c             | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/stack.h             | 20 +++++++++++
 lib/x86/asm/stack.h     | 14 ++++++++
 lib/x86/stack.c         | 31 ++++++++++++++++
 x86/Makefile.common     |  4 +++
 11 files changed, 172 insertions(+), 2 deletions(-)
 create mode 100644 lib/arm/asm/stack.h
 create mode 100644 lib/arm64/asm/stack.h
 create mode 100644 lib/powerpc/asm/stack.h
 create mode 100644 lib/ppc64/asm/stack.h
 create mode 100644 lib/stack.c
 create mode 100644 lib/stack.h
 create mode 100644 lib/x86/asm/stack.h
 create mode 100644 lib/x86/stack.c

diff --git a/Makefile b/Makefile
index 09999c6..b0ce2c7 100644
--- a/Makefile
+++ b/Makefile
@@ -22,7 +22,8 @@ cflatobjs := \
 	lib/printf.o \
 	lib/string.o \
 	lib/abort.o \
-	lib/report.o
+	lib/report.o \
+	lib/stack.o
 
 # libfdt paths
 LIBFDT_objdir = lib/libfdt
@@ -42,7 +43,8 @@ cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
 
 CFLAGS += -g
 CFLAGS += $(autodepend-flags) -Wall -Werror
-CFLAGS += $(call cc-option, -fomit-frame-pointer, "")
+frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
+CFLAGS += $(call cc-option, $(frame-pointer-flag), "")
 CFLAGS += $(call cc-option, -fno-stack-protector, "")
 CFLAGS += $(call cc-option, -fno-stack-protector-all, "")
 
diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/libcflat.h b/lib/libcflat.h
index b58a8a1..55bddca 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -81,6 +81,9 @@ extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
 extern void report_abort(const char *msg_fmt, ...);
 extern int report_summary(void);
 
+extern void dump_stack(void);
+extern void dump_frame_stack(const void *instruction, const void *frame);
+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\
diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
new file mode 100644
index 0000000..e69de29
diff --git a/lib/stack.c b/lib/stack.c
new file mode 100644
index 0000000..b0a0295
--- /dev/null
+++ b/lib/stack.c
@@ -0,0 +1,96 @@
+#include <libcflat.h>
+#include <stack.h>
+
+#define MAX_DEPTH 20
+
+static void print_stack(const void **return_addrs, int depth,
+			bool top_is_return_address)
+{
+	int i = 0;
+
+	printf("\tSTACK:");
+
+	/* @addr indicates a non-return address, as expected by the stack
+	 * pretty printer script. */
+	if (depth > 0 && !top_is_return_address) {
+		printf(" @%lx", (unsigned long) return_addrs[0]);
+		i++;
+	}
+
+	for (; i < depth; i++) {
+		printf(" %lx", (unsigned long) return_addrs[i]);
+	}
+	printf("\n");
+}
+
+void dump_stack(void)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	depth = backtrace(return_addrs, MAX_DEPTH);
+	print_stack(&return_addrs[1], depth ? depth - 1 : 0, true);
+}
+
+void dump_frame_stack(const void *instruction, const void *frame)
+{
+	const void *return_addrs[MAX_DEPTH];
+	int depth;
+
+	return_addrs[0] = instruction;
+	depth = backtrace_frame(frame, &return_addrs[1], MAX_DEPTH - 1);
+	print_stack(return_addrs, depth + 1, false);
+}
+
+#ifndef HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	void *addr;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	/* __builtin_return_address requires a compile-time constant argument */
+#define GET_RETURN_ADDRESS(i)						\
+	if (max_depth == i)						\
+		goto done;						\
+	addr = __builtin_return_address(i);				\
+	if (!addr)							\
+		goto done;						\
+	return_addrs[i] = __builtin_extract_return_addr(addr);		\
+	depth = i + 1;							\
+
+	GET_RETURN_ADDRESS(0)
+	GET_RETURN_ADDRESS(1)
+	GET_RETURN_ADDRESS(2)
+	GET_RETURN_ADDRESS(3)
+	GET_RETURN_ADDRESS(4)
+	GET_RETURN_ADDRESS(5)
+	GET_RETURN_ADDRESS(6)
+	GET_RETURN_ADDRESS(7)
+	GET_RETURN_ADDRESS(8)
+	GET_RETURN_ADDRESS(9)
+	GET_RETURN_ADDRESS(10)
+	GET_RETURN_ADDRESS(11)
+	GET_RETURN_ADDRESS(12)
+	GET_RETURN_ADDRESS(13)
+	GET_RETURN_ADDRESS(14)
+	GET_RETURN_ADDRESS(15)
+	GET_RETURN_ADDRESS(16)
+	GET_RETURN_ADDRESS(17)
+	GET_RETURN_ADDRESS(18)
+	GET_RETURN_ADDRESS(19)
+	GET_RETURN_ADDRESS(20)
+
+#undef GET_RETURN_ADDRESS
+
+done:
+	walking = 0;
+	return depth;
+}
+#endif  /* HAVE_ARCH_BACKTRACE */
diff --git a/lib/stack.h b/lib/stack.h
new file mode 100644
index 0000000..bb6b9aa
--- /dev/null
+++ b/lib/stack.h
@@ -0,0 +1,20 @@
+#ifndef _STACK_H_
+#define _STACK_H_
+
+#include <libcflat.h>
+#include <asm/stack.h>
+
+#ifndef HAVE_ARCH_BACKTRACE_FRAME
+static inline int
+backtrace_frame(const void *frame __unused, const void **return_addrs __unused,
+		int max_depth __unused)
+{
+	return 0;
+}
+#endif
+
+#ifndef HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth);
+#endif
+
+#endif
diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
new file mode 100644
index 0000000..fc4766d
--- /dev/null
+++ b/lib/x86/asm/stack.h
@@ -0,0 +1,14 @@
+#ifndef _X86ASM_STACK_H_
+#define _X86ASM_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#define HAVE_ARCH_BACKTRACE_FRAME
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
+
+#define HAVE_ARCH_BACKTRACE
+int backtrace(const void **return_addrs, int max_depth);
+
+#endif
diff --git a/lib/x86/stack.c b/lib/x86/stack.c
new file mode 100644
index 0000000..d75c0be
--- /dev/null
+++ b/lib/x86/stack.c
@@ -0,0 +1,31 @@
+#include <libcflat.h>
+#include <stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	static int walking;
+	int depth = 0;
+	const unsigned long *bp = (unsigned long *) frame;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = 1;
+
+	for (depth = 0; depth < max_depth; depth++) {
+		return_addrs[depth] = (void *) bp[1];
+		if (return_addrs[depth] == 0)
+			break;
+		bp = (unsigned long *) bp[0];
+	}
+
+	walking = 0;
+	return depth;
+}
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+	return backtrace_frame(__builtin_frame_address(0), return_addrs,
+			       max_depth);
+}
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 3a14fea..ca80367 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -12,6 +12,7 @@ cflatobjs += lib/x86/atomic.o
 cflatobjs += lib/x86/desc.o
 cflatobjs += lib/x86/isr.o
 cflatobjs += lib/x86/acpi.o
+cflatobjs += lib/x86/stack.o
 
 $(libcflat): LDFLAGS += -nostdlib
 $(libcflat): CFLAGS += -ffreestanding -I lib
@@ -19,6 +20,9 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
 CFLAGS += -m$(bits)
 CFLAGS += -O1
 
+# dump_stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
 libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
 
 FLATLIBS = lib/libcflat.a $(libgcc)
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v6 2/5] x86: lib: debug dump on unhandled exceptions

[Peter Feiner]

Prints the stack trace and registers.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/x86/desc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 16cbba5..75765ef 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -38,6 +38,67 @@ struct ex_record {
 
 extern struct ex_record exception_table_start, exception_table_end;
 
+static const char* exception_mnemonic(int vector)
+{
+	switch(vector) {
+	case 0: return "#DE";
+	case 1: return "#DB";
+	case 2: return "#NMI";
+	case 3: return "#BP";
+	case 4: return "#OF";
+	case 5: return "#BR";
+	case 6: return "#UD";
+	case 7: return "#NM";
+	case 8: return "#DF";
+	case 10: return "#TS";
+	case 11: return "#NP";
+	case 12: return "#SS";
+	case 13: return "#GP";
+	case 14: return "#PF";
+	case 16: return "#MF";
+	case 17: return "#AC";
+	case 18: return "#MC";
+	case 19: return "#XM";
+	default: return "#??";
+	}
+}
+
+static void unhandled_exception(struct ex_regs *regs, bool cpu)
+{
+	printf("Unhandled %sexception %ld %s at ip %016lx\n",
+	       cpu ? "cpu " : "", regs->vector,
+	       exception_mnemonic(regs->vector), regs->rip);
+	if (regs->vector == 14)
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
+
+	printf("error_code=%04lx      rflags=%08lx      cs=%08lx\n"
+	       "rax=%016lx rcx=%016lx rdx=%016lx rbx=%016lx\n"
+	       "rbp=%016lx rsi=%016lx rdi=%016lx\n"
+#ifdef __x86_64__
+	       " r8=%016lx  r9=%016lx r10=%016lx r11=%016lx\n"
+	       "r12=%016lx r13=%016lx r14=%016lx r15=%016lx\n"
+#endif
+	       "cr0=%016lx cr2=%016lx cr3=%016lx cr4=%016lx\n"
+#ifdef __x86_64__
+	       "cr8=%016lx\n"
+#endif
+	       ,
+	       regs->error_code, regs->rflags, regs->cs,
+	       regs->rax, regs->rcx, regs->rdx, regs->rbx,
+	       regs->rbp, regs->rsi, regs->rdi,
+#ifdef __x86_64__
+	       regs->r8, regs->r9, regs->r10, regs->r11,
+	       regs->r12, regs->r13, regs->r14, regs->r15,
+#endif
+	       read_cr0(), read_cr2(), read_cr3(), read_cr4()
+#ifdef __x86_64__
+	       , read_cr8()
+#endif
+	);
+	dump_frame_stack((void*) regs->rip, (void*) regs->rbp);
+	exit(7);
+}
+
 static void check_exception_table(struct ex_regs *regs)
 {
     struct ex_record *ex;
@@ -53,8 +114,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %lu\n", regs->vector);
-    abort();
+    unhandled_exception(regs, false);
 }
 
 static void (*exception_handlers[32])(struct ex_regs *regs);
@@ -75,10 +135,7 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %lu\n", regs->vector);
-	if (regs->vector == 14)
-		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
-	abort();
+	unhandled_exception(regs, true);
 }
 
 #define EX(NAME, N) extern char NAME##_fault;	\
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v6 3/5] lib: dump stack on failed assert()

[Peter Feiner]

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 55bddca..df50615 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -92,10 +92,12 @@ extern void dump_frame_stack(const void *instruction, const void *frame);
 
 #define assert(cond)							\
 do {									\
-	if (!(cond))							\
+	if (!(cond)) {							\
 		printf("%s:%d: assert failed: %s\n",			\
-		       __FILE__, __LINE__, #cond),			\
+		       __FILE__, __LINE__, #cond);			\
+		dump_stack();						\
 		abort();						\
+	}								\
 } while (0)
 
 #endif
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v6 4/5] scripts: pretty print stack traces

[Peter Feiner]

The script runs addresses through addr2line to get file names and line
numbers.  Also prints out source code lines if files are available.
For example

0x004013f5: ac_test_exec at x86/access.c:818
            }
      >     ac_test_setup_pte(at, pool);
            r = ac_test_do_access(at);
0x004014dd: ac_test_run at x86/access.c:852
                ++tests;
      >         successes += ac_test_exec(&at, &pool);
            } while (ac_test_bump(&at));
0x0040157d: main at x86/access.c:873
            printf("starting test\n\n");
      >     r = ac_test_run();
            return r ? 0 : 1;
0x0040028e: start64 at x86/cstart64.S:206
                lea __argv(%rip), %rsi
      >         call main
                mov %eax, %edi

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 scripts/pretty_print_stacks.py | 89 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100755 scripts/pretty_print_stacks.py

diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
new file mode 100755
index 0000000..ee5a52e
--- /dev/null
+++ b/scripts/pretty_print_stacks.py
@@ -0,0 +1,89 @@
+#!/usr/bin/env python
+
+import re
+import subprocess
+import sys
+import traceback
+
+# Subvert output buffering.
+def puts(string):
+    sys.stdout.write(string)
+    sys.stdout.flush()
+
+def pretty_print_stack(binary, line):
+    addrs = line.split()[1:]
+    # Addresses are return addresses unless preceded by a '@'. We want the
+    # caller address so line numbers are more intuitive. Thus we subtract 1
+    # from the address to get the call code.
+    for i in range(len(addrs)):
+        addr = addrs[i]
+        if addr.startswith('@'):
+            addrs[i] = addr[1:]
+        else:
+            addrs[i] = '%lx' % (int(addrs[i], 16) - 1)
+
+    # Output like this:
+    #        0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
+    #         (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
+    cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+    cmd.extend(addrs)
+
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+    out, err = p.communicate()
+    if p.returncode != 0:
+        puts(line)
+        return
+
+    for line in out.splitlines():
+        m = re.match('(.*) at [^ ]*/kvm-unit-tests/([^ ]*):([0-9]+)(.*)', line)
+        if m is None:
+            puts('%s\n' % line)
+            return
+
+        head, path, line, tail = m.groups()
+        line = int(line)
+        puts('%s at %s:%d%s\n' % (head, path, line, tail))
+        try:
+            lines = open(path).readlines()
+        except IOError:
+            continue
+        if line > 1:
+            puts('        %s\n' % lines[line - 2].rstrip())
+        puts('      > %s\n' % lines[line - 1].rstrip())
+        if line < len(lines):
+            puts('        %s\n' % lines[line].rstrip())
+
+def main():
+    if len(sys.argv) != 2:
+        sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
+        sys.exit(1)
+
+    binary = sys.argv[1]
+
+    try:
+        while True:
+            # Subvert input buffering.
+            line = sys.stdin.readline()
+            if line == '':
+                break
+
+            if not line.strip().startswith('STACK:'):
+                puts(line)
+                continue
+
+            try:
+                pretty_print_stack(binary, line)
+            except Exception:
+                puts('Error pretty printing stack:\n')
+                puts(traceback.format_exc())
+                puts('Continuing without pretty printing...\n')
+                while True:
+                    puts(line)
+                    line = sys.stdin.readline()
+                    if line == '':
+                        break
+    except:
+        sys.exit(1)
+
+if __name__ == '__main__':
+    main()
-- 
2.7.0.rc3.207.g0ac5344

[PATCH kvm-unit-tests v6 5/5] scripts: automatically pretty print stacks

[Peter Feiner]

Now run_tests.sh automatically pipes test output through
scripts/pretty_print_stacks.py. Can be disabled with
./configure --disable-pretty-print-stacks.

Pretty printing disabled on standalone tests because there's no
binary.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 configure               | 10 ++++++++++
 run_tests.sh            |  8 +++++++-
 scripts/mkstandalone.sh |  2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 958321d..ba6c55b 100755
--- a/configure
+++ b/configure
@@ -11,6 +11,7 @@ arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
 host=$arch
 cross_prefix=
 endian=""
+pretty_print_stacks=yes
 
 usage() {
     cat <<-EOF
@@ -25,6 +26,8 @@ usage() {
 	    --prefix=PREFIX        where to install things ($prefix)
 	    --kerneldir=DIR        kernel build directory for kvm.h ($kerneldir)
 	    --endian=ENDIAN        endianness to compile for (little or big, ppc64 only)
+	    --[enable|disable]-pretty-print-stacks
+	                           enable or disable pretty stack printing (enabled by default)
 EOF
     exit 1
 }
@@ -61,6 +64,12 @@ while [[ "$1" = -* ]]; do
 	--ld)
 	    ld="$arg"
 	    ;;
+	--enable-pretty-print-stacks)
+	    pretty_print_stacks=yes
+	    ;;
+	--disable-pretty-print-stacks)
+	    pretty_print_stacks=no
+	    ;;
 	--help)
 	    usage
 	    ;;
@@ -149,4 +158,5 @@ API=$api
 TEST_DIR=$testdir
 FIRMWARE=$firmware
 ENDIAN=$endian
+PRETTY_PRINT_STACKS=$pretty_print_stacks
 EOF
diff --git a/run_tests.sh b/run_tests.sh
index 89e8f84..7e0237f 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -46,7 +46,13 @@ while getopts "g:hv" opt; do
     esac
 done
 
-RUNTIME_arch_run="./$TEST_DIR/run >> test.log"
+if [ "$PRETTY_PRINT_STACKS" = "yes" ]; then
+	log_redir="> >(./scripts/pretty_print_stacks.py \$kernel >> test.log)"
+else
+	log_redir=">> test.log"
+fi
+
+RUNTIME_arch_run="./$TEST_DIR/run $log_redir"
 config=$TEST_DIR/unittests.cfg
 rm -f test.log
 printf "BUILD_HEAD=$(cat build-head)\n\n" > test.log
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 1d3a20d..ef15bc8 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -41,6 +41,8 @@ generate_test ()
 	echo "#!/bin/bash"
 	echo "export STANDALONE=yes"
 	echo "export HOST=\$(uname -m | sed -e s/i.86/i386/ | sed -e 's/arm.*/arm/')"
+	echo "export PRETTY_PRINT_STACKS=no"
+
 	config_export ARCH
 	config_export ARCH_NAME
 	config_export PROCESSOR
-- 
2.7.0.rc3.207.g0ac5344

Re: [PATCH kvm-unit-tests v6 0/5] Debugging aids

[Andrew Jones]

On Thu, Mar 10, 2016 at 04:47:34PM -0800, Peter Feiner wrote:
> Some aids for debugging: stack dumping, register dumping, and printing
> failing assertion conditions.
> 
> Compiled on ppc64, arm64, i386, x86_64.
> 
> v6:
> 	* Don't print dump_stack's call to backtrace() in the stack
> 	  trace rather than having backtrace() skip over the first
> 	  return address in the trace.
> 	* Actually removed lib/asm-generic/stack.c.
> 
> v5:
> 	* Removed unused "b" flag from run_tests.sh
> 	* Removed misleading comment in mkstandalone.sh.
> 
> v4:
> 	* Moved around arch and asm files per Linux style (hopefully).
> 	* Added --[enable|disable]-pretty-print-stacks to ./configure.
> 	  This has obviated the -p command-line option in
> 	  ./run_tests.sh that I introduced in v1.
> 	* Fixed arch-neutral backtrace so backtrace itself isn't
> 	  included.
> 
> v3:
> 	* Renamed functions to match standard backtrace functions
> 	* Added asm-generic stack walker that uses __builtin_return_address.
> 	* Moved offset out of libcflat and into pretty printer
> 	* Made pretty printer more robust w.r.t. errors: now it just
> 	  continues printing.
> 
> v2:
>         * Fixed a bunch tests on x86 that broke with
>           -fno-omit-frame-pointer.
>         * Only use -fno-omit-frame-pointer on x86. I suspect arm & ppc
>           will have similar problems that I fixed for x86.
> 
> 
> 
> Peter Feiner (5):
>   lib: backtrace printing
>   x86: lib: debug dump on unhandled exceptions
>   lib: dump stack on failed assert()
>   scripts: pretty print stack traces
>   scripts: automatically pretty print stacks
> 
>  Makefile                       |  6 ++-
>  configure                      | 10 +++++
>  lib/arm/asm/stack.h            |  0
>  lib/arm64/asm/stack.h          |  0
>  lib/libcflat.h                 |  9 +++-
>  lib/powerpc/asm/stack.h        |  0
>  lib/ppc64/asm/stack.h          |  0
>  lib/stack.c                    | 96 ++++++++++++++++++++++++++++++++++++++++++
>  lib/stack.h                    | 20 +++++++++
>  lib/x86/asm/stack.h            | 14 ++++++
>  lib/x86/desc.c                 | 69 +++++++++++++++++++++++++++---
>  lib/x86/stack.c                | 31 ++++++++++++++
>  run_tests.sh                   |  8 +++-
>  scripts/mkstandalone.sh        |  2 +
>  scripts/pretty_print_stacks.py | 89 +++++++++++++++++++++++++++++++++++++++
>  x86/Makefile.common            |  4 ++
>  16 files changed, 347 insertions(+), 11 deletions(-)
>  create mode 100644 lib/arm/asm/stack.h
>  create mode 100644 lib/arm64/asm/stack.h
>  create mode 100644 lib/powerpc/asm/stack.h
>  create mode 100644 lib/ppc64/asm/stack.h
>  create mode 100644 lib/stack.c
>  create mode 100644 lib/stack.h
>  create mode 100644 lib/x86/asm/stack.h
>  create mode 100644 lib/x86/stack.c
>  create mode 100755 scripts/pretty_print_stacks.py
> 
> -- 
> 2.7.0.rc3.207.g0ac5344
>

Thanks for this series! It looks good to me (my r-b may be added to
all patches)

drew