inline assembly errors

[NunoLava1998]

I am using inline assembly to do lidt (No, i don't know how to combine a .c and a .S and i neither can, anyway). However, i get this jumble of errors that make absolutely no sense (making even less sense when you realize the errors appear twice):


Here is my relevant code (line 20 to end):

Code: Select all

void idtinstall(void){ // 20
	asm("cli"); // 21
	idtp.limit = (sizeof (struct idt_e) * 256) - 1; // 22
	idtp.base = &idt; // 23
	uint8_t i = 0;
	while (i++ != 255) {
		i++;
		__asm__ volatile( "mov 0x00, %0"
		 : "b" (i)
		); // 29
	}; // 30
	__asm__ volatile( "lidt [%0]" // 31
	 : "b" (idtp) // 32
	); // 33
}; // 34, line end, end line.

What is wrong with this code? It is supposed to load the IDT. (Everything here is declared, don't worry).

[iansjack]

Explain your constraint "b" in the inline assembler.

[Schol-R-LEA]

You might want to use this GCC Inline Assembly HOWTO as reference for Ianjack's question.

While you are at it, please explain that loop conditional, and specifically a) what the effect of the while() loop as given is (or, if it is just a delay loop, why it requires any inline assembly at all); b) why you aren't using a for() loop; c) why you are testing for an exact match (!=) on the loop condition rather than a range of less than (<); d) why you increment the same index twice on successive lines (and what the effect of doing it between the test and the body of the loop will be, especially in light of that last question); and e) if you are intending to increment by two (which I doubt is the case), why you couldn't use the sum operator (+=) thusly:

Code: Select all

    // I am assuming you are using GCC with a version of the 
    // C standard later than C99, and thus can use inline  
    // declarations. Now, ask yourself, why would I be 
    // making this point, when the original didn't use 
    // inline declarations at all?

    for (uint8_t i = 0;  i < 255; i += 2) {
        __asm__ volatile( "mov 0x00, %0"  : "b" (i));
    } 

You might want to reconsider the way you are using AT&T syntax on one line and Intel (specifically the NASM dialect, with square brackets for indirection) on another (or, conversely, why you are moving what I can only assume you meant to be a register variable into a constant value). I suggest that you try to relate this to Ianjack's question, too.

Code: Select all

      __asm__ volatile( "mov 0x00, %0"    
       : "b" (i)
      ); // 29
  

if this is Intel syntax, then it should be "mov %0, 0x00", that is to say, destination, source order; conversely, if it is meant to use AT&T syntax (as would usually be the case in GCC), it should be written as "movl 0x00, %0", with the operand size marker to show that it is supposed to be a double-word (32-bit) value (if you mean to use a quad-word - 64-bit - value, then it would be "movq 0x00, %0"). Of course, that's assuming that you have the correct operands in the first place...

Code: Select all

   __asm__ volatile( "lidt [%0]" // 31
    : "b" (idtp) // 32
   ); // 33

Intel syntax uses square brackets for indirection, but AT&T syntax uses parentheses, which would be

Code: Select all

   __asm__ volatile( "lidt (%0)" // 31
    : "b" (idtp) // 32
   ); // 33

For that matter, why can't you link a C program object file with an assembly one (something which is certainly possible, and which you seem to have managed in the past)? Once the source is compiled or assembled, the object format is exactly the same. If you can link two object files from two C source files, or two from assembly source, you certainly can link one from each to each other.

I want you to understand that we aren't teasing you or trolling you by not giving the answers directly. We often use the Socratic approach here (in general, not just with you) because, in many ways, understanding why you got something wrong is more important than fixing it is - if we just answer the question directly, without driving home the reason the previous code was flawed, you are likely to make the same mistake again.

[NunoLava1998]

You might want to use this GCC Inline Assembly HOWTO as reference for Ianjack's question.

While you are at it, please explain that loop conditional, and specifically a) what the effect of the while() loop as given is (or, if it is just a delay loop, why it requires any inline assembly at all); b) why you aren't using a for() loop; c) why you are testing for an exact match (!=) on the loop condition rather than a range of less than (<); d) why you increment the same index twice on successive lines (and what the effect of doing it between the test and the body of the loop will be, especially in light of that last question); and e) if you are intending to increment by two (which I doubt is the case), why you couldn't use the sum operator (+=) thusly:

Code: Select all

    // I am assuming you are using GCC with a version of the 
    // C standard later than C99, and thus can use inline  
    // declarations. Now, ask yourself, why would I be 
    // making this point, when the original didn't use 
    // inline declarations at all?

    for (uint8_t i = 0;  i < 255; i += 2) {
        __asm__ volatile( "mov 0x00, %0"  : "b" (i));
    } 

You might want to reconsider the way you are using AT&T syntax on one line and Intel (specifically the NASM dialect, with square brackets for indirection) on another (or, conversely, why you are moving what I can only assume you meant to be a register variable into a constant value). I suggest that you try to relate this to Ianjack's question, too.

Code: Select all

      __asm__ volatile( "mov 0x00, %0"    
       : "b" (i)
      ); // 29
  

if this is Intel syntax, then it should be "mov %0, 0x00", that is to say, destination, source order; conversely, if it is meant to use AT&T syntax (as would usually be the case in GCC), it should be written as "movl 0x00, %0", with the operand size marker to show that it is supposed to be a double-word (32-bit) value (if you mean to use a quad-word - 64-bit - value, then it would be "movq 0x00, %0"). Of course, that's assuming that you have the correct operands in the first place...

