static value got changed in a `while(1)` expression

clfbbn · **Joined:** Mon Dec 02, 2019 9:42 pm **Posts:** 5

Hi there, I am following some guides on writing a toy OS and my source code is located at https://github.com/hellobbn/bbnOS

I am now writing the part of multi-threading and have 2 tasks: testA() and testB() in kmain.c, and the first time interrupt cause the os to execute the testA() function.
However, it triggers a #PF page fault exception immediately.

After some debugging of the code, I found that in testA(), there is a while loop and the assembly looked like this:

Code:

while (1) {                                                   0x3234a            jmp 0x3230b  <testA+27>                      testA+90
             print("- [testA] test A ");                  0x32316            mov %ecx,(%esp)                                  testA+38
                                                                    0x32319            movl $0x0,0x4(%esp)                            testA+41
                                                                    0x32321            movl $0xf,0x8(%esp)                             testA+49
                                                                    0x32329            movl $0x0,0xc(%esp)                            testA+57
                                                                    0x32331            mov %eax,%ebx                                    testA+65
                                                                    0x32333            mov %edx,-0xc(%ebp)                          testA+67
                                                                    0x32336            call 0x32a90 <fb_print_color>               testA+70

and in my fb_print_color() function there defines 2 static value x_pos and y_pos for tracking current print position, and the above code changes y_pos to a very large number, causing the #PF.

The error occurred when I am trying to move some global declaration to `global.c` . My code in master branch is ok and can be run using `make qemu`, however, the code in branch pf, which is 1 commit ahead addresses the issue.

Can someone help me with this, thanks!

iansjack · **Posted:** Sun Dec 22, 2019 12:50 am

Run your code under gdb and set a watch on the variable in question. This will break the program when the variable is altered, allowing you to inspect the state of the machine at that point.

clfbbn · **Joined:** Mon Dec 02, 2019 9:42 pm **Posts:** 5

iansjack wrote:

Run your code under gdb and set a watch on the variable in question. This will break the program when the variable is altered, allowing you to inspect the state of the machine at that point.

Hi, I have run the code with gdb already and the value got changed in the assembly code jmp 0x3230b <testA+27>, which, as you can see, is happened when the program is executing while(1). To be exact, the while(1) has 3 lines as below:

Code:

while(1) {
     print(“testA”)
}

And the change of value happened in the first line, which is while(1)

iansjack · **Posted:** Sun Dec 22, 2019 2:16 am

A jump instruction can't change a memory location, so the fault cannot be there. From what you have shown the most likely cause is the call instruction. Check the value of the stack immediately before it to see if it is overwriting your variable.

clfbbn · **Joined:** Mon Dec 02, 2019 9:42 pm **Posts:** 5

iansjack wrote:

A jump instruction can't change a memory location, so the fault cannot be there. From what you have shown the most likely cause is the call instruction. Check the value of the stack immediately before it to see if it is overwriting your variable.

I still think it is the instruction that caused the issue. In the attachment there are the screenshots of my debugging process, and the place for y_pos is 0x35804.

As you can see, before executing the jmp instruction, the value is 3, however, after the jmp, the higher 2 bytes of the value are changed. I also noticed that the jmp is located at <testA + 90>, which seems not right, and the jmp instruction takes many steps and I have to click `si` many times to get to next instruction, so I think it is executing the right instruction?

The only thing I changed is that I moved some global declaration from header file to source file and in global.h, I used `extern` to mark them. I don't understand why this causes error.

reapersms · **Joined:** Fri Oct 04, 2019 10:10 am **Posts:** 48

That source view is not really telling the truth. Seeing the real disassembly of testA without the source correlation may show more. I would expect at least a mov $imm, %ecx after that, but there is likely more than that.

Questions that may lead to revelation: (-- edits after looking at github)

What is the calling convention using ebx for?
What is the value of esp during this call?
Is 35804 near any of the thread stacks? -- possibly
Is print() a macro or inlined function? that looks more like it's direct calling fb_print_color(<str>, 0, 15, 0) -- a macro that expands to that
You mention the print function is using static globals to track x/y. That sounds like it won't be reentrant, and would probably do strange and wonderful things if two threads printed at the same time. Is it protected with a mutex or other synchronization method? -- it is not

Observations:

Prime candidates for bugs would be the task switching code not getting some of the registers quite right, and the print code being completely non-reentrant.

clfbbn · **Joined:** Mon Dec 02, 2019 9:42 pm **Posts:** 5

reapersms wrote:

That source view is not really telling the truth. Seeing the real disassembly of testA without the source correlation may show more. I would expect at least a mov $imm, %ecx after that, but there is likely more than that.

Questions that may lead to revelation: (-- edits after looking at github)

What is the calling convention using ebx for?
What is the value of esp during this call?
Is 35804 near any of the thread stacks? -- possibly
Is print() a macro or inlined function? that looks more like it's direct calling fb_print_color(<str>, 0, 15, 0) -- a macro that expands to that
You mention the print function is using static globals to track x/y. That sounds like it won't be reentrant, and would probably do strange and wonderful things if two threads printed at the same time. Is it protected with a mutex or other synchronization method? -- it is not

Observations:

Prime candidates for bugs would be the task switching code not getting some of the registers quite right, and the print code being completely non-reentrant.

Thanks for your reply! Using `objdump` to dissemble my source I found that the ebp is very close to the address of y_pos, and there is an instruction: mov -0x8(%ebp), %eax that cause the change of the y_pos value. Now I am figuring out what is causing this to happen. Thanks!

I am using a global array task_stack for the stack and the address of the stack: &task_stack[0] = 0x35812, and &task_stack[1] is 0x35813, as the stack grows to low address, maybe I need to set the higher address to $ebp instead of the lower address?

nullplan · **Joined:** Wed Aug 30, 2017 8:24 am **Posts:** 1605

So this means you have a stack overflow. Haven't seen one of those in a while. Your stack is too small for all the things you are putting onto it, so it starts to overwrite global data. Short term solution: larger stack, and try to trim out large stack allocations. Long term solution: Allocate your stack somewhere else in the address space and put guard pages below it, so it crashes when the stack overflows instead of corrupting arbitrary state.

clfbbn · **Joined:** Mon Dec 02, 2019 9:42 pm **Posts:** 5

nullplan wrote:

So this means you have a stack overflow. Haven't seen one of those in a while. Your stack is too small for all the things you are putting onto it, so it starts to overwrite global data. Short term solution: larger stack, and try to trim out large stack allocations. Long term solution: Allocate your stack somewhere else in the address space and put guard pages below it, so it crashes when the stack overflows instead of corrupting arbitrary state.

Hi, it is not actually a stack overflow, instead, I mistakenly choose the higher address as the base of the stack, as the example below shows:

Code:

char task_stack[100];          // the stack
p_task->stack = tack_stack;  // wrong
p_task->stack = task_stack + 100 - 1; // correct

And thank you for mentioning the solution, and hopefully I will implement that, thanks!

nullplan · **Joined:** Wed Aug 30, 2017 8:24 am **Posts:** 1605

Actually, you can initialize the stack pointer to point behind the stack. The push instruction decrements the stack pointer first and then writes stuff to the resulting address. As do all pushing actions of the CPU (including the pushes it does after an interrupt). But I don't know how you are using the stack pointer variable there, didn't look into your code.

Also, an array of 32-bit values should model a CPU stack better, since it automatically gets a 32-bit alignment. JM2C.

OSDev.org

static value got changed in a `while(1)` expression

Who is online