FAT12 Bootsector Problem [SOLVED] (Off-By-1 Error)

[Firestryke31]

I've been working on a bootsector for FAT12 that loads a simple flat (up to) 64KB real mode binary to 0x1000:0000 and runs it. So far it works great except for one thing: after a few sectors it just stops working properly. It says it loaded all of the data, and doesn't give any errors (it should display a message and busywait if there is an error), but when the program it loads tries to access data after a certain point it just gets whatever was there previously. I've checked the disk image, and everything looks fine (all of the data is there, is sequential in cluster count, and the FAT maps properly). I'm 99.9% sure I'm doing something stupid in the loading code:

Code: Select all

loadLoop:
	;; Save the current cluster
	push ax
	
	;; Find the LBA of the cluster
	dec ax
	dec ax
	mul byte [Boot.SPC]
	
	;; These were calculated earlier
	add ax, [ROOTSTART]
	add ax, [ROOTLENGTH]
	
	;; Load all of the sectors in the cluster
	movzx cx, byte [Boot.SPC]
.clusterLoop:
	push cx
	push ax
	;; Convert the cluster's LBA to CHS
	call getCHS
	;; only want one sector (in case a cluster crosses a track boundary)
	mov al, 1
	;; Get the destination
	mov bx, di
	call readDiskCHS
	;; Automatically advance destination pointer
	add di, [Boot.BPS]
	
	;; get the LBA and move to the next cluster
	pop ax
	inc ax
	
	;; Get the loop counter and loop
	pop cx
	loop .clusterLoop

	
	;; Get the current cluster's number back
	pop ax
	
	;; Save it, we'll need it for even/odd checking
	push ax
	
	;; Get the byte index into the FAT of the cluster (* by 1.5)
	mov bx, ax
	shr bx, 1
	add ax, bx
	
	;; From that we can figure the sector index of the cluster in the FAT
	;; and the byte index into this sector of the cluster
	xor dx, dx
	div ax, word [Boot.BPS]
	inc dx
	mov si, dx
	add ax, [Boot.RSC]
	;; SI = cluster's index, AX = cluster's FAT sector

	;; Save the destination segment so we don't interfere
	;; and use DS for smaller instructions
	push es
	
	;; A.K.A. mov es, ds
	push ds
	pop es
	
	;; Save the sector LBA (we want this one and the next one)
	push ax
	
	;; Load it
	call getCHS
	mov bx, freeSpace
	add si, bx
	mov al, 1
	call readDiskCHS
	
	;; Get the next sector in case the cluster index spans two
	;; of them
	pop ax
	inc ax
	push bx
	call getCHS
	pop bx
	add bx, [Boot.BPS]
	mov al, 1
	call readDiskCHS
	
	;; Get the real destination back now due to stack layout
	pop es
	
	;; Get the last cluster (the one we just loaded) for even/odd checking
	pop bx
	mov ax, [si]
	test bx, 1
	jnz .odd
	;; if even, lop off top 4 bits
	shl ax, 4
.odd:
	;; Shift down to get the actual number
	shr ax, 4
	cmp ax, 0x0FF0
	jb loadLoop

It just falls apart about halfway through. I've already fixed a couple of bugs, but unfortunately none of them have been the cause of this problem. If you need more info (i.e. the hex of the relevant parts of the FAT, the sectors of the data file, etc.) I'd be happy to give it to you. Lastly, if any of the comments are wrong, they probably are. I just now went back and commented it to help me try and figure out what my own code is doing. Remember kids: Don't wait until later; always comment your code!

Thank you for taking the time to fix my stupid mistakes .

[Thor]

If I had to guess, I would say that it calculates the CHS incorrectly if the LBA is a multiple of SectorsPerTrack (An error I had that produced very similar problems to what you described). The standard calculations don't work in this case due to whoever made the CHS addressing scheme being retarded and starting sectors at 1.

Of course, I can't tell unless I see your get CHS code...

[xyjamepa]

Hi,

You could take a look at this, it might be helpful, it helped me alot.

Good luck

[Firestryke31]

@Thor: Here's the LBA to CHS code:

Code: Select all

;; LBA to CHS
getCHS:
	;; Borrowed from http://en.wikipedia.org/wiki/CHS_conversion
	xor dx, dx
	mov bx, [Boot.SPT]
	div ax, bx
	inc dx
	mov cl, dl
	xor dx, dx
	mov bx, [Boot.HPD]
	div ax, bx
	mov ch, al
	mov dh, dl
	sal ah, 6
	or ah, cl
	mov cl, ah
	ret

Yes, that's almost a copy/paste from the Wikipedia article (I made some changes to fit the rest of my coding style) but this code has worked in other projects so I'm not entirely convinced it's the problem. I've looked in the debugger and it seems to be failing after loading the first cluster (cluster 2) which then points to the next cluster sequentially (no fragmentation). I'll take another look at my cluster LBA calculations...

@abuashraf: I'll look into that. Maybe I won't need it though.

[Thor]

Fails after first cluster? Then I would guess it would be your FAT reading code, since the first cluster will be in the root directory, whereas the rest are in the FAT...

I'll take a look

Edit: For starters, I want to see the value of ax in two places:

Code: Select all

   ;; These were calculated earlier
   add ax, [ROOTSTART]
   add ax, [ROOTLENGTH]
   
   ;;RIGHT HERE

   ;; Load all of the sectors in the cluster
   movzx cx, byte [Boot.SPC]

and

Code: Select all

.odd:
   ;; Shift down to get the actual number
   shr ax, 4

   ;;RIGHT HERE

   cmp ax, 0x0FF0
   jb loadLoop

That way we can see where your code is going wrong...

[Firestryke31]

At the first part AX is 0x0021.
At the next part AX is 0x0040 (should be 0x0003).

The stack appears to not be what I thought, or the current cluster value gets modified before it gets saved. The reason it doesn't crash and burn is because it loads cluster 0x40, which points to 0x00, which points to the EOF value 0x0FF0 due to the media descriptor byte.

Yay, I get to play the stack trace game!

Edit: After doing some better debugging, I found I had an off-by-1 error. Here is where it was:

Code: Select all

	;; From that we can figure the sector index of the cluster in the FAT
	;; and the byte index into this sector of the cluster
	xor dx, dx
	div ax, word [Boot.BPS]
;	inc dx		;; D'OH! WTF?! Why on Earth did I put this here?!
	mov si, dx
	add ax, [Boot.RSC]
	;; SI = cluster's index, AX = cluster's FAT sector

I shall now mark this thread as [SOLVED].

[Thor]

Glad to hear you fixed it