Interrupt directive ignored

[SeaLiteral]

I have been trying to develop a kernel for some time. I started in March this year, but then I took a break, and now I've returned to it. I haven't got very far with it though. I am stuck at interrupts. It seems like I have misunderstood something. I started following the Barebones tutorial, then the meaty skeleton tutorial, though I may have started deviating a bit from it as I tried to write some standard library functions before reading the suggested implementations, and if my versions worked too, I didn't necessarily switch to the versions in the tutorial. Then it took me some time to figure out how to set up the GDT, but I think I managed to get it right. Then I wrote code to make an IDT, and also a simple interrupt handler that would set a global volatile variable so that I would be able to see it had changed. I hadn't set the IDT to actually refer to that handler, but I don't think that matters for what happened next. I tried to compile the code and:

arch/i386/cpu.c:175:52: warning: 'interrupt' attribute directive ignored [-Wattributes]
__attribute__((interrupt)) void testHandler(struct isframe* testFrame){
^~~~~~~
arch/i386/cpu.c: In function 'testHandler':
arch/i386/cpu.c:175:61: warning: unused parameter 'testFrame' [-Wunused-parameter]
__attribute__((interrupt)) void testHandler(struct isframe* testFrame){

Code:

Code: Select all

struct isframe{ //interrupt stack frame
    int errCode;
    int eip;
    int ecs;
    int flags;
};
__attribute__((interrupt)) void testHandler(struct isframe* testFrame){
    willChange=1;
}

I also tried changing the int elements to uint32_t elements, changing struct isframe to struct interrupt_frame and even changing the order of the elements in the struct in case I'd got them backwards. I also tried leaving out the error code part. The warning doesn't say what it's looking for to identify the structure, but I strongly suspect it must be something with that structure, since the only thing I can find in GCC's documentation is "and you must define struct interrupt_frame as described in the processor’s manual", which is why I tried to see if calling the structure interrupt_frame and the error code error_code made any difference. It didn't. But from my understanding of the Intel documentation, my structure looks right, so I thought I would have a look at the source code of some existing kernels. But so far, every other kernel I've looked at just uses assembly so as not to need the interrupt directive. I guess I'm probably just missing something obvious, but after several days stuck at this point, I think I might as well ask if anyone else understands that directive.

[iansjack]

What version of GCC are you using? I believe that attribute was only implemented very recently for x86 processors.

Edit - I think it was introduced with GCC v7.

[SeaLiteral]

What version of GCC are you using? I believe that attribute was only implemented very recently for x86 processors.

It's 6.3.0, so I think that might be the reason. That may also explain why so few operating systems (none I could find) use that feature. I'll see what happens when I update the cross compiler. Thanks.

[SeaLiteral]

Well, I rebootstrapped GCC so now it's 7.2, then I used that to build the 7.2 cross compiler. The cross compiler got compiled, and now I've tried to compile my kernel with it. But then it just gives me a bunch of errors from some assembly file, and it's a different filename each time. Example:

Code: Select all

i686-elf-gcc --sysroot=/home/larsrune/tmas/sysroot -isystem=/usr/include -MD -c stdio/printf.c -o stdio/printf.libk.o -std=gnu11 -O2 -g -ffreestanding -Wall -Wextra   -D__is_libc -Iinclude -D__is_libk 

If I've counted them correctly, it's 37 errors, all of them about invalid instruction suffixes for push and pop. Apparently, GCC generates some temporary assembly files, and the only information I could find about debugging those was how to save them. Should I spend another day compiling some other version (7.1?) of GCC, or could it be something else? I want to believe it's just some stupid mistake in my commands, but I can't see anything wrong with this:

Code: Select all

i686-elf-gcc -v
Using built-in specs.
COLLECT_GCC=i686-elf-gcc
COLLECT_LTO_WRAPPER=/home/larsrune/.local/libexec/gcc/i686-elf/7.2.0/lto-wrapper
Target: i686-elf
Configured with: ../gcc-7.2.0/configure --target=i686-elf --prefix=/home/larsrune/.local --disable-nls --enable-languages=c,c++ --without-headers --with-system-zlib
Thread model: single
gcc version 7.2.0 (GCC)

I'm pretty sure those are the same options I used with 6.3.

[iansjack]

Are you using a custom version of binutils also (you should be)? (Those errors typically occur when trying to assemble 32-bit code with a 64-bit assembler - is your base system 64-bit?) If you are, I'd suggest updating it to the latest version.

The assembler files that you are seeing the errors in are the intermediate files. gcc converts a C source file to assembler (which will have an arbitrary name) and then runs gas to assemble this file (and then ld to link it).

[SeaLiteral]

Are you using a custom version of binutils also (you should be)?

I didn't remove the old i686-elf binutils before updating GCC, and when I check the version of i686-elf-as it's 2.28, which should be compatible with GCC 7.2.

(Those errors typically occur when trying to assemble 32-bit code with a 64-bit assembler - is your base system 64-bit?) If you are, I'd suggest updating it to the latest version.

I'd assume i686-elf-gcc uses i686-elf-as, but I'll try and update the i686-elf binutils anyway. Or is it something else I need to update?

Also, I've checked the output of -save-temps and i686-elf-gcc is generating 32-bit assembly as it should. Is there some way to check which assembler it is using? I know it's possible to change it when compiling GCC, but that doesn't tell me what it's using now.

[SeaLiteral]

Updating binutils to 2.29 did fix the assembly issue.
But now I got this:

Code: Select all

arch/i386/cpu.c:200:22: sorry, unimplemented: 80387 instructions aren't allowed in interrupt service routine

Even if I use a completely empty do-nothing interrupt handler, I get that error. I guess that's what I get for using newly implemented stuff in GCC, that could probably use more testing. And I don't have floating point code in the interrupt handler or any float in the isframe structure for that matter. I guess this is what I get for relying on newly implemented compiler features. I might as well use assembly then.

[onlyonemac]

The recommended way to create interrupt handlers with GCC is by using an inline assembly wrapper. There are a few options listed on the wiki or you can use my version:

Code: Select all

#define create_isr_wrapper(irq)\
uint32_t irq_##irq##_handler()\
{\
	uint32_t	current_offset;\
	asm goto("jmp %l[isr_end]":::"memory":isr_end);\
\
	isr_start:\
	asm volatile("pushal\npushl %1\ncall %0\nadd $4, %%esp\npopal\niret"::"r"((uint32_t) 0x01234567), "r"((uint32_t) irq):"memory");\
	isr_end:\
\
	current_offset = 0;\
	while (*((uint32_t*) (&&isr_start + current_offset)) != 0x01234567)\
	{\
		current_offset++;\
	}\
	*((uint32_t*) (&&isr_start + current_offset)) = irq_handler;\
	return (uint32_t) &&isr_start;\
}

This creates a macro that generates interrupt handlers. Create the handlers for each interrupt by invoking the macro:

Code: Select all

create_isr_wrapper(0x20);
create_isr_wrapper(0x21);
create_isr_wrapper(0x22);
create_isr_wrapper(0x23);
...

Repeat as needed for whichever interrupts you need to handle. Each interrupt handler will call a function

Code: Select all

void irq_handler(uint32_t irq)

where "irq" is the number of the interrupt that occurred (the same number that you pass to the macro), this function must be defined before you use the macro. Each invocation of the macro also defines a function

Code: Select all

irq_0x20_handler()

(where 0x20 is replaced with the number of the interrupt) and when this function is called it returns the address in memory of the interrupt handler for that interrupt, this is the address that you need to put in your IDT.

Personally I found this approach somewhat neater than the ones on the wiki. I can't really remember how the macro works because it's been quite a while since I wrote it, so I can't offer any more details, sorry.

EDIT: You'll also need type "uint32_t" to be defined, if not use "typedef unsigned int uint32_t;" or replace each occurrence of "uint32_t" with "unsigned int".

[iansjack]

If you check the intermediate assembler code you will find (almost certainly) that it is using mix registers, or some other extended features. Use the appropriate -mnoxxx (e.g. -mnommx) switch to avoid this.

[Korona]

GCC 7 should support -mgeneral-regs-only for x86, which saves you from maintaining a -msoft-float -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx list.

[SeaLiteral]

Thanks, everyone. I read this:

If you check the intermediate assembler code you will find (almost certainly) that it is using mix registers, or some other extended features. Use the appropriate -mnoxxx (e.g. -mnommx) switch to avoid this.

and got rid of the warning using the -mno-80387 option. And then I read this:

GCC 7 should support -mgeneral-regs-only for x86, which saves you from maintaining a -msoft-float -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx list.

which also fixed it, so I've switched to that. As for

The recommended way to create interrupt handlers with GCC is by using an inline assembly wrapper.

Now that I fixed the issue with the interrupt directive, I think I'll use the interrupt directive unless of course I keep running into funny issues with it, then I will definitely reconsider assembly wrappers. Also, uint32_t is defined, I'm using that type in the interrupt stack frame structure.

And I removed the error code part of the structure, since it's only exception handlers and not interrupt handlers that take those.

[Octocontrabass]

EDIT: You'll also need type "uint32_t" to be defined, if not use "typedef unsigned int uint32_t;" or replace each occurrence of "uint32_t" with "unsigned int".

If you want uint32_t, you should include stdint.h. Having a different definition of uint32_t than the compiler can break things in unexpected ways.

[SeaLiteral]

EDIT: You'll also need type "uint32_t" to be defined, if not use "typedef unsigned int uint32_t;" or replace each occurrence of "uint32_t" with "unsigned int".

If you want uint32_t, you should include stdint.h. Having a different definition of uint32_t than the compiler can break things in unexpected ways.

When I said uint32_t was defined, I meant I had included stdint.h. I also use uint16_t in the same file. But I guess I should have mentioned the header file was included. I assume that when onlyonemac said "you need type "uint32_t" to be defined" it didn't mean I should define it manually.

[onlyonemac]

EDIT: You'll also need type "uint32_t" to be defined, if not use "typedef unsigned int uint32_t;" or replace each occurrence of "uint32_t" with "unsigned int".

If you want uint32_t, you should include stdint.h. Having a different definition of uint32_t than the compiler can break things in unexpected ways.

I can't include stdint.h because there is no stdint.h. I'm not using the standard library. My understanding of C compilers is that I can define uint32_t however I want, because the compiler itself doesn't know about anything besides the primitive types (char, short, int, long, etc.) and modifiers (i.e. unsigned) and everything else is resolved to those types during compilation.

[Korona]

stdint.h is provided by the compiler, its part of C's freestanding subset.

I don't think that is the only problem of that code. WTF is that doing?

Code: Select all

   current_offset = 0;\
   while (*((uint32_t*) (&&isr_start + current_offset)) != 0x01234567)\
   {\
      current_offset++;\
   }\
   *((uint32_t*) (&&isr_start + current_offset)) = irq_handler;

EDIT: Oh, I see, it's patching the assembly at runtime because you didn't use the correct constraint for it to accept static function pointers (IIRC it's p with a c modifier). Oh god. Not only will this fail if your code segment is write-protected (which it really should be), it is also daily WTF worthy.

Write your stubs in a real assembler file, not in some kind of hacky and unreadable inline assembler. If you really want to write it in inline assembler, use GCC's unit-level assembly extension.