Assembly problem with GCC (from FreeRTOS port

guillep2k wrote on Tuesday, January 24, 2006:

Hi… I am a newbie with ARM assembler, so I am not fully sure about why this happens. In the LPC2000 port, the portSAVE_CONTEXT macro reads like this:

#define portSAVE_CONTEXT()
{
    extern volatile void * volatile pxCurrentTCB;
    extern volatile unsigned portLONG ulCriticalNesting;

/* Push R0 as we are going to use the register. */
    asm volatile ( "STMDB SP!, {R0}" );

/* Set R0 to point to the task stack pointer. */
    asm volatile ( "STMDB SP,{SP}^" );
    asm volatile ( "SUB SP, SP, #4" );
    asm volatile ( "LDMIA SP!,{R0}" );

/* Push the return address onto the stack. */
    asm volatile ( "STMDB R0!, {LR}" );

/* Now we have saved LR we can use it instead of R0. */
    asm volatile ( "MOV LR, R0" );

/* Pop R0 so we can save it onto the system mode stack. */
    asm volatile ( "LDMIA SP!, {R0}" );

/* Push all the system mode registers onto the task stack. */
    asm volatile ( "STMDB LR,{R0-LR}^");
    asm volatile ( "SUB LR, LR, #60" );

/* Push the SPSR onto the task stack. */
    asm volatile ( "MRS R0, SPSR" );
    asm volatile ( "STMDB LR!, {R0}" );

    asm volatile ( "LDR R0, =ulCriticalNesting " );
    asm volatile ( "LDR R0, [R0]" );
    asm volatile ( "STMDB LR!, {R0}" );

/* Store the new top of stack for the task. */
    asm volatile ( "LDR R0, %0" : : "m" (pxCurrentTCB) );
    asm volatile ( "STR LR, [R0]" );
    ( void ) ulCriticalNesting;
}

However, once compiled the code looked like this in one of my ISR functions:

12b8:    e92d0001     stmdb    sp!, {r0}
12bc:    e94d2000     stmdb    sp, {sp}^
12c0:    e24dd004     sub    sp, sp, #4    ; 0x4
12c4:    e8bd0001     ldmia    sp!, {r0}
12c8:    e9204000     stmdb    r0!, {lr}
12cc:    e1a0e000     mov    lr, r0
12d0:    e8bd0001     ldmia    sp!, {r0}
12d4:    e94e7fff     stmdb    lr, {r0, r1, r2, r3, r4, r5, r6, r7, r8, r9, sl, fp, ip, sp, lr}^
12d8:    e24ee03c     sub    lr, lr, #60    ; 0x3c
12dc:    e14f0000     mrs    r0, SPSR
12e0:    e92e0001     stmdb    lr!, {r0}
12e4:    e59f0b18     ldr    r0, [pc, #2840]    ; 1e04 <.text+0x1e04>
12e8:    e5900000     ldr    r0, [r0]
12ec:    e92e0001     stmdb    lr!, {r0}
12f0:    e59fe0d8     ldr    lr, [pc, #216]    ; 13d0 <.text+0x13d0>
12f4:    e59e0000     ldr    r0, [lr]
12f8:    e580e000     str    lr, [r0]
12fc:    e59f60d0     ldr    r6, [pc, #208]    ; 13d4 <.text+0x13d4>
1300:    e5963000     ldr    r3, [r6]
1304:    e24eb004     sub    fp, lr, #4    ; 0x4

13d0:    400009a4     andmi    r0, r0, r4, lsr #19     ; pxCurrentTCB
13d4:    400000e8     andmi    r0, r0, r8, ror #1      ; ulCriticalNesting

1e04:    400000e8     andmi    r0, r0, r8, ror #1      ; ulCriticalNesting

You can see at 12f0 that LR gets scratched!!! So, when 12f8 wants to save LR into [R0], it is saving a different value than expected, which is the position of the top of the task stack. What is actually saved into [R0] (i.e., pxCurrentTCB->pxTopOfStack) is the address of… pxCurrentTCB->pxTopOfStack!!!

I temporarily changed the macro into:

#define portSAVE_CONTEXT()
{
    extern volatile void * volatile pxCurrentTCB;
    extern volatile unsigned portLONG ulCriticalNesting;
/* Push R0 as we are going to use the register. */
    asm volatile ( "STMDB SP!, {R0}" );
/* Set R0 to point to the task stack pointer. */
    asm volatile ( "STMDB SP,{SP}^" );
    asm volatile ( "SUB SP, SP, #4" );
    asm volatile ( "LDMIA SP!,{R0}" );
/* Push the return address onto the stack. */
    asm volatile ( "STMDB R0!, {LR}" );
/* Now we have saved LR we can use it instead of R0. */
    asm volatile ( "MOV LR, R0" );
/* Pop R0 so we can save it onto the system mode stack. */
    asm volatile ( "LDMIA SP!, {R0}" );
/* Push all the system mode registers onto the task stack. */
    asm volatile ( "STMDB LR,{R0-LR}^");
    asm volatile ( "SUB LR, LR, #60" );
/* Push the SPSR onto the task stack. */
    asm volatile ( "MRS R0, SPSR" );
    asm volatile ( "STMDB LR!, {R0}" );
    asm volatile ( "LDR R0, =ulCriticalNesting " );
    asm volatile ( "LDR R0, [R0]" );
    asm volatile ( "STMDB LR!, {R0}" );
    asm volatile ( "MOV R4, LR" ); /*GAP*/
/* Store the new top of stack for the task. */
    asm volatile ( "LDR R0, %0" : : "m" (pxCurrentTCB) );
    asm volatile ( "STR R4, [R0]" ); /*GAP*/
/*  asm volatile ( "STR LR, [R0]" ); GAP*/
    ( void ) ulCriticalNesting;
}

Notice the usage of R4 to save the contents of LR. Having done this, my program started working. However I am a bit worried about this problem… Is it perhaps a bug of GCC assembler ("gcc (GCC) 4.0.1 (WinARM)")? A bad use of the %0/"m" syntax? Do I have the wrong GCC options? Check my options here:

Compiling:
gcc -c  -mcpu=arm7tdmi  -I. -gdwarf-2 -DREENTRANT_SYSCALLS_PROVIDED -DROM_RUN -DGCC_ARM7 -I ./include -O2 -Wall -Wcast-align -Wimplicit -Wwrite-strings -Wpointer-arith -Wswitch -Wreturn-type -Wunused -Wshadow -Wa,-adhlns=lib/µp.lst   -MD -MP -MF .de
p/µp.o.d -Wstrict-prototypes -Wmissing-declarations -Wmissing-prototypes  -std=gnu99 lib/µp.c -o lib/µp.o

Linking:
gcc  -mcpu=arm7tdmi  -I. -gdwarf-2 -DREENTRANT_SYSCALLS_PROVIDED -DROM_RUN -DGCC_ARM7 -I ./include -O2 -Wall -Wcast-align -Wimplicit -Wwrite-strings -Wpointer-arith -Wswitch -Wreturn-type -Wunused -Wshadow -Wa,-adhlns=build/crt0.lst   -MD -MP -MF .d
ep/h02.elf.d build/crt0.o   h02.o lib/error.o lib/heap_3.o lib/list.o lib/mballoc.o lib/port.o lib/portISR.o lib/printf.o lib/queue.o lib/serial.o lib/spi.o lib/tasks.o lib/uart.o lib/util.o lib/µp.o     --output h02.elf -nostartfiles -Wl,-Map=h02.m
ap,–cref  -lc  -lm -lc -lgcc -Tbuild/LPC2138-ROM.ld

Any thoughts will be greately appreciated.

Guille

nobody wrote on Tuesday, January 24, 2006:

The link register is one of the first registers to be saved.  See the comment "Now we have saved LR we can use it instead of R0", and the asm lines 12cc, 12d0.  Once the LR is saved it is used as a scratch register deliberately so all other registers are saved as is.  LR is actually used as a stack pointer to save the context, then the top of the stack is saved as you say.  The code is correct as intended prior to your change.

Which version of GCC are you using (just out of interest)?

What did not work prior to your change?

guillep2k wrote on Tuesday, January 24, 2006:

Perhaps you just browsed through my messages and didn’t quite catch the problem. LR is used as a stack register starting from:

/* Now we have saved LR we can use it instead of R0. */ \
asm volatile ( "MOV LR, R0" ); \

Later, it must be saved at the last instruction, so the task can later be switched back:

asm volatile ( "STR LR, [R0]" );

But what point is saving LR if it was overwritten by the compiler?

GCC version is 4.0.1 (WinARM). What did not work is that the next switch towards that task would give a data abort exception, since its stack is now corrupted.

Guille

rtel wrote on Tuesday, January 24, 2006:

(as has been said above) The link register is saved with its initial value at the start of the routine.  Once it has been saved it can be used at will and is used as a stack pointer.

LR is used to point to the address at which various items are pushed onto the stack.  It is always incremented to point to the top of the stack so at the end of the code LR points to the last items pushed on the stack as part of the task context.  This value (top of context stack) is then saved into the TCB pxTopOfStack member, as you note in your original post.  This is so the scheduler knows how to find the context the next time the task executes.  The second time LR is saved it is therefore saving some very important information into the task control block and not attempting to save the LR context.

This is tried and trusted code so I don’t know why you are having a problem.  When does the problem occur?  On the first execution, after a few minutes, following and ISR, etc???

Regards.

guillep2k wrote on Tuesday, January 24, 2006:

Hi, Richard. The problem is that *sometimes* the compiler corrupts the intended code. If you read carefully and compare de C asm instruccionts against the resulting assembler code you will notice that the LR register is overwriten beyond the control of the portSAVE_CONTEXT macro:

12e4: e59f0b18 ldr r0, [pc, #2840] ; ulCriticalNesting 
12e8: e5900000 ldr r0, [r0]
// THE LR AFTER THIS INSTRUCTION IS DE LR WE WANT TO SAVE
12ec: e92e0001 stmdb lr!, {r0}
// THE FOLLOWING INSTRUCTION SHOULD NOT USE LR!!!!!
12f0: e59fe0d8 ldr lr, [pc, #216] ; pxCurrentTCB
12f4: e59e0000 ldr r0, [lr]
// THE VALUE OF LR AT THIS POINT IS NOT CORRECT
12f8: e580e000 str lr, [r0]

Guille

rtel wrote on Tuesday, January 24, 2006:

asm volatile ( "LDR R0, =ulCriticalNesting " );
>12e4: e59f0b18 ldr r0, [pc, #2840] ;
Correct…

asm volatile ( "LDR R0, [R0]" );
>12e8: e5900000 ldr r0, [r0]
Correct…

asm volatile ( "STMDB LR!, {R0}" );
>12ec: e92e0001 stmdb lr!, {r0}
Correct…

/* Store the new top of stack for the task. */
asm volatile ("LDR R0,%0" : : "m" pxCurrentTCB) );
>12f0: e59fe0d8 ldr lr, [pc, #216] ;
>12f4: e59e0000 ldr r0, [lr]
Huh???!!!!

asm volatile ( "STR LR, [R0]" );
>12f8: e580e000 str lr, [r0]
Correct…

When I look at my build disassembled it uses R3, not LR.  We need to see how GCC allocates the register to use.  I’m sure it should not use LR whatever as this will always mess up your return.

Where in particular does it generate this code?  Is it for each portSAVE_CONTEXT() or in one place in particular.

Are you running from RAM - its a verrrrry long shot, but if so could the code have been corrupted?

I have never seen this before.  I am using the GNUARM version of GCC V4.0.1, but have not given this version much exercise.

Regards.

guillep2k wrote on Tuesday, January 24, 2006:

In my code it sometimes uses R3, sometimes R5, sometimes R7 (works fine, then) and sometimes uses LR (crashes). I don’t know exactly what makes the compiler choose between one register or the other. That might depend on what the ISR code does or where the ISR is positioned in ROM, I don’t know. The point is that I’ve got about 6 or 7 ISRs and some of them scratch the LR register and others scratch the R3 register.

I got the linker configured to output a .lss file from which I am extracting the assembler code listing: I am not dissasembling from the board, so code corruption is a non-issue.

I could send you my project to try to reproduce the problem.

Guille

rtel wrote on Tuesday, January 24, 2006:

I need to check the manual.  I seem to recall that there is a way of telling it which registers to use (or not use).  This may provide a quick solution.

Yes, please send your project in a zip file to the address form the contacts page on the FreeRTOS.org site.  It will be interesting to see if GNUARM does the same (most probably).  I have never seen this before!

Regards.

rtel wrote on Tuesday, January 24, 2006:

To conclude this thread - it does indeed appear that the LR can be used inadvertently though the compilers choice of registers.  The suggested modification of the lines:

asm volatile ( "LDR  R0, %0" : : "m" pxCurrentTCB) );  
asm volatile ( "STR  LR, [R0]" );        \

to

asm volatile ( "LDR  R0, =pxCurrentTCB" );
asm volatile ( "LDR  R0, [R0]" );     
asm volatile ( "STR  LR, [R0]" );      \

is a sound fix. 

A "known issue" will be raised and fixed for the next release.

Thanks.

imajeff wrote on Friday, February 17, 2006:

Rather, to conclude this thread, the solution would help.

The problem is that you are using individual asm() lines for each instruction, when the instructions are actually related. They should be included all together in one as in this pseudo asm:

asm ("
  line 1 \n
  line 2 \n
  line 3 \n
");

I can’t verify your particular problem (I compile for other targets), but it has solved similar problems for me. Also, look in GCC documentation for how you can add C-expression parameters to GCC’s inline asm function.

nobody wrote on Friday, February 17, 2006:

So putting the code in the same asm block will guarantee that the LR register is not used as a temp?

imajeff wrote on Tuesday, February 21, 2006:

Ok, I’m not an expert on this but… as far as GCC wanting to use a certain reg as a temp would depend on the gcc target built. What I’ve noticed with GCC-m68hc12 is that it only generates code immediately before and/or after each asm block. It says also I can say `asm volatile ("")` and ‘volatile’ specifies that it should not try to change any code within.

But prerhaps the most important thing is to find the document section for inline asm, and how to use constraints. I use parameters to tell gcc which registers my asm block clobbers, so it knows on the C level.

In a nutshell, it seems gcc is unique to other C compilers I know of in that it uses restraints and C expression parameters to interface C with inline asm, instead of scanning the asm to try and figure what it’s doing. In my observation, that’s why the inline asm is in quotes instead of brackets as in other compilers.