Bug awareness - finding the address of arguments is a NoNo

[SANiK]

Err, to save you guys a bunch of debug time, be aware that PSPlink causes random bugs when trying to print a float.
The bug results in PSPlink crashing.
Usbhostfs reports an invalid magic error.

The bug is caused by TyRaNid attempting to find the address of a function argument... although, the PSP uses registers to store/pass arguments, hence the bug.

The bug is in the float printing code of PSPlink, hence avoid printing floats in PSPlink.

And never try to get the address of a function argument!

[Raphael]

Could you be more specific where that code is that tries to find the adress of an argument/print floats, that is faulty?
Can't seem to find that part.

[jbit]

And never try to get the address of a function argument!

Erm, why exactly... GCC will automatically store the register on the stack and give that address if you try to get the address of a function argument. The following output is from ee-gcc, but should be similar on PSP compiler...

Original C code:

Code: Select all

void somefunc(int blah)
{
    someotherfunc(&blah);
}

Unoptimized assembly code (with some annotations ;)

Code: Select all

00000020 <somefunc>&#58;
  20&#58;   addiu   sp,sp,-48
  24&#58;   sd  ra,32&#40;sp&#41;
  28&#58;   sd  s8,16&#40;sp&#41;
  2c&#58;   move    s8,sp
  30&#58;   sw  a0,0&#40;s8&#41;    ; value of "blah" is stored on the stack
  34&#58;   jal 0 <someotherfunc>
  38&#58;   move    a0,s8    ; arg0 for someotherfunc is set to the address of "blah" on the stack
  3c&#58;   move    sp,s8
  40&#58;   ld  ra,32&#40;sp&#41;
  44&#58;   ld  s8,16&#40;sp&#41;
  48&#58;   jr  ra
  4c&#58;   addiu   sp,sp,48

EDIT:I should point out the above ASM is raw MIPS asm, so "move a0,s8" is in the branch delay slot for JAL, therefore it's execution is completed before the branch is completed.

[chp]

Also, a printf(char* buf, ...)-style function is a varargs-function, which will store the value before '...' on the stack, because it will need to get the address to 'buf' before it starts processing the format-string. I assume it was a function similar to this that you were using?

[TyRaNiD]

It does seem to be true that the PSP compiler is broken, it tries to store the value onto the stack but doesn't actually do so until after the code which uses the value. However I fail to see why it would actually die completely, and at any rate I don't fucking care.

[SANiK]

jbit - The PSPcompiler has its issues =/

chp/Raphael - The bug is in the PSPLink code which handles the conversion of floats to string, and not within the function printf() itself

/*Taken from util.c*/

Code: Select all

static int is_inf(float val)
{
        void *p;
        unsigned int conv;
        int sign;
        int exp;
        int mantissa;

BUG: p = (void *) &val;
        conv = *((unsigned int *) p);
        sign = (conv >> 31) & 1;

        exp = (conv >> 23) & 0xff;
        mantissa = conv & 0x7fffff;

        if((exp == 255) && (mantissa == 0))
        {
                if(sign)
                {
                        return -1;
                }
                else
                {
                        return 1;
                }
        }

        return 0;
}

[Raphael]

Code: Select all

static int is_inf(float val)
{
	void *p;
	unsigned int conv;
	int sign;
	int exp;
	int mantissa;

	p = (void *) &val;
	conv = *((unsigned int *) p);
	sign = (conv >> 31) & 1;

	exp = (conv >> 23) & 0xff;
	mantissa = conv & 0x7fffff;

	if((exp == 255) && (mantissa == 0))
	{
		if(sign)
		{
			return -1;
		}
		else
		{
			return 1;
		}
	}

	return 0;
}


int main(int argc, char *argv[])
{
	float x = rand()/32768.f;
	if (is_inf(x))
	{
		printf("IsINF\n");
	}

	sceKernelExitGame();
	return(0);
}

Compiled with same flags as psplink results in the following compiler output:

Code: Select all

$LC1:
	.ascii	"IsINF\012\000"
 #NO_APP
	.section	.rodata.cst4,"aM",@progbits,4
	.align	2
