apsun / loliOS

Lightweight & operational Linux-inspired OS.
33 stars 1 forks source link

Register/memory corruption when compiled with gcc -Og #36

Closed apsun closed 8 months ago

apsun commented 1 year ago

Repro: /build.sh -rOg run, hit any key, instant BSOD in terminal_tty_read. Backtrace:

Exception: Page-fault exception (14)

Registers:
eax: 0xfffffffd     ebx: 0xfffffffd     ecx: 0x00000001     edx: 0x00000466
esi: 0x0043e020     edi: 0x00000000     ebp: 0x0041bf54     esp: 0x00000004
cr0: 0x80000011     cr2: 0x0000000c     cr3: 0x00415000     cr4: 0x00000010
eip: 0x0040df60  eflags: 0x00000046   error: 0x00000000
cs: 0x0010   ds: 0x0018   es: 0x0018   fs: 0x0018   gs: 0x0018   ss: 0x819c

Backtrace:
 at 0x0040df60 (0x00800050, 0x08400008, 0x00000400, 0x00407b33, 0x00000008)
 at 0x0040144f (0x00000000, 0x08400008, 0x00000400, 0x08049c0a, 0x0804e56c)
 at 0x00402f5e (0x00000000, 0x00000000, 0x083ffeb8, 0x00400164, 0x0041bfc0)
 at 0x00402fb9 (0x0041bfc0, 0x083ffeb8, 0x0804e56c, 0x08049c0a, 0x00000400)
 at 0x00400164 (0x00000000, 0x00000000, 0x083ffef8, 0x0804ad97, 0x00000000)
 at 0x0804a19d (0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000)
 at 0x0804ad97 (0x083fff3f, 0x00000081, 0x0804e56c, 0x00000000, 0x00000000)
 at 0x0804ae43 (0x083fff3f, 0x00000081, 0x00000000, 0x00000000, 0x00000000)

I suspect an optimization in a new version of GCC is breaking something in the scheduler code. Strangely this only happens in -Og; levels 0, 1, 2, 3 don't exhibit this behavior. clang doesn't do this either.

$ gcc --version
gcc (GCC) 12.2.1 20230201
apsun commented 8 months ago

I think what's going on is that no_caller_saved_registers doesn't do what I thought it does. In my mind, I thought it meant "save all registers", but in reality it means "save all registers that are clobbered". If a register isn't used in the code, it won't actually save it.

(gdb) disas scheduler_yield_impl
Dump of assembler code for function scheduler_yield_impl:
   0x00408203 <+0>: push   %ebp
   0x00408204 <+1>: mov    %esp,%ebp
   0x00408206 <+3>: push   %esi
   0x00408207 <+4>: push   %ebx
   0x00408208 <+5>: push   %ecx
   0x00408209 <+6>: push   %edx
   0x0040820a <+7>: push   %eax
   0x0040820b <+8>: sub    $0x4,%esp
   0x0040820e <+11>:    mov    0x8(%ebp),%esi
   0x00408211 <+14>:    call   0x408179 <scheduler_next_pcb>
   0x00408216 <+19>:    cmp    %esi,%eax
   0x00408218 <+21>:    je     0x408248 <scheduler_yield_impl+69>
   0x0040821a <+23>:    mov    %eax,%ebx
   0x0040821c <+25>:    test   %esi,%esi
   0x0040821e <+27>:    je     0x40823c <scheduler_yield_impl+57>
   0x00408220 <+29>:    mov    %esp,%edx
   0x00408222 <+31>:    mov    %ebp,%eax
   0x00408224 <+33>:    mov    %edx,0xd4(%esi)
   0x0040822a <+39>:    mov    %eax,0xd8(%esi)
   0x00408230 <+45>:    sub    $0xc,%esp
   0x00408233 <+48>:    push   %esi
   0x00408234 <+49>:    call   0x406ef8 <process_unset_context>
   0x00408239 <+54>:    add    $0x10,%esp
   0x0040823c <+57>:    mov    0x4(%ebx),%eax
   0x0040823f <+60>:    test   %eax,%eax
   0x00408241 <+62>:    je     0x408252 <scheduler_yield_impl+79>
   0x00408243 <+64>:    cmp    $0x1,%eax
   0x00408246 <+67>:    je     0x40825b <scheduler_yield_impl+88>
   0x00408248 <+69>:    lea    -0x14(%ebp),%esp
   0x0040824b <+72>:    pop    %eax
   0x0040824c <+73>:    pop    %edx
   0x0040824d <+74>:    pop    %ecx
   0x0040824e <+75>:    pop    %ebx
   0x0040824f <+76>:    pop    %esi
   0x00408250 <+77>:    pop    %ebp
   0x00408251 <+78>:    ret
   0x00408252 <+79>:    sub    $0xc,%esp
   0x00408255 <+82>:    push   %ebx
   0x00408256 <+83>:    call   0x406f67 <process_run>
   0x0040825b <+88>:    sub    $0xc,%esp
   0x0040825e <+91>:    push   %ebx
   0x0040825f <+92>:    call   0x406f11 <process_set_context>
   0x00408264 <+97>:    mov    0xd4(%ebx),%eax
   0x0040826a <+103>:   mov    0xd8(%ebx),%edx
   0x00408270 <+109>:   mov    %eax,%esp
   0x00408272 <+111>:   mov    %edx,%ebp
   0x00408274 <+113>:   add    $0x10,%esp
   0x00408277 <+116>:   jmp    0x408248 <scheduler_yield_impl+69>

Notice that edi is not pushed onto the stack, which is exactly the register which is corrupted:

   0x0040de7b <+72>:    call   0x40d907 <terminal_tty_get_readable_bytes>
   0x0040de80 <+77>:    mov    %eax,%ebx
   0x0040de82 <+79>:    cmp    $0xfffffffd,%eax
   0x0040de85 <+82>:    jne    0x40dea8 <terminal_tty_read+117>
=> 0x0040de87 <+84>:    cmpl   $0x0,0xc(%edi)

Honestly I'm surprised scheduler_yield_impl even works at all, the whole thing should be rewritten in asm

apsun commented 8 months ago

Fixed in 9e12482381ee121cbf7facef5d61029bb8a068bf (hopefully)