Weird Problem

[alsig]

I?ve a very simple system with a very weird problem.

When I run my kernel in VMWare with the ?for?-loop excluded the system stalls. It doesn't reset or triple-faults, it just hangs. It writes "Hello" and hangs.
If I include the "for"-loop it write "Hello", my cpu name and "Done!".
I?ve also included my startup file, lnker script and the implementation of ?get_cpu_id?.

My main:

Code: Select all

int main()
{
  int max_cpuid_call,i;
  unsigned char cpu_sign[13];
 
  init_video();
  printf("Hello\n");
	
  //for(i = 0; i < strlen(cpu_sign); i++)
  //cpu_sign[i]= 0;
	
  max_cpuid_call = get_cpu_id(cpu_sign);
  cpu_sign[13] = 0;
  printf("%s\n",cpu_sign);
  printf("Done!");
  for (;;);
  return 0;
}

My linker script

Code: Select all

OUTPUT_FORMAT("binary")
ENTRY(start)
phys = 0x00100000;
SECTIONS
{
  .text phys : AT(phys) {
    code = .;
    *(.text)
    . = ALIGN(4096);
  }
  .data : AT(phys + (data - code))
  {
    data = .;
    *(.data)
    . = ALIGN(4096);
  }
  .bss : AT(phys + (bss - code))
  {
    bss = .;
    *(COMMON)
	*(.bss)
    . = ALIGN(4096);
	bss_end = .;
  }
  end = .;
}

My startup code:

Code: Select all

    ; Switches gdt's
	lgdt [gdt_ptr]

	mov ax,LINEAR_DATA_SEL
	mov ds,ax
	mov es,ax
	mov ss,ax
	mov fs,ax
	mov gs,ax
	jmp LINEAR_CODE_SEL:sbat
sbat:
	
	EXTERN bss, bss_end
	mov edi,bss
	mov ecx,bss_end
	sub ecx,edi
	xor eax,eax
	rep stosb

	mov esp,_sys_stack

	call _main
    jmp $

My implementation of "get_cpu_id"

Code: Select all

	push ebp
	mov	 ebp, esp
	xor	eax,eax
	cpuid
	push eax
	mov	eax, [ebp + 8]
	mov DWORD [eax],     ebx
	mov DWORD [eax + 4], edx
	mov DWORD [eax + 8], ecx
	pop	eax
	pop	ebp
	ret

[/code]

[dave]

There is nothing wierd about what is happening. You boot code has an endless loop in it ( jmp $ ). When you call you "main" it pushes the return address on the stack so main has somewhere to return too. Which in this case is jmp $. This cause your computer to endlessly loop on this jmp instruction.

dave

[alsig]

That isn't my problem...
My problem is that I have to include the "for"-loop to be able to print the "cpu_sign" variable.
If I exclude the "for"-loop my kernel hangs right after the "Hello"-printf.

Regards
Jens Alsig

[bluecode]

hi,

how did you declare get_cpu_id()? Normally your compiler would just push a pointer on the stack and not the whole structure. If so you might be trashing your stack within your get_cpu_id() function...

have fun

[alsig]

how did you declare get_cpu_id()

I declared my get_cpu_id like this:

Code: Select all

extern unsigned int get_cpu_id(unsigned char *string); 

Regards
Jens Alsig

[deadmutex]

how is get_cpu_id() defined?

Edit:
oops, I didn't see where it was...

[Da_Maestro]

Your problem is that you are trying to determine the length of an uninitialised string.

Instead of:

Code: Select all

 for(i = 0; i < strlen(cpu_sign); i++) 

use:

Code: Select all

 
#define CPU_SIGN_LEN    13
...
unsigned char cpu_sign[CPU_SIGN_LEN];
...
for(i = 0; i < CPU_SIGN_LEN; i++ )

strlen will scan the string until it finds the null terminator character...if there isn't one it will keep going forever. Thusly you must initialise the string at a known length first.

Adam

[alsig]

Hi everybody.

I just read my first post and there were a BIG error....

My problem is that I HAVE to include the "for"-loop to get my kernel to run!

Sorry, for the trouble!

Just to clarify my problem is the following:

If I include the "for"-loop the kernel works (strlen or const value).
If I exclude the "for"-loop the kernel stalls at the function call "get_cpu_id".

I have done a bit of debugging and it seems that I mess up my stack in the "get_cpu_id" function.
If I modify the function so that I only pushes and pops "ebp" the kernel works (No cpu id though)
If I pushes and pops two values (ebp and ex. eax) the kernels stalls!

Regards Jens Alsig

[Da_Maestro]

Let me get this straight....if you include the for loop...the code works..yes?


Then just include the for loop...problem solved :-p

Adam

[alsig]

Let me get this straight....if you include the for loop...the code works..yes?

Jep!

Then just include the for loop...problem solved :-p

Nope That isn't a option! I have to know why

Regards Jens Alsig

[Da_Maestro]

Beats me

Try single stepping your code in Bochs. I know it's painfull to do but it has to be done sometimes.

TIP: Learn how to place breakpoints in the code. Getting nasm to disassemble the kernel image directly will help with this. Good luck!

Adam

[mrd]

ok buddy. strlen(cpu_sign) when cpu_sign is unitialized string = asking for trouble. who knows where you're writing zeroes to in that for loop.
cpu_sign[13] = 0 once again you're writing outside the bounds of the array. you declared it 13 bytes long, and you're referencing the 14th byte.

get_cpu_id() fills the first 12 bytes of the array, and the 13th is left untouched. it's possible whatever random value is in this position is causing printf() to crash..

other than that, i'd ensure your final image is linked properly and all generated addresses are accurate.

[Da_Maestro]

You know that correct programming practices should call for ALL variables to be properly initialised. This is just plain common sense!

Yes strlen is a BIG no-no on an uninitialised string. The code that I suggested with the #define defining the length is good programming practice for 3 reasons:
1. You can now change the length of that string by just changing one number (maintainability).
2. The length is consistent throughout the entire program now
3. The string is properly initialised.

Sorry dude, but it looks like leaving the for loop in there is your best option. Either that, or you can initialise the string statically by declaring it like this:

Code: Select all

unsigned char cpu_sign[13] = {0,0,0,0,0,0,0,0,0,0,0,0,0};

I don't know if that is the correct syntax (if anyone wants to correct me feel free to do so) but you get the idea of what I'm doing I hope.

In future peoples, always initialise every variable in your code. Otherwise you run into all sorts of troubles including:
1. Bugs that are not obvious
2. Security issues
3. Code that is not maintainable

'Nuff said.

Adam

[Da_Maestro]

oh and one other thing....null terminators are needed in C-style strings. Your "get_cpu_id" method forgets to do that

[mrd]

Code: Select all

unsigned char cpu_sign[13] = {0,0,0,0,0,0,0,0,0,0,0,0,0};

you don't need all that..

Code: Select all

unsigned char cpu_sign[13] = {0};

this will initialize the entire array to zero.

In future peoples, always initialise every variable in your code.

initializing every variable will increase the size of your image and your footprint. you can safely utilize uninitialized variables if you are careful in your code. however, it's a good rule for beginners or if you don't want to spend the time to optimize.

oh and one other thing....null terminators are needed in C-style strings. Your "get_cpu_id" method forgets to do that

this is a great point. although in your code you add the null char after the call, you should get in the habit of writing your code to be portable. if your function fills a text buffer, it should either null terminate or return the length of the string.