x86: Upgrade backtrace
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 19 Jul 2016 23:02:23 +0000 (19:02 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 27 Jul 2016 16:52:43 +0000 (12:52 -0400)
Previously, we would report zero PCs if the FP was 0.  Now we'll at least
report the PC we were given.

Now that we're using two different functions for the kernel and user, we
can be more careful in the kernel's function when backtracing.

Finally, this is more clear with regards to the end conditions for the
loop.

But wait, you say, isn't that fp check in the kernel's BT unnecessarily
slow?  fp < KERNBASE, so we don't need to write the far more
understandable:

if (!fp || fp < KERNBASE)
break;

and we can write this instead!

if (fp < KERNBASE)
break;

Those generate the same code.  Don't be too clever, the compiler might
outsmart you.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/kdebug.c

index 9254765..139353f 100644 (file)
@@ -331,10 +331,11 @@ size_t backtrace_list(uintptr_t pc, uintptr_t fp, uintptr_t *pcs,
                       size_t nr_slots)
 {
        size_t nr_pcs = 0;
-       while ((fp > MMAP_LOWEST_VA) && (nr_pcs < nr_slots)) {
-               /* could put some sanity checks in here...  i used to at least check for
-                * kernel addrs, but now we also bt user stacks. (dangerous!) */
+
+       while (nr_pcs < nr_slots) {
                pcs[nr_pcs++] = pc;
+               if (!fp || fp < KERNBASE)
+                       break;
                printd("PC %p FP %p\n", pc, fp);
                /* We used to set PC = retaddr - 1, where the -1 would put our PC back
                 * inside the function that called us.  This was for obscure cases where
@@ -353,17 +354,16 @@ size_t backtrace_user_list(uintptr_t pc, uintptr_t fp, uintptr_t *pcs,
        size_t nr_pcs = 0;
        uintptr_t frame[2];
 
-       for (;;) {
+       while (nr_pcs < nr_slots) {
+               pcs[nr_pcs++] = pc;
+               if (!fp)
+                       break;
                error = copy_from_user(frame, (const void *) fp, 2 * sizeof(uintptr_t));
-               if (unlikely(error) || unlikely(nr_pcs >= nr_slots))
+               if (unlikely(error))
                        break;
-
-               /* For now straight memory access, waiting for copy_from_user(). */
-               pcs[nr_pcs++] = pc;
                pc = frame[1];
                fp = frame[0];
        }
-
        return nr_pcs;
 }