$LC0:
	.word	939524096
	.text
	.align	2
	.globl	main
	.ent	main
main:
	.frame	$sp,16,$31		# vars= 8, regs= 1/0, args= 0, gp= 0
	.mask	0x80000000,-8
	.fmask	0x00000000,0
	.set	noreorder
	.set	nomacro
	
	addiu	$sp,$sp,-16
	sw	$31,8($sp)
	jal	rand
	nop

	mtc1	$2,$f0
	lui	$2,%hi($LC0)
	cvt.s.w	$f1,$f0
	lwc1	$f0,%lo($LC0)($2)
	lw	$3,0($sp)          # !WRONG
	li	$2,8323072			# 0x7f0000
	mul.s	$f1,$f1,$f0
	ori	$2,$2,0xffff
	and	$5,$3,$2
	ext	$3,$3,23,8
	li	$2,255			# 0xff
	bne	$3,$2,$L2
	swc1	$f1,0($sp)          # !WRONG

	lui	$4,%hi($LC1)
	bne	$5,$0,$L2
	addiu	$4,$4,%lo($LC1)

	jal	printf
	nop

$L2:
	jal	sceKernelExitGame
	nop

	lw	$31,8($sp)
	move	$2,$0
	j	$31
	addiu	$sp,$sp,16

As Tyranid said, gcc tries to put the float on stack and pop as integer register to do the conversion. However, it puts it in wrong order, hence this code produces wrong results.
Funnily enough, with -O1 it puts them in correct order, but not with any optimization value.

Correct result is given with this code in any case though:

Code: Select all

static int is_inf(float val)
{
	unsigned int conv;
	int sign;
	int exp;
	int mantissa;

	asm("mfc1 %0, %1\n":"=r"(conv):"f"(val));
	sign = (conv >> 31) & 1;

	exp = (conv >> 23) & 0xff;
	mantissa = conv & 0x7fffff;

	if((exp == 255) && (mantissa == 0))
	{
		if(sign)
		{
			return -1;
		}
		else
		{
			return 1;
		}
	}

	return 0;
}

Here's a patch for PSPLINKUSB (plus a small fix for VFPU disasm with prefixes) - I could probably commit it myself, but I don't feel like I should be messing on Tyranids repository:

Code: Select all

Index: psplink/util.c
===================================================================
--- psplink/util.c	(revision 2280)
+++ psplink/util.c	(working copy)
@@ -860,14 +860,16 @@
 
 static int is_nan(float val)
 {
-	void *p;
+	//void *p;
 	unsigned int conv;
 	int sign;
 	int exp;
 	int mantissa;
 
-	p = (void *) &val;
-	conv = *((unsigned int *) p);
+	/*p = (void *) &val;
+	conv = *((unsigned int *) p);*/
+	// Help stupid GCC compiler
+	asm("mfc1 %0, %1\n":"=r"(conv):"f"(val));
 	sign = (conv >> 31) & 1;
 
 	exp = (conv >> 23) & 0xff;
@@ -883,14 +885,16 @@
 
 static int is_inf(float val)
 {
-	void *p;
+	//void *p;
 	unsigned int conv;
 	int sign;
 	int exp;
 	int mantissa;
 
-	p = (void *) &val;
-	conv = *((unsigned int *) p);
+	/*p = (void *) &val;
+	conv = *((unsigned int *) p);*/
+	// Help stupid GCC compiler
+	asm("mfc1 %0, %1\n":"=r"(conv):"f"(val));
 	sign = (conv >> 31) & 1;
 
 	exp = (conv >> 23) & 0xff;
Index: pspsh/disasm.C
===================================================================
--- pspsh/disasm.C	(revision 2280)
+++ pspsh/disasm.C	(working copy)
@@ -1149,7 +1149,7 @@
 // [hlide] added pfx_sat_names
 static const char * const pfx_sat_names[4] =
 {
-  "",  "[0:1]",  "",  "[-1:1]"
+  "",  "0:1",  "",  "-1:1"
 };
 
 /* VFPU prefix instruction operands.  The *_SH_* values really specify where