'sti' cause crash.. need some help

~ · Post by ~ » Wed Mar 07, 2007 2:37 pm

Assuming everything is OK, it could be that the compiler used renders wrong addresses and writes to any location other than the intended one.

First, I wasn't able to read the contents of the floppy once I removed it and put it in again. Maybe there's something wrong with the floppy MBR.

Other thing that I did was to remove the code from kernel that executes LGDT and kept interrupts enabled. I loaded it alternatively without GRUB by jumping directly into 8:101030

It gave me a message like:

Code: Select all

Whoops! System Halted!

Division By Zero

At least it responded to interrupts and printed a message. If i had loaded the GDT it had directly restarted without messages.

I also have attached an image of the original sample disk. The only thing I did was to copy your original KERNEL.EXE and add a boot entry that says "MSVC Kernel". Everything else from that image is unchanged.

But it will give you a message like:

Code: Select all

Whoops! System Halted!

General Protection Fault

That's why I assume that the compiler used rendered wrong code for building the GDT and probably other parts, because by not loading it, at least it gives a "Division By Zero" message which tells us that it indeed responded to interrupts.

Gnippel · Post by **Gnippel** » Wed Mar 07, 2007 2:47 pm

@~: wow! thanks for all the testing! that's really helpfull!

so, it turns out to be the PshPack thing ?
then what is the alternative of '__attribute__((packed))' for msvc?
anybody knows?

thanks!

Combuster · Post by **Combuster** » Wed Mar 07, 2007 2:52 pm

last hint:

Code: Select all

void isrs_install()
{
    idt_set_gate(0, (unsigned)isr0, 0x08, 0x8E);
    idt_set_gate(1, (unsigned)isr1, 0x08, 0x8E);
    idt_set_gate(2, (unsigned)isr2, 0x08, 0x8E);
/*...*/
    idt_set_gate(29, (unsigned)isr29, 0x08, 0x8E);
    idt_set_gate(30, (unsigned)isr30, 0x08, 0x8E);
    idt_set_gate(31, (unsigned)isr31, 0x08, 0x8E);
}

and the PIC generates interrupt 32 (!)

for the rest, a session using bochs debugger reveals nothing wrong with the compiler, gdt structures or whatever. My guess is that vs automatically enforces packed structures.

Gnippel · Post by **Gnippel** » Wed Mar 07, 2007 3:05 pm

Combuster wrote:last hint:
Code: Select all
void isrs_install()
{
    idt_set_gate(0, (unsigned)isr0, 0x08, 0x8E);
    idt_set_gate(1, (unsigned)isr1, 0x08, 0x8E);
    idt_set_gate(2, (unsigned)isr2, 0x08, 0x8E);
/*...*/
    idt_set_gate(29, (unsigned)isr29, 0x08, 0x8E);
    idt_set_gate(30, (unsigned)isr30, 0x08, 0x8E);
    idt_set_gate(31, (unsigned)isr31, 0x08, 0x8E);
}
and the PIC generates interrupt 32 (!)

for the rest, a session using bochs debugger reveals nothing wrong with the compiler, gdt structures or whatever. My guess is that vs automatically enforces packed structures.

yes, i know..
that is in irq_install();

void irq_install()
{
irq_remap();

idt_set_gate(32, (unsigned)irq0, 0x08, 0x8E);
idt_set_gate(33, (unsigned)irq1, 0x08, 0x8E);
/* ..... */
idt_set_gate(46, (unsigned)irq14, 0x08, 0x8E);
idt_set_gate(47, (unsigned)irq15, 0x08, 0x8E);
}

and in kernel.c:
void main(unsigned long magic, unsigned long addr)
{
gdt_install();
idt_install();
isrs_install();
irq_install();
init_video();
timer_install();
keyboard_install();

__asm sti;

for( ; ; );
}

i don't see anything wrong there...

pcmattman · Post by **pcmattman** » Wed Mar 07, 2007 3:26 pm

Try this

Code: Select all

__asm__ __volatile__ ( "sti" );

Gnippel · Post by **Gnippel** » Wed Mar 07, 2007 3:47 pm

pcmattman wrote:Try this
Code: Select all
__asm__ __volatile__ ( "sti" );

i'm using visual studio.. so that can't be done like that there..
__asm sti or:
__asm {
sti
}
is the right to do in msvc..

frank · Post by **frank** » Wed Mar 07, 2007 5:52 pm

When I run it in bochs it crashes on iret, with an invalid CS. It looks to me like you forgot to pop something off of the stack that you pushed up there. However, I didn't have time to look at the sources, so I can't be sure.

~ · Post by ~ » Wed Mar 07, 2007 11:21 pm

If you have a disassembler/hex editor like HIEW or W32Dasm, you will be able to see that the Visual C binary has opcode bytes that probably shouldn't exist, such as 0xCC for INT 3 instructions (lots of them), whereas the original KERNEL.BIN file doesn't even have not one of them.

