Serious bug in C compiler of DDE 27 related to memset()

After recompiling !KinoAmp with the new compiler, I found out that it could not display its OSD. After some debugging I found the origin of the problem:

here is the C code

player_t* new_player(void)
{
  player_t* player = ka_mem_alloc(sizeof(*player));
  int features;

memset(player, 0, sizeof(*player));
player→play = play_status_stop;
osd_attach(player);
// LDRH/STRH support
if (swix(OSPlatformFeatures, IN|OUT, 0, &features) == NULL)
{
if (features & 0×80)
player→scaleconv.config |= ka_scaleconv_config_ldrh;
else
player→scaleconv.config &= ~ka_scaleconv_config_ldrh;
}
return player;
}

MOV    R12,R13
STMDB  R13!{R4,R11,R12,R14,PC}
SUB    R11,R12,#4
CMP    R13,R10
BLMI   stack related function
SUB    R13,R13,#4
LDR    R0, = &104C4; sizeof(*player)
BL     ka_mem_alloc(sizeof(*player)); on exit R0 is allocated player ptr
LDR    R12,= &104C;  sizeof / 16 
MOR    R4,R0
MOV    R1,#0
MOV    R2,#0
MOV    R3,#0
MOV    R14,#0
memset loop:
STMIA  R0!{R1-R3,R14}
SUBS   R12,R12,#1
BNE    loop
STR    R1,[R0,#0] ; cf sizeof % 16 = 4 bytes

SUB    R0,R0,R12,LSL #4 <= Wrong, R12 is now 0 so on exit R0 != player ptr

MOV    R3,#3

STR    R3,[R0,#2632]

BL     osd_attach(player)

as I indicate in the generated code, after the inlined memset(), the code attempts to use R12 to reset R0 to the start of the structure but R12 does not contain its original value anymore, the loop decreased it to 0.

The C compiler options used are:
-c -depend !Depend -IC: -throwback -fah -apcs 3/32/fpe2/swst/fp/nofpr -memaccess -L22-S22-L41

Ouch! Some testing here suggests that this is a pretty old bug – stretching at least as far back as cc 5.65 from 2006.

Experimenting with the size of player_t suggests that the bug is triggered whenever the structure becomes so large that it can’t use an immediate constant in the “SUB R0,R0,xxx”, at which point it switches to using R12 under the false assumption that it still holds the correct value. So it will probably start happening whenever the size is greater than 4K.

I guess a safe workaround would be to create a function that wraps memset, so that the compiler will never see any direct calls to memset with a constant size parameter.

I’ve added this to the bug tracker, so hopefully it will be fixed by the time the next DDE release rolls round.

Hope the release a quick fix soon, in the mean time its back to the my old compiler on my RiscPC. I don’t want to track down and work around all the potentially broken memset() in my modules and applications, maybe with the exception of KinoAmp for which I add in mind toying with some NEON optimised routines at some point.

Looking back at the generated code, I also hope that the compiler has a way to ensure that the pointer given on entry is word aligned before using such an inlined STM loop.

Looking back at the generated code, I also hope that the compiler has a way to ensure that the pointer given on entry is word aligned before using such an inlined STM loop.

I don’t think that it does. I came unstuck in Fat32fs with an inlined memset (or it may have been memcpy) used on an unaligned pointer. I ended up having to ensure that the pointer was aligned.

I also hope that the compiler can be made to realise that (in the example given above ) that R4 has what it wants.

It looks like the compiler takes into account the expected alignment of the type when deciding if it should inline the memset. memset of a char * will use the library function, as will __packed structures.