OSDev.org

Posted: **Tue Feb 07, 2012 6:40 pm**

I'm trying to compile my kernel, and all worked fine, until I turned on optimization to -O3.

The relevant code is as follows (A lot has been removed to locate the error, so it's not the actual code):

Code: Select all

void outportb(uint16_t port, uint8_t value);

void isr_handler(isr_argument_t regs)
{
  if(__LIKELY(regs.int_nr >= IRQ0 && regs.int_nr <= IRQ15)) {
    if(regs.int_nr >= IRQ7) {
      outportb(0xA0, 0x20);
    }

    outportb(0x20, 0x20);
  }
}

And outportb:

Code: Select all

outportb:
	mov	dx, word [esp+4]			; dx = port
	mov	al, byte [esp+8]			; al = value
	out	dx, al
	ret

However, this gets compiled to the following:

Code: Select all

push   %ebp
mov    %esp,%ebp
sub    $0x18,%esp
mov    0x2c(%ebp),%eax
lea    -0x20(%eax),%edx
cmp    $0xf,%edx
ja     c0101926 <isr_handler+0x46>
cmp    $0x26,%eax
ja     c0101910 <isr_handler+0x30>
movl   $0x20,0xc(%ebp)                       ; ?????????????????????
movl   $0x20,0x8(%ebp)                       ; ?????????????????????
leave  
jmp    c0103100 <outportb>

I've traced the error back to the lines with the question marks. Those shouldn't be there, and I'm not sure how they ever came to be.
It seems like it comes from the outportb(0x20, 0x20) instruction. However, why does this function overwrite the parameters, even though the calling function may still actually use them (one of them happens to be the segment register value, causing a crash)?

I guess I have to specify to GCC that I don't want it to wreck the parameters. How do I do that?

PS: This function is only called from assembly language.

Posted: **Tue Feb 07, 2012 7:25 pm**

Hi,

That looks like a perfectly valid tail call optimisation to me.

The problem is that you want "isr_handler()" not to modify it's own arguments. You could try "void isr_handler(const isr_argument_t regs)" but then you won't be able to change the caller's registers if/when you need to.

The normal approach would be to pass a pointer to the structure rather than passing the structure itself, with "void isr_handler(isr_argument_t *pointer_to_regs)". This has the added advantage that later on you could do something like "return kernel_API(pointer_to_regs)" and it will work (rather than creating a copy of the entire structure to pass to "kernel_API()" so that "kernel_API()" can only modify a discarded copy).

Cheers,

Brendan

Posted: **Tue Feb 07, 2012 8:29 pm**

Thanks for your answer!

I've tried making the structure const, but it didn't fix it.
But your other idea sounded like a good solution to me. Good point about passing it as a parameter to another function - at first I didn't want to waste the instructions, but it will probably be faster.

Posted: **Wed Feb 08, 2012 12:16 am**

How are you calling the code? If you do it from assembly you'll have to remember that there is no convention that arguments on the stack must be preserved.

OSDev.org

GCC ruins stack

GCC ruins stack

Re: GCC ruins stack

Re: GCC ruins stack

Re: GCC ruins stack