Debugging aids

Signed-off-by Peter Feiner <[email protected]>

---
 lib/libcflat.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

lib/libcflat.h +2 -1

index 39100f2..1f0049c 100644
@@ -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)
 

Signature · 6 lines

2.7.0.rc3.207.g0ac5344

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

Makefile cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \ · line 45

Quoted 2 lines · Peter Feiner

Makefile +1 -1

-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.

Signed-off-by Peter Feiner <[email protected]>

---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

lib/libcflat.h +4 -3

index 42c94df..1798716 100644
@@ -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

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 <[email protected]>

---
 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

run_tests.sh +8 -2

index 2c8af36..13bc089 100755
@@ -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

scripts/pretty_print_stacks.py new file +51 -0

new file mode 100755
index 0000000..49564bd
@@ -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:

Quoted 137 lines · 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 <[email protected]>

---
 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

run_tests.sh +8 -2

index 2c8af36..13bc089 100755
@@ -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

scripts/pretty_print_stacks.py new file +51 -0

new file mode 100755
index 0000000..49564bd
@@ -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:

scripts/pretty_print_stacks.py line 61

Quoted 10 lines

scripts/pretty_print_stacks.py new file +10 -0

+                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.

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.

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.

Quoted 3 lines · Andrew Jones

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.

Quoted 9 lines

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

scripts/mkstandalone.sh +0 -0

index c7e78f8..d6607b8 100755
@@ -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/')"

scripts/mkstandalone.sh +4 -0

+
+	# 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.

Quoted 5 lines

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)?

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 <[email protected]>

---
 x86/eventinj.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

x86/eventinj.c +0 -0

index bddedce..202ac19 100644

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 <[email protected]>

---
 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

Makefile +2 -1

index ddba941..40ea4ec 100644
@@ -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, "")
 

arm/Makefile.common +1 -0

index dd3a0ca..054bdee 100644
@@ -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

lib/arm/dump_stack.c new file +6 -0

new file mode 100644
index 0000000..528ba63
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	return 0;
+}

lib/libcflat.h +0 -0

index 1f0049c..42c94df 100644

lib/libcflat.h +1 -0

+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({				\

lib/powerpc/dump_stack.c new file +6 -0

new file mode 100644
index 0000000..528ba63
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+	return 0;
+}

lib/printf.c +2 -0

index 2aec59a..e97fca9 100644
@@ -259,3 +259,40 @@ int printf(const char *fmt, ...)
     puts(buf);
     return r;
 }
+
+static void print_stack(unsigned long *stack, int depth,

lib/printf.c +5 -0

+	}
+	printf("\n");
+}
+
+#define MAX_DEPTH 10

lib/printf.c +10 -0

+
+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);
+}

lib/x86/dump_stack.c new file +24 -0

new file mode 100644
index 0000000..6e9d126
@@ -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;
+}

powerpc/Makefile.common +1 -0

index 2ce6494..694dea0 100644
@@ -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)

x86/Makefile.common +4 -0

index 3a14fea..d7c7eab 100644
@@ -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)

Signature · 1 line

2.7.0.rc3.207.g0ac5344

Quoted 70 lines · 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 <[email protected]>

---
 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

Makefile +2 -1

index ddba941..40ea4ec 100644
@@ -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, "")
 

arm/Makefile.common +1 -0

index dd3a0ca..054bdee 100644
@@ -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

lib/arm/dump_stack.c new file +6 -0

new file mode 100644
index 0000000..528ba63
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+     return 0;
+}

lib/libcflat.h +0 -0

index 1f0049c..42c94df 100644
@@ -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);
 

lib/libcflat.h extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...); · line 70

Quoted 3 lines

lib/libcflat.h +3 -0

+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.

Quoted 31 lines

lib/libcflat.h +1 -0

+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({                           \

lib/powerpc/dump_stack.c new file +6 -0

new file mode 100644
index 0000000..528ba63
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+     return 0;
+}

lib/printf.c +7 -0

index 2aec59a..e97fca9 100644
@@ -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: " );

lib/printf.c int printf(const char *fmt, ...) · line 273

Quoted 5 lines

lib/printf.c +5 -0

+     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().

Quoted 10 lines

lib/printf.c +9 -0

+     }
+     printf("\n");
+}
+
+#define MAX_DEPTH 10
+
+void dump_stack(unsigned long ip, unsigned long bp)
+{
+     unsigned long stack[MAX_DEPTH];

lib/printf.c int printf(const char *fmt, ...) · line 288

Quoted 6 lines

lib/printf.c +6 -0

+     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?

Quoted 74 lines

lib/printf.c +10 -0

+
+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);
+}

lib/x86/dump_stack.c new file +24 -0

new file mode 100644
index 0000000..6e9d126
@@ -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;
+}

powerpc/Makefile.common +1 -0

index 2ce6494..694dea0 100644
@@ -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)

x86/Makefile.common +4 -0

index 3a14fea..d7c7eab 100644
@@ -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)
 

Quoted 3 lines

x86/Makefile.common +0 -1

 FLATLIBS = lib/libcflat.a $(libgcc)
--

2.7.0.rc3.207.g0ac5344

Thanks,
drew

Quoted 71 lines · Andrew Jones

Quoted 70 lines · 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 <[email protected]>

---
 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

Makefile +2 -1

index ddba941..40ea4ec 100644
@@ -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, "")
 

arm/Makefile.common +1 -0

