Local Variable Dying?

Mudi

You're pushing your luck little man
Joined
Jan 25, 2006
Messages
815
Website
mudiweb.com
Any one have any idea why in the function shown below

Code:
char latch_loadgraphics(int episode)
{
int retval = 0;
   SpriteTable = NULL;

   if (latch_loadheader(episode)) { retval = 1; goto abort; }
   if (latch_loadlatch(episode)) { retval = 1; goto abort; }
   if (latch_loadsprites(episode)) { retval = 1; goto abort; }

abort:;
   if (SpriteTable) free(SpriteTable);
   if (BitmapTable) free(BitmapTable);
   return retval;
}
The parameter 'episode' suddenly gets changed to 0 between loadlatch() and loadsprites()? I've debugged just by logging the value of episode through each function, and it changes to 0 between those two... not that it should matter, because I'm passing a copy to the function, not a pointer! WTF?

This only happens in the GP2X compiled version, by the way. I compiled the same code in GCC/Cygwin and it worked perfectly.

ALSO, I agree goto is ugly, but this is ported code, and I'm too lazy to get rid of it for now.
 

sand_man

Member
Joined
Nov 4, 2005
Messages
727
>>ALSO, I agree goto is ugly, but this is ported code, and I'm too lazy to get rid of it for now.

lol I was going to say something before I saw that sentence ;).

Anyway, that is weird. Things like this have happened to me before too. I really don't know a solution to your problem.
 

luteijn

Member
Joined
Dec 23, 2005
Messages
253
Age
44
Location
The Hague, NL
Website
luteijn.xs4all.nl
what are those functions doing, maybe doing something lowlevel which is trashing your stack? Maybe something is optimized wrongly?

About the goto's. Why get rid of them? Don't really see the problem with using them this way to jump to the abort code... much more readable than e.g. nested if then else..

P.
 

Squidge

Certified Guru
Joined
Nov 16, 2003
Messages
8,495
Location
UK
Website
Visit site
The only other way besides goto is either nested if/else statements, or go C++ and use exceptions.

As for your weird problem, I'd look at the assembler. Use objdump to get the proper code, and then turn optimization off and try again. Just don't tell objdump to intermix the source code if you've got optimization on - you'll end up with crap :)
 

yaustar

UK GP32 & GP2X Owner
Joined
Oct 18, 2003
Messages
2,714
Location
UK
Website
Visit site
Get rid of the GOTOs and see if it still happens. 3 IF statements does not make code unreadable:

Code:
    if (latch_loadheader(episode)) 
        retval = 1;
    else if (latch_loadlatch(episode)) 
        retval = 1;
    else if (latch_loadsprites(episode)) 
        retval = 1;
 

luteijn

Member
Joined
Dec 23, 2005
Messages
253
Age
44
Location
The Hague, NL
Website
luteijn.xs4all.nl
yaustar posted on Feb 8 2006 at 12:56 PM said:
Get rid of the GOTOs and see if it still happens. 3 IF statements does not make code unreadable:
I doubt the goto's affect anything as they never get called during normal operation..

Code:
    if (latch_loadheader(episode)) 
        retval = 1;
    else if (latch_loadlatch(episode)) 
        retval = 1;
    else if (latch_loadsprites(episode)) 
        retval = 1;