Code: Select all

   __asm__ volatile( "lidt [%0]" // 31
    : "b" (idtp) // 32
   ); // 33

Intel syntax uses square brackets for indirection, but AT&T syntax uses parentheses, which would be

Code: Select all

   __asm__ volatile( "lidt (%0)" // 31
    : "b" (idtp) // 32
   ); // 33

For that matter, why can't you link a C program object file with an assembly one (something which is certainly possible, and which you seem to have managed in the past)? Once the source is compiled or assembled, the object format is exactly the same. If you can link two object files from two C source files, or two from assembly source, you certainly can link one from each to each other.

I want you to understand that we aren't teasing you or trolling you by not giving the answers directly. We often use the Socratic approach here (in general, not just with you) because, in many ways, understanding why you got something wrong is more important than fixing it is - if we just answer the question directly, without driving home the reason the previous code was flawed, you are likely to make the same mistake again.

I haven't put ELF file loading in my OS yet, and i have no idea how the compiler is gonna know "ah here's label in .asm/.c/.cpp/.c++" (hence why i need to use a elf executable).

I am trying in particular to fill 0x000 to 0x255 IN MEMORY with 0x00's to make the IDT have space, and in the second IA load the IDT itself.

[iansjack]

But why have you specified "b" as an output constraint (omitting the necessary "=", as the error messages tell you) when there is no output from the assembler instructions (as the "lval" error tells you)?

[NunoLava1998]

Really, GCC? Here's my entire lidt.c:

Code: Select all

#include "defs.h"
struct idt_e
{
	unsigned short base_lo;
	unsigned short sel;
	unsigned char zero;
	unsigned char flags;
	unsigned short base_hi;
} __attribute__((packed));
struct idt_ptr
{
	unsigned short limit;
	unsigned int base;
} __attribute__((packed));

struct idt_e idt[256];
struct idt_ptr idtp;


void idtinstall(void){
	asm("cli");
	idtp.limit = (sizeof (struct idt_e) * 256) - 1;
	idtp.base = &idt;
    	for (char i = 0;  i < 255; i++) {
       		__asm__ volatile( "movb %0, 0x00"  : "b" (i));
    	}
	__asm__ volatile( "lidt (%0)"
	 : "b" (idtp)
	);
};

(Yes, i know "idtp.limit = (sizeof (struct idt_e) * 256) - 1;" is an error, but i will fix this later.)

[iansjack]

You still haven't explained your use of the constraint.

The suspicion has to be that, yet again, you have copied code with no understanding of what it means. It doesn't help that you copied it wrongly, but at least you have some very clear error messages explaining exactly what is wrong.

[JAAman]

Really, GCC? Here's my entire lidt.c:

yes, really, don't blame your compiler when you did things wrong

include/lidt.c:23:12: error: invalid conversion from 'idt_e(*)[256]' to 'unsigned int' [-fpermissive]
idtp.base = &idt;

this is quite obvious -- the proper way to fix this is to use the proper type in your idtp struct (hint: it should not be an unsigned int... because it is not an unsigned integer)

include/lidt.c:24:28: warning: comparison is always true due to limited range of data type [-wtype-limits]
for (char i = 0; i < 255; i++) {

this is a very important warning, since it is telling you this code will not work the way you think it does -- note the sizes of the various types are implementation defined, as is the signed-ness of the char type, in this case, char is defined as an 8-bit signed value -- meaning it can hold numbers from -128 to 127 and can never reach 255, thus that comparison will always be true (creating an infinite loop) -- correct fix: change the type for your counter



the remainder of the errors all refer to your incorrectly written inline ASM, which, as other people have tried to tell you is written wrong (not something I can help you with)

also: why are you doing that loop in ASM instead of C? write it in C and your code will be cleaner, easier to read, easier to maintain, and some of the errors will go away

[Schol-R-LEA]

@NunoLava1998 First off, you might want to change your .sig again; aside from the fact that you did not actually take a hiatus from the forum, the blasted image is just too big, it is getting in the way of reading things.

Also, when quoting a single part of a large post, you don't need to keep the rest of the post. Editing your quotes is simple politeness.

In any case, you still haven't explained what the loop is supposed to be for (even if it were exiting the loop correctly, the only effect it would have is to burn about 765 cycles, which is probably too short for a wait loop if that's the intention, and in any case the compiler is very likely to optimize it away as it is; if preventing the optimization is the reason it is using inline assembly, well, there are better ways to get that result), nor why you are setting the inline assembly code to use the EBX register for input (if you want to use it for output, you would use "=b", but that still doesn't explain what you are trying to do here). You have not addressed or even acknowledged most of the questions and points the others here are trying to make.

If you aren't going to listen to our advice, why are you asking for it?

Seriously, what are you doing on this forum? You keep asking for help, and we keep trying to help you, but the only part of what we tell you which you seem to listen to is the code examples we post - and you frequently use them in ways that make it clear that you didn't understand them. What are you trying to do?

You keep insisting that you are not trying to make some kind of Frankenstein's Monster OS by sewing together random bits of code, but every time you post here, that's exactly what you seem to be doing. This isn't going to work. If you don't understand what you are doing, you aren't going to succeed.

Even if this isn't the case, you aren't telling us what you are doing. Please, simply acknowledge what we've said, otherwise people will stop trying to help you (many already have, I am sure).