Broken Stack Tracer

[arraystock]

I've been trying to get this stack tracer to work, but I always get a page fault.

Code: Select all

void kstrace(int depth) {
  unsigned int *ebp = &depth - 2;
  printf("Stack trace:\n");
  for (unsigned int frame = 0; frame < depth; ++frame) {
    unsigned int eip = ebp[1];
    if (eip == 0)
      break;
    ebp = (unsigned int *)(ebp[0]);
    unsigned int *arguments = &ebp[2];
    printf("  %02X\n", eip);
  }
}

As you can see, it's copied pretty much straight from the wiki.

If I use GDB, I can see what's going on:

Code: Select all

Remote debugging using :1234
k_main (mboot=0x10000) at src/kernel/kernel.c:104
104	  kstrace(10);
(gdb) continue
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x0010472e in kstrace (depth=10) at src/libk/stdlib.c:126
126	    unsigned int eip = ebp[1];
(gdb) stop
(gdb) backtrace
#0  0x0010373e in kstrace (depth=10) at src/libk/stdlib.c:126
#1  0x0010310d in k_main (mboot=0x10000) at src/kernel/kernel.c:104
#2  0x00104d14 in start ()
(gdb) 

And on the kernel's ouput, I see this:

Code: Select all

Stack trace:
  0x0010310D
  0x00104D14
  0xF000FF53
(Page Fault)

It's like it's shifted. I'm new to using GDB, stack tracing, and debugging in general. It's never something I've done or used before.
If you need a register dump, let me know.

I don't know what I need to do here, so that's why I'm asking for help. Thanks in advance!

[Octocontrabass]

Code: Select all

  0xF000FF53

That's one of the interrupt vectors in the IVT. You've run out of stack frames, but you're continuing to trace into garbage. The wiki mentions something about that...

Note that the above code requires a NULL return address, and GDB backtracing requires a NULL %ebp, to know when to stop. Otherwise the traces will run off into garbage. [...] You can use call in place of push/jmp, but your tracer will need to check for a NULL %ebp, rather than a NULL return address.

Did you properly set up the stack before jumping to your kernel?

[arraystock]

In my start function I do set up the stack and with a NULL return address. Maybe I'm doing this incorrectly?

Code: Select all

extern enable_paging
extern k_main

section .multiboot

align 4
dd 0x1BADB002                   ; Magic.
dd 0x00                         ; Flags.
dd - (0x1BADB002 + 0x00)        ; Checksum.

section .text

global start
start:
  cli                   ; Disable interrupts.
  call enable_paging    ; Enable paging and remap kernel to 3GB.
  mov esp, stack        ; Set up the stack.
  xor ebp, ebp          ; Prepare NULL stack return address so traces know where to stop.
  push ebp
  push ebx              ; Load multiboot header location.
  call k_main           ; Call k_main().
  jmp $                 ; Loop forever.

section .bss
resb 8192               ; Measured in bytes, 8KB.
stack:

[Octocontrabass]

You're using CALL instead of the wiki example's PUSH/JMP, so the return address is not null how the example function expects. You'll have to either change how you set up the stack, or change the function to expect the stack you set up.

[arraystock]

I changed the function to check ebp and it works now. Thank you!

[Korona]

Where on the wiki is that? The code is broken: the expression ebp = &depth + 2 is undefined behavior (at least once you dereference ebp). Making assumptions about the layout of the stack frame is not possible. The compiler is allowed to move the variable depth to a different location e.g. when the function is inlined. This code will indeed break with -O2 on clang (something noticed by the member qookie of this forum). It should use something like

Code: Select all

asm ("mov %%ebp, %0" : "r"(ebp));

instead. Can you point me to the wiki article so that I can update it?

[qookie]

Korona: This is the article https://wiki.osdev.org/Stack_Trace they're most likely talking about(it does the same thing and there doesn't seem to be any other wiki page about stack traces)

[nullplan]

I took the liberty of performing this change. Also, to add a type for the stack frame, so we don't have to cast all our variables like we're desparately trying not to use assembly in the first place. Seriously, what's the point of writing a stack tracer in C if you are going to make your working pointer a pointer to unsigned int? And then cast when you dereference to the lower level.

Also, removed the references to the null return value. The code was already zeroing out EBP, so at the end of the trace, we will see a frame with EBP = 0 and EIP pointing to the kernel's start function. We can just stop there.

EDIT: I also updated the assembly tracer. It had the same stop condition, which I fixed, and also failed to actually allocate the stack frame necessary to save the registers it was saving. Oh, and adding the leave instruction may be gratuitous, but is recommended in the software optimization guide. Weirdly, the enter instruction is not recommended.

[Korona]

That's great. Thank you for changing it, the old version was not the greatest piece of code on the wiki to say the least.