OSDev.org

Posted: **Sat Dec 10, 2005 12:00 am**

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]

Posted: **Sat Dec 10, 2005 12:00 am**

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

Posted: **Sat Dec 10, 2005 12:00 am**

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

Posted: **Sat Dec 10, 2005 12:00 am**

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

Posted: **Sun Dec 11, 2005 12:00 am**

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

Posted: **Sun Dec 11, 2005 12:00 am**

how is get_cpu_id() defined?

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

Posted: **Mon Dec 12, 2005 12:00 am**

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

Posted: **Mon Dec 12, 2005 12:00 am**

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

Posted: **Mon Dec 12, 2005 12:00 am**

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

Posted: **Tue Dec 13, 2005 12:00 am**

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

Posted: **Tue Dec 13, 2005 12:00 am**

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

Posted: **Wed Dec 14, 2005 12:00 am**

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.

Posted: **Wed Dec 14, 2005 12:00 am**

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

Posted: **Wed Dec 14, 2005 12:00 am**

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

Posted: **Fri Dec 16, 2005 12:00 am**

Da_Maestro wrote:
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.

Da_Maestro wrote: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.

Da_Maestro wrote: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.

OSDev.org

Weird Problem

Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem

Re: Weird Problem