index dd3a0ca..054bdee 100644
@@ -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

lib/arm/dump_stack.c new file +6 -0

new file mode 100644
index 0000000..528ba63
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+     return 0;
+}

lib/libcflat.h +0 -0

index 1f0049c..42c94df 100644
@@ -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);
 

Quoted 7 lines

lib/libcflat.h extern void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...); · line 70

Quoted 3 lines

lib/libcflat.h +3 -0

+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".

Quoted 5 lines

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 :-)

Quoted 9 lines

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.

Quoted 31 lines

Quoted 31 lines

lib/libcflat.h +1 -0

+
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
 
 #define container_of(ptr, type, member) ({                           \

lib/powerpc/dump_stack.c new file +6 -0

new file mode 100644
index 0000000..528ba63
@@ -0,0 +1,6 @@
+#include "libcflat.h"
+
+int walk_stack(unsigned long bp, unsigned long *stack, int max_depth)
+{
+     return 0;
+}

lib/printf.c +7 -0

index 2aec59a..e97fca9 100644
@@ -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: " );

Quoted 8 lines

lib/printf.c int printf(const char *fmt, ...) · line 273

Quoted 5 lines

lib/printf.c +5 -0

+     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.

Quoted 11 lines

Quoted 11 lines

lib/printf.c +9 -0

+     }
+     printf("\n");
+}
+
+#define MAX_DEPTH 10
+
+void dump_stack(unsigned long ip, unsigned long bp)
+{
+     unsigned long stack[MAX_DEPTH];

Quoted 8 lines

lib/printf.c int printf(const char *fmt, ...) · line 288

Quoted 6 lines

lib/printf.c +6 -0

+     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.

Quoted 74 lines

Quoted 74 lines

lib/printf.c +10 -0

+
+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);
+}

lib/x86/dump_stack.c new file +24 -0

new file mode 100644
index 0000000..6e9d126
@@ -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;
+}

powerpc/Makefile.common +1 -0

index 2ce6494..694dea0 100644
@@ -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)

x86/Makefile.common +4 -0

index 3a14fea..d7c7eab 100644
@@ -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)
 

Quoted 6 lines

Quoted 3 lines

x86/Makefile.common +0 -1

 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signed-off-by Peter Feiner <[email protected]>

---
 lib/libcflat.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

lib/libcflat.h +4 -2

index 42c94df..1798716 100644
@@ -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

Signature · 6 lines

2.7.0.rc3.207.g0ac5344

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

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 <[email protected]>

---
 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

run_tests.sh +8 -2

index 2c8af36..13bc089 100755
@@ -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

scripts/pretty_print_stacks.py new file +61 -0

new file mode 100755
index 0000000..49564bd
@@ -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)

Signature · 1 line

2.7.0.rc3.207.g0ac5344

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.

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 <[email protected]>

---
 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

Makefile +2 -1

index 72e6711..536f2e6 100644
@@ -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, "")
 

arm/Makefile.common +1 -0

index dd3a0ca..ede8851 100644
@@ -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

lib/arm/asm/stack.h new file +1 -0

new file mode 100644
index 0000000..f101d03
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>

lib/arm64/asm/stack.h new file +1 -0

new file mode 100644
index 0000000..f101d03
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>

lib/asm-generic/stack.c new file +0 -0

new file mode 100644
index 0000000..93219cb

lib/asm-generic/stack.c new file +51 -0

+
+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;
+}

lib/asm-generic/stack.h new file +14 -0

new file mode 100644
index 0000000..381fce4
@@ -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

lib/libcflat.h +3 -0

index b58a8a1..55bddca 100644
@@ -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) ({				\

lib/powerpc/asm/stack.h new file +1 -0

new file mode 100644
index 0000000..f101d03
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>

lib/ppc64/asm/stack.h new file +1 -0

new file mode 100644
index 0000000..f101d03
@@ -0,0 +1 @@
+#include <asm-generic/stack.h>

lib/printf.c +13 -0

index 2aec59a..096e175 100644
@@ -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]);

lib/printf.c +19 -0

+
+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);
+}

lib/x86/asm/stack.c new file +25 -0

new file mode 100644
index 0000000..b30b3b1
@@ -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;
+}

lib/x86/asm/stack.h new file +12 -0

new file mode 100644
index 0000000..c43bb96
@@ -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

powerpc/Makefile.common +1 -0

index 424983e..ea8f669 100644
@@ -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)

x86/Makefile.common +4 -0

index 3a14fea..c4750f0 100644
@@ -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)

Signature · 1 line

2.7.0.rc3.207.g0ac5344

Signed-off-by Peter Feiner <[email protected]>

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 <[email protected]>

---
 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

run_tests.sh +4 -1

index 89e8f84..b6f2003 100755
@@ -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

Quoted 56 lines · 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 <[email protected]>

---
 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

run_tests.sh +3 -0

index 89e8f84..b6f2003 100755
@@ -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"
 

run_tests.sh while getopts "g:hv" opt; do · line 48

Quoted 12 lines

run_tests.sh +4 -1

-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?

Quoted 57 lines · Andrew Jones

Quoted 56 lines · 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 <[email protected]>

---
 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

run_tests.sh +3 -0

index 89e8f84..b6f2003 100755
@@ -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"
 

Quoted 15 lines

run_tests.sh while getopts "g:hv" opt; do · line 48

Quoted 12 lines

run_tests.sh +4 -1

-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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html