I always put the conditional block in {} as the quoted way is asking for a cock-up when you add a printf, or other small thingy to existing code.
You can also do
Code:
    if (latch_loadheader(episode) || (latch_loadlatch(episode) ||(latch_loadsprites(episode)) {
        retval = 1;
}
Although I'd not use that as it relies on your compiler turning this into something that shortcuts the logical-or, and is really not very readable or safe...


P.
 
Last edited by a moderator:

FluffyPanda

I have 6 little blue things. W00t.
Joined
Nov 25, 2005
Messages
568
Age
40
Location
Italy
Website
www.fluffypanda.com
Squidge posted on Feb 8 2006 at 12:43 PM said:
The only other way besides goto is either nested if/else statements, or go C++ and use exceptions.
I don't see why you couldn't do something like:

Code:
#define ERR_HEADER -1024

if (latch_loadheader(episode)) { return ERR_HEADER; }
and then deal with exceptions further up the call stack. Then again, I suppose freeing memory referenced by local pointers might be a good reason, but the provided source doesn't seem to be using local vars anyway.

edit: to stay on topic... can we see the latch_loadheader function? That seems to be where the problem is. Form aside, there's nothing wrong with the code you've posted so far.
 
Last edited by a moderator:

Mudi

You're pushing your luck little man
Joined
Jan 25, 2006
Messages
815
Website
mudiweb.com
All right, I'm pretty sure that it's a problem at the assembler level. It runs through that code if I specify -O0 in the compiler options without problem. But I hate using -O0, so I don't like that as a final solution.

Here is the -S assembly dump with -O0 for latch_loadgraphics() (works):
Code:
latch_loadgraphics:
	@ args = 0, pretend = 0, frame = 8
	@ frame_needed = 1, uses_anonymous_args = 0
	mov	ip, sp
	stmfd	sp!, {fp, ip, lr, pc}
	sub	fp, ip, #4
	sub	sp, sp, #8
	str	r0, [fp, #-20]
	mov	r3, #0
	str	r3, [fp, #-16]
	ldr	r2, .L181
	mov	r3, #0
	str	r3, [r2, #0]
	ldr	r0, [fp, #-20]
	bl	latch_loadheader
	mov	r3, r0
	cmp	r3, #0
	beq	.L170
	mov	r3, #1
	str	r3, [fp, #-16]
	b	.L172
.L170:
	ldr	r0, [fp, #-20]
	bl	latch_loadsprites
	mov	r3, r0
	cmp	r3, #0
	beq	.L173
	mov	r3, #1
	str	r3, [fp, #-16]
	b	.L172
.L173:
	ldr	r0, [fp, #-20]
	bl	latch_loadlatch
	mov	r3, r0
	cmp	r3, #0
	beq	.L172
	mov	r3, #1
	str	r3, [fp, #-16]
.L172:
	ldr	r3, .L181
	ldr	r3, [r3, #0]
	cmp	r3, #0
	beq	.L176
	ldr	r3, .L181
	ldr	r3, [r3, #0]
	mov	r0, r3
	bl	free
.L176:
	ldr	r3, .L181+4
	ldr	r3, [r3, #0]
	cmp	r3, #0
	beq	.L178
	ldr	r3, .L181+4
	ldr	r3, [r3, #0]
	mov	r0, r3
	bl	free
.L178:
	ldr	r3, [fp, #-16]
	and	r3, r3, #255
	mov	r0, r3
	sub	sp, fp, #12
	ldmfd	sp, {fp, sp, pc}
.L182:
	.align	2
.L181:
	.word	SpriteTable
	.word	BitmapTable
	.size	latch_loadgraphics, .-latch_loadgraphics
	.comm	crashflag,4,4
	.comm	crashflag2,4,4
	.comm	crashflag3,4,4
	.comm	objdefsprites,164,4
	.comm	BitmapData,4,4
	.comm	LatchHeader,60,4
	.comm	BitmapBufferRAMSize,4,4
	.comm	getbit_bytepos,20,4
	.comm	getbit_bitmask,5,1
	.comm	RawData,4,4
And here is the -O1 'minimal optimization' version (doesn't work)
Code:
latch_loadgraphics:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	stmfd	sp!, {r4, lr}
	mov	r4, r0
	mov	r2, #0
	ldr	r3, .L196
	str	r2, [r3, #0]
	bl	latch_loadheader
	cmp	r0, #0
	bne	.L186
	mov	r0, r4
	bl	latch_loadsprites
	cmp	r0, #0
	bne	.L186
	mov	r0, r4
	bl	latch_loadlatch
	cmp	r0, #0
	moveq	r4, r0
	beq	.L190
.L186:
	mov	r4, #1
.L190:
	ldr	r3, .L196
	ldr	r0, [r3, #0]
	cmp	r0, #0
	blne	free
.L191:
	ldr	r3, .L196+4
	ldr	r0, [r3, #0]
	cmp	r0, #0
	blne	free
.L193:
	and	r0, r4, #255
	ldmfd	sp!, {r4, pc}
.L197:
	.align	2
.L196:
	.word	SpriteTable
	.word	BitmapTable
	.size	latch_loadgraphics, .-latch_loadgraphics
	.comm	crashflag,4,4
	.comm	crashflag2,4,4
	.comm	crashflag3,4,4
	.comm	objdefsprites,164,4
	.comm	BitmapData,4,4
	.comm	LatchHeader,60,4
	.comm	BitmapBufferRAMSize,4,4
	.comm	getbit_bytepos,20,4
	.comm	getbit_bitmask,5,1
	.comm	RawData,4,4
I didn't see any problems just by glancing at it, but I'm no expert on assembly... anyone else see any trouble?

Also: I'm quite sure that latch_loadheader has nothing to do with it. I can re-arrange the order of those functions, it always fails on the third one.
 

Mudi

You're pushing your luck little man
Joined
Jan 25, 2006
Messages
815
Website
mudiweb.com
Is there a GCC 3.x toolchain I can try compiling this on? I don't trust the new GCC 4 releases much yet...
 

Squidge

Certified Guru
Joined
Nov 16, 2003
Messages
8,495
Location
UK
Website
Visit site
If the problem only occurs on the third function, then surely it's loadlatch thats doing the funny business, so have you tried just getting that to return and do nothing? Then progressively allow it to run more code until the value gets corrupted? then you know where the problem is, and can probably have a guess at a work around.
 

Mudi

You're pushing your luck little man
Joined
Jan 25, 2006
Messages
815
Website
mudiweb.com
The value gets corrupted in latch_loadgraphics(). Right after running whichever the second function is. It doesn't matter which order I run latch_loadlatch() and latch_loadgraphics(), and I can skip latch_loadheader entirely if I supply it with the correct constants (in which case it still bugs out on one of the two).

Also, there is nothing so low-level in either function that should trash the stack, they just read from a file and put the values in global variables. Unless a global pointer is somehow getting assigned to that argument (and I don't see any way that could happen here), it should not mess with that value at all.
 

Mudi

You're pushing your luck little man
Joined
Jan 25, 2006
Messages
815
Website
mudiweb.com
Nah, if it was an address or array-overrun issue I doubt it would change if I compiled with -O0, and it probably wouldn't work when I compiled it for cygwin. Whatever, I'm running out of patience here, I guess I'll just release a binary compiled with -O0.
 

WinterMute

Still Fresh
Joined
Jul 16, 2004
Messages
41
Age
52
Location
Portsmouth, UK
Website
www.devkitpro.org
The problem is obviously in the latch_loadsprites function, the assembly you've pasted shows that the compiler places the argument to latch_loadgraphics in r4 then loads r0 from r4 before calling each function. Somewhere in loadsprites r4 is being trashed with O1+.

Pasting the assembly for that function might help
 

slygamer

Member
Joined
Sep 19, 2005
Messages
795
Location
Brisbane, Australia
Website
Visit site
Mudi posted on Feb 9 2006 at 11:55 AM said:
Nah, if it was an address or array-overrun issue I doubt it would change if I compiled with -O0, and it probably wouldn't work when I compiled it for cygwin. Whatever, I'm running out of patience here, I guess I'll just release a binary compiled with -O0.
Show us the source for the latch functions and we might be able to pinpoint the problem in them that you aren't seeing.
 
Last edited by a moderator:
Top