devkitARM thumb stack exposure bug

[robinwatts]

Hi Gents,

I've been trying to get the NDS version of ScummVM to build using thumb mode, and I've run into a problem that I believe is down to a code generation/optimisation bug with thumb, that results in valid data being exposed below the stack pointer.

First, the smallest cutdown example I have been able to produce. Save the following as test.c:

Code: Select all

extern doStreamReadBlock(int *, char *, int size, int);

char readStream(int *s)
{
    char c = 0;
    doStreamReadBlock(s, &c, 1, *s);
    return c;
}

Then build using:

arm-eabi-gcc -c test.c -o test.o -mcpu=arm9tdmi -mthumb -Os

Then disassemble the resulting object file using:

arm-eabi-objdump -x -d test.o

The important bit of the output is as follows:

Code: Select all

Disassembly of section .text:

00000000 <readStream>:
   0:   b510            push    {r4, lr}
   2:   b082            sub     sp, #8
   4:   466c            mov     r4, sp
   6:   3407            adds    r4, #7
   8:   2300            movs    r3, #0
   a:   7023            strb    r3, [r4, #0]
   c:   6803            ldr     r3, [r0, #0]
   e:   1c21            adds    r1, r4, #0
  10:   2201            movs    r2, #1
  12:   f7ff fffe       bl      0 <doStreamReadBlock>
                        12: R_ARM_THM_CALL      doStreamReadBlock
  16:   b002            add     sp, #8
  18:   7820            ldrb    r0, [r4, #0]
  1a:   bc10            pop     {r4}
  1c:   bc02            pop     {r1}
  1e:   4708            bx      r1

Keen eyed observers with spot the problem at offset 16. r4 points to sp+7 at this point. When we execute add sp,#8 we expose the data that r4 points to. If we get an interrupt at this point, which executes code on the same stack (exactly as the libnds IRQ handler does), this will overwrite the value pointed to by r4.

The next instruction then reads from r4, which may by now be bogus.

I'm using devkitARM release 23b 4.3.0.

I hope I've given enough information here, and that this is the right forum to raise this issue. If I've missed anything, or I should be raising a bug report anywhere else, please tell me.

Thanks,

Robin

[WinterMute]

We should be rolling devkitARM release 24 fairly soon using gcc 4.3.2 which produces this code.

Code: Select all

00000000 <readStream>:
   0:	b590      	push	{r4, r7, lr}
   2:	b085      	sub	sp, #20
   4:	af00      	add	r7, sp, #0
   6:	6078      	str	r0, [r7, #4]
   8:	1c3a      	adds	r2, r7, #0
   a:	320f      	adds	r2, #15
   c:	2300      	movs	r3, #0
   e:	7013      	strb	r3, [r2, #0]
  10:	687b      	ldr	r3, [r7, #4]
  12:	681c      	ldr	r4, [r3, #0]
  14:	687b      	ldr	r3, [r7, #4]
  16:	1c3a      	adds	r2, r7, #0
  18:	320f      	adds	r2, #15
  1a:	1c18      	adds	r0, r3, #0
  1c:	1c11      	adds	r1, r2, #0
  1e:	2201      	movs	r2, #1
  20:	1c23      	adds	r3, r4, #0
  22:	f7ff fffe 	bl	0 <doStreamReadBlock>
  26:	1c3b      	adds	r3, r7, #0
  28:	330f      	adds	r3, #15
  2a:	781b      	ldrb	r3, [r3, #0]
  2c:	1c18      	adds	r0, r3, #0
  2e:	46bd      	mov	sp, r7
  30:	b005      	add	sp, #20
  32:	bc90      	pop	{r4, r7}
  34:	bc02      	pop	{r1}
  36:	4708      	bx	r1

Mad code but appears to do the right thing


Unfortunately using the correct architecture for the DS arm9 and enabling optimisations produces this

arm-eabi-gcc -mthumb -mthumb-interwork -march=armv5te -mtune=arm946e-s -O3 -c test.c

Code: Select all

00000000 <readStream>:
   0:	b510      	push	{r4, lr}
   2:	b082      	sub	sp, #8
   4:	466c      	mov	r4, sp
   6:	3407      	adds	r4, #7
   8:	2300      	movs	r3, #0
   a:	7023      	strb	r3, [r4, #0]
   c:	6803      	ldr	r3, [r0, #0]
   e:	1c21      	adds	r1, r4, #0
  10:	2201      	movs	r2, #1
  12:	f7ff fffe 	bl	0 <doStreamReadBlock>
  16:	b002      	add	sp, #8
  18:	7820      	ldrb	r0, [r4, #0]
  1a:	bd10      	pop	{r4, pc}

which exhibits the same problem

I'll have to report this one upstream, thanks for taking the time to produce a good testcase though.

[robinwatts]

I'll have to report this one upstream, thanks for taking the time to produce a good testcase though.

You're welcome. When you do pass it upstream, I'd be fascinated to see a URL to the bug report you generate (if there is one), cos I'd love to watch the process by which it gets fixed...

I've looked in the GCC bug database, but can't see it there at the moment.

Thanks,

Robin

[WinterMute]

I've been doing a bit of research before I report this to get as much information as possible to help narrow it down.

devkitARM r24 does the right thing with -O1 or adding -fno-schedule-insns2 for -O2 & -O3, that should help in the meantime until we figure it out.

EDIT: bug now reported on gcc bugzilla as http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38644