C calling issue (x86)

[bemk]

Hey,

In my interrupt handler I push the all the registers (all nasm), before calling the interrupt and fault handling code. I always assumed that the C function would only READ from those arguments, but never wrote into them. For the longest time, this has been true, but recently, with a change to the way I handle page faults, I get a general protection fault, because the C code has overwritten the argument on the stack.

I can fix this by inserting a nop instruction at the end of the page fault code, but that just looks strange. I can also turn of the -Os flag, which also seems to work.

My real question is, have I discovered a bug in GCC, or have I just missed something in the C calling conventions, which allows the callee to edit values that are on the stack as arguments.

Compiled with: gcc -o bin/arch-x86-mm-page_table.o -D VERSION="v0.1.3.1-36-g54d41bd" -c -std=gnu89 -nostdlib -fno-builtin -nostdinc -fno-stack-protector -pipe -Os -Wall -Wextra -Iinclude/ -D X86 -D __INTEL -m32 -mtune=native -march=i386 -D SLAB src/arch/x86/mm/page_table.c

For anyone interested, here is the C and assembly code, and the the full repo can be found on github.

I would greatly appreciate your feedback.

Kind regards, Bemk.

Code: Select all

void
x86_pagefault(isrVal_t registers)
{
        addr_t fault_addr = 0;
        asm ("mov %%cr2, %%eax\n\t"
             "mov %%eax, %0\n\t"
             : "=r" (fault_addr)
             :
             : "%eax", "memory");

        /*if (++idx % 0x100 == 0)
        {
                addr_t pde = fault_addr >> 22;
                addr_t pte = (fault_addr >> 12) & 0x3FF;
                int* pt = vpt[pde];
                printf("Fault addr: %X\t", (int)fault_addr);
                printf("pde: %X - %X\t", *(int*)&vpd[pde], (int)pde);
                printf("pte: %X - %X\n", pt[pte], (int)pte);
        }*/

        if (registers.errCode & 4)
        {
                /* User space page faults */
                if (registers.errCode & 2)
                        vm_user_fault_write(fault_addr,(registers.errCode & 1));
                else
                        vm_user_fault_read(fault_addr, (registers.errCode & 1));
        }
        else
        {
                /* Kernel space page faults */
                if (registers.errCode & 2)
                        vm_kernel_fault_write(fault_addr,(registers.errCode&1));
                else
                        vm_kernel_fault_read(fault_addr,(registers.errCode&1));
        }
        /*
         * There is this really weird thing going on here where gcc wants to
         * edit values that are on the stack as arguments. Not sure if this is
         * a bug in gcc, or something I missed in the calling conventions.
         *
         * Adding anything that isn't a function call seems to fix this issue,
         * so hereby the shortest thing I could come up with, the trusty old
         * nop.
         */
        asm("nop");
}

Below is an asm dump of the page fault code.

Code: Select all

00000257 <x86_pagefault>:
 257:	55                   	push   %ebp
 258:	89 e5                	mov    %esp,%ebp
 25a:	8b 55 30             	mov    0x30(%ebp),%edx
 25d:	0f 20 d0             	mov    %cr2,%eax
 260:	89 c1                	mov    %eax,%ecx
 262:	f6 c2 04             	test   $0x4,%dl
 265:	74 23                	je     28a <x86_pagefault+0x33>
 267:	f6 c2 02             	test   $0x2,%dl
 26a:	74 0f                	je     27b <x86_pagefault+0x24>
 26c:	83 e2 01             	and    $0x1,%edx
 26f:	89 4d 08             	mov    %ecx,0x8(%ebp)
 272:	89 55 0c             	mov    %edx,0xc(%ebp)
 275:	5d                   	pop    %ebp
 276:	e9 fc ff ff ff       	jmp    277 <x86_pagefault+0x20>
			277: R_386_PC32	vm_user_fault_write
 27b:	83 e2 01             	and    $0x1,%edx
 27e:	89 4d 08             	mov    %ecx,0x8(%ebp)
 281:	89 55 0c             	mov    %edx,0xc(%ebp)
 284:	5d                   	pop    %ebp
 285:	e9 fc ff ff ff       	jmp    286 <x86_pagefault+0x2f>
			286: R_386_PC32	vm_user_fault_read
 28a:	f6 c2 02             	test   $0x2,%dl
 28d:	74 0f                	je     29e <x86_pagefault+0x47>
 28f:	83 e2 01             	and    $0x1,%edx
 292:	89 4d 08             	mov    %ecx,0x8(%ebp)
 295:	89 55 0c             	mov    %edx,0xc(%ebp)
 298:	5d                   	pop    %ebp
 299:	e9 fc ff ff ff       	jmp    29a <x86_pagefault+0x43>
			29a: R_386_PC32	vm_kernel_fault_write
 29e:	83 e2 01             	and    $0x1,%edx
 2a1:	89 4d 08             	mov    %ecx,0x8(%ebp)
 2a4:	89 55 0c             	mov    %edx,0xc(%ebp)
 2a7:	5d                   	pop    %ebp
 2a8:	e9 fc ff ff ff       	jmp    2a9 <x86_pagefault+0x52>
			2a9: R_386_PC32	vm_kernel_fault_read

[Combuster]

In pretty much any stack-based calling convention, the callee owns the space allocated for its arguments and is free to write them.


If the caller would own that space, it'd have to copy the variables from its own arguments and calculations in place, duplicating everything because they need to be in the correct order for the callee. The callee would then have to create more locations on the stack to store possible intermediate results. If the callee owns it, it may reuse those variables and reduce the needed stack space. And it makes compilers simpler in general.

The typical solution here is to copy the stackpointer at a strategic moment and pass it in as a pointer to a struct registers matching the remaining stack layout. That way the compiler can't modify the struct of its own accord (it doesn't own it), and you can still change the return values of the interrupt by explicitly writing the struct. Plus if you change the push order, you only need to change the struct definition accordingly rather than changing the entire signature.

[bemk]

Thanks for the quick reply.

Now this is useful information. Basically this means I missed something in the C calling convention.
The problem is now solved. Thank very much.

Kind regards, Bemk.