The best thing would be to have functional 16/32 assembly knowledge to distinguish what is being compiled and what is additional (and bad) for our program. Maybe it's possible to produce a rawer code with MSVC, maybe looking to disable every debugging functionality, etc.

Maybe taking out those INT 3 codes would help. If not, it would be ideal to examine the contents of both the IDT(R) and GDT(R) as well as the rest of registers just before the point where it crashes, which we'll have to find by intensive testing in the worst case.

~ · Post by ~ » Thu Mar 08, 2007 12:28 am

Well, I think I found the VERY problem of this VERY kernel version.

A 16-bit iret (IRET instead of IRETD, returns 16-bit regs, not 32-bit) is causing a stack corruption. It looks like only half of the registers are being restored when returning from INTERRUPT. Try changing the opcode prefix for 16-bit registers operation, 0x66, by a simple NOP instruction 0x90 to solve it. It must be done because bytes cannot be easily removed from a readily linked executable without getting further instructions with wrong addresses.

Look at the byte at the physical, actual offset 0x0000169C inside the file HACK.EXE and KERNEL.EXE:

------------------------------------
| 169C: 66 CF IRET | 16-bit registers
------------------------------------

Modified by:

------------------------------------
| 169C: 90 CF IRETD | 32-bit registers
------------------------------------

I have attached a new disk image with a new entry called "Hacked MSVC Kernel". Note that I didn't recompile it with Visual C, the only thing I did was to change the byte 0x66 by a NOP 0x90.

It is sure that if you keep using MSVC and if there is really not a way of avoiding and making sure that it isn't putting prefixes for 16-bit instructions in 32-bit code, then you will have to be editing manually your machine code in the worst case, and is very likely that there will be many more hard to find instructions with problems like this or other not foreseen at the moment.

Maybe it would be better to use the original tools (mostly GCC/NASM, etc.) indicated by each tutorial for not overcomplicating things and not facing nearly impossible bugs.

I hope that the attached disk image works fine for you as well, but you must bear in mind that whenever you recompile this code or code like this you will keep facing this problem. My last bet is that MSVC is producing 16-bit code (isn't it MSVC6, or maybe its built-in assembler produces it for low level inline assembly like IRET?). Maybe there is some way to produce fully 32-bit code as in nasm by something like:

Code: Select all

a32 iret

Which in fact would only produce the opcode byte for 0xCF, automatically interpreted as a 32-bit interrupt return if in protected mode.

------------------------------------------

HINT: Try replacing all the occurrences of the mnemonic IRET with IRETD (note the D at the end denoting that it will pop DWORDs out of the stack on interrupt return); maybe that is the easy fix.

Gnippel · Post by **Gnippel** » Thu Mar 08, 2007 1:09 am

@~: wow! you where right!
it's 2 places in the sources where there are iret, now i've changed it to iretd and it works!
i never even thought about that being where the problem was..

thank you so much!

os64dev · Post by **os64dev** » Thu Mar 08, 2007 1:09 am

Gnippel wrote:@~: wow! thanks for all the testing! that's really helpfull!

so, it turns out to be the PshPack thing ?
then what is the alternative of '__attribute__((packed))' for msvc?
anybody knows?

thanks!

Guess it should be something like:

Code: Select all

#pragma pack(push, 1) //- packing is now 1 byte and store old
struct _packed_thingy {
    char x;
    short y;
    int z
} packed_thingy;
#pragma pack(pop) //- restore old packing alignment

sizeof(packed_thingy) should be 7 this is also how you can test your structures simply do a sizeof and see if they have the correct size.

otherwise use the compiler switch /Zp1

the iretd sollutions rings a bell i had the same problem with 64bit code. I used iretd but i should have used iretq.
regards

~ · Post by ~ » Thu Mar 08, 2007 2:16 am

I have always wondered why it has been done like that by Intel/AMD, having 4 or so combinations for a simple IRET functionality that should be consistent with the current processor mode instead of having real mode IRET (0xCF), 16-bit protected mode (0x66 0xCF), 32-bit protected mode (0xCF) and 64-bit mode (0x48 0xCF) by means of a confusing prefix or no prefix at all...

Candy · Post by **Candy** » Thu Mar 08, 2007 10:21 am

os64dev wrote:
Code: Select all
#pragma pack(push, 1) //- packing is now 1 byte and store old
struct _packed_thingy {
    char x;
    short y;
    int z
} packed_thingy;
#pragma pack(pop) //- restore old packing alignment
sizeof(packed_thingy) should be 7 this is also how you can test your structures simply do a sizeof and see if they have the correct size.

otherwise use the compiler switch /Zp1

That's a tad compiler specific. You might want to inform people what compiler it's specifically for.

os64dev · Post by **os64dev** » Mon Mar 19, 2007 2:58 am

well as Gnippel said he was using msvc i thought i should give the answer for msvc

i use gcc so that would be the attribute packed thingy but indeed i should have mentioned msvc to clarify.