Local Variable Dying?

[Mudi]

Any one have any idea why in the function shown below

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]

>>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]

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]

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]

Get rid of the GOTOs and see if it still happens. 3 IF statements does not make code unreadable:

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

 

[slygamer]

The latch_loadheader function is trashing your stack.

 

[luteijn]

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..

    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

    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.

 

[FluffyPanda]

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:

#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.

 

[Mudi]

Dbl post, sorry

 

[Mudi]

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):

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)

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]

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]

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]

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.

 

[slygamer]

Are you under- or over-running an array in any of the latch functions? Using an address of a local variable for anything?

 

[Mudi]

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]

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]

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.