2019-10-11 10:18 AM
We encountered what appears to be a compiler bug as rewriting the code slightly by adding intermediate variables solved the issue.
Relevant code :
#define HT16K33_DISPLAY_NB_COLUMNS (8)
#define HT16K33_DISPLAY_NB_ROWS (16)
#define HT16K33_DISPLAY_NB_COLUMN_BYTES (HT16K33_DISPLAY_NB_ROWS/8)
class HT16K33_SevenSegment {
void writeASCII(const uint8_t ledPos, const char character);
private:
uint8_t screen[HT16K33_DISPLAY_NB_COLUMNS*HT16K33_DISPLAY_NB_COLUMN_BYTES];
}
const char AlphaNum[] = { /* ... list of bytes ... */ };
void
HT16K33_SevenSegment::writeASCII(const uint8_t ledPos, const char character) {
assert(ledPos<3);
assert(character<100);
#if 1
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES] = AlphaNum[character-'0'-7];
#else
{
int charIdx=character-'0'-7;
char charVal=AlphaNum[charIdx];
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES]=charVal;
}
#endif
}
In the above extract, the code in the first leg of the preprocessor "if" does not work - "screen" does not get the correct data value. The evaluation of the right hand expression by the debugger shows "0x39" or 57, but the value that is actually written is 0x40 which is not in the AlphaNum table.
With the code in the "else" part, we do get the correct value.
I think that the cause can be detemined by examining the generated code:
Dysfunctionnal code with assembly:
/**
* Write a 1 ASCII Character from position 0.
*/
void
HT16K33_SevenSegment::writeASCII(const uint8_t ledPos, const char character) {
8003470: b580 push {r7, lr}
8003472: b082 sub sp, #8
8003474: af00 add r7, sp, #0
8003476: 6078 str r0, [r7, #4]
8003478: 0008 movs r0, r1
800347a: 0011 movs r1, r2
800347c: 1cfb adds r3, r7, #3
800347e: 1c02 adds r2, r0, #0
8003480: 701a strb r2, [r3, #0]
8003482: 1cbb adds r3, r7, #2
8003484: 1c0a adds r2, r1, #0
8003486: 701a strb r2, [r3, #0]
assert(ledPos<3);
8003488: 1cfb adds r3, r7, #3
800348a: 781b ldrb r3, [r3, #0]
800348c: 2b02 cmp r3, #2
800348e: d905 bls.n 800349c <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x2c>
8003490: 4b10 ldr r3, [pc, #64] ; (80034d4 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x64>)
8003492: 4a11 ldr r2, [pc, #68] ; (80034d8 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x68>)
8003494: 4811 ldr r0, [pc, #68] ; (80034dc <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x6c>)
8003496: 21e0 movs r1, #224 ; 0xe0
8003498: f001 f9fa bl 8004890 <__assert_func>
assert(character<100);
800349c: 1cbb adds r3, r7, #2
800349e: 781b ldrb r3, [r3, #0]
80034a0: 2b63 cmp r3, #99 ; 0x63
80034a2: d905 bls.n 80034b0 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x40>
80034a4: 4b0e ldr r3, [pc, #56] ; (80034e0 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x70>)
80034a6: 4a0c ldr r2, [pc, #48] ; (80034d8 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x68>)
80034a8: 480c ldr r0, [pc, #48] ; (80034dc <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x6c>)
80034aa: 21e1 movs r1, #225 ; 0xe1
80034ac: f001 f9f0 bl 8004890 <__assert_func>
#if 1
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES] = AlphaNum[character-'0'-7];
80034b0: 1cbb adds r3, r7, #2
80034b2: 781b ldrb r3, [r3, #0]
80034b4: 3b37 subs r3, #55 ; 0x37
80034b6: 001a movs r2, r3
80034b8: 1cfb adds r3, r7, #3
80034ba: 781b ldrb r3, [r3, #0]
80034bc: 005b lsls r3, r3, #1
80034be: 4909 ldr r1, [pc, #36] ; (80034e4 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x74>)
80034c0: 5c89 ldrb r1, [r1, r2]
80034c2: 687a ldr r2, [r7, #4]
80034c4: 18d3 adds r3, r2, r3
80034c6: 1c0a adds r2, r1, #0
80034c8: 72da strb r2, [r3, #11]
int charIdx=character-'0'-7;
char charVal=AlphaNum[charIdx];
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES]=charVal;
}
#endif
}
Working code with assembly:
/**
* Write a 1 ASCII Character from position 0.
*/
void
HT16K33_SevenSegment::writeASCII(const uint8_t ledPos, const char character) {
8003470: b580 push {r7, lr}
8003472: b084 sub sp, #16
8003474: af00 add r7, sp, #0
8003476: 6078 str r0, [r7, #4]
8003478: 0008 movs r0, r1
800347a: 0011 movs r1, r2
800347c: 1cfb adds r3, r7, #3
800347e: 1c02 adds r2, r0, #0
8003480: 701a strb r2, [r3, #0]
8003482: 1cbb adds r3, r7, #2
8003484: 1c0a adds r2, r1, #0
8003486: 701a strb r2, [r3, #0]
assert(ledPos<3);
8003488: 1cfb adds r3, r7, #3
800348a: 781b ldrb r3, [r3, #0]
800348c: 2b02 cmp r3, #2
800348e: d905 bls.n 800349c <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x2c>
8003490: 4b13 ldr r3, [pc, #76] ; (80034e0 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x70>)
8003492: 4a14 ldr r2, [pc, #80] ; (80034e4 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x74>)
8003494: 4814 ldr r0, [pc, #80] ; (80034e8 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x78>)
8003496: 21e0 movs r1, #224 ; 0xe0
8003498: f001 fa00 bl 800489c <__assert_func>
assert(character<100);
800349c: 1cbb adds r3, r7, #2
800349e: 781b ldrb r3, [r3, #0]
80034a0: 2b63 cmp r3, #99 ; 0x63
80034a2: d905 bls.n 80034b0 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x40>
80034a4: 4b11 ldr r3, [pc, #68] ; (80034ec <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x7c>)
80034a6: 4a0f ldr r2, [pc, #60] ; (80034e4 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x74>)
80034a8: 480f ldr r0, [pc, #60] ; (80034e8 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x78>)
80034aa: 21e1 movs r1, #225 ; 0xe1
80034ac: f001 f9f6 bl 800489c <__assert_func>
#if 0
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES] = AlphaNum[character-'0'-7];
#else
{
int charIdx=character-'0'-7;
80034b0: 1cbb adds r3, r7, #2
80034b2: 781b ldrb r3, [r3, #0]
80034b4: 3b37 subs r3, #55 ; 0x37
80034b6: 60fb str r3, [r7, #12]
char charVal=AlphaNum[charIdx];
80034b8: 200b movs r0, #11
80034ba: 183b adds r3, r7, r0
80034bc: 490c ldr r1, [pc, #48] ; (80034f0 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x80>)
80034be: 68fa ldr r2, [r7, #12]
80034c0: 188a adds r2, r1, r2
80034c2: 7812 ldrb r2, [r2, #0]
80034c4: 701a strb r2, [r3, #0]
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES]=charVal;
80034c6: 1cfb adds r3, r7, #3
80034c8: 781b ldrb r3, [r3, #0]
80034ca: 005b lsls r3, r3, #1
80034cc: 687a ldr r2, [r7, #4]
80034ce: 18d3 adds r3, r2, r3
80034d0: 183a adds r2, r7, r0
80034d2: 7812 ldrb r2, [r2, #0]
80034d4: 72da strb r2, [r3, #11]
}
#endif
}
Where should we report this?
We're using STMCubeIDE V1.0.2 and "com.st.stm32cube.ide.mcu.externaltools.gnu-tools-for-stm32.7-2018-q2-update.win32_1.0.0.201904181610" for the compiler which comes with the IDE.
2019-10-11 08:19 PM
It's clearly trying to do something that looks similar in both cases. I'm not going to try to step through it exactly. Not convinced it's a bug, but could be.
You should add assert(character >= '0' + 7) to verify you're in bounds. That could be one explanation.
Bug reports for the compiler probably (?) should go here:
https://bugs.launchpad.net/gcc-arm-embedded
2019-10-12 12:51 AM
Hi
Thank you for your reply.
In my humble experience, it's very likely a bug. I have detected more than 10 confirmed compiler bugs in my carreer ;-).
While debugging this, code execution was deterministic, the debugger showed the expected value for AlphaNum[character-'0'-7], and the index value character-'0'-'7' was 11 for the closely inspected execution, well within the length of the array. This is part of a 3 Digit 7 segment LED driver and all three digits were wrong with the original code (we were trying to show "AbC" and the display showed "rubbish"), but correct with the alternative code (the display was showing "AbC").
I'll probably post the issue on launchpad.
2019-10-12 01:24 AM
I've analysed the code in more detail - the assembly code seems to do the same thing, I'll have to check the byte/halfword/word consistency.
void
HT16K33_SevenSegment::writeASCII(const uint8_t ledPos, const char character) {
8003470: b580 push {r7, lr} // Store current stack pointer
8003472: b082 sub sp, #8 // Reserve stack
8003474: af00 add r7, sp, #0 // Get new stack pointer
8003476: 6078 str r0, [r7, #4] // Store 'this' on stack Location #4
8003478: 0008 movs r0, r1 // r0(ledpos)=r1(ledpos)
800347a: 0011 movs r1, r2 // r1(character)=r0(character)
800347c: 1cfb adds r3, r7, #3 // r3=Address for Location #3
800347e: 1c02 adds r2, r0, #0 // r2(ledpos)=r0(ledpos)
8003480: 701a strb r2, [r3, #0] // Store r2(ledpos) in Location #3
8003482: 1cbb adds r3, r7, #2 // r3=address for Location #2
8003484: 1c0a adds r2, r1, #0 // r2(character)=r1(character)
8003486: 701a strb r2, [r3, #0] // Store r2(character) in Location #2
// At this point:
// r7#2: character
// r7#3: ledpos
// r7#4: this
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES] = AlphaNum[character-'0'-7];
80034b0: 1cbb adds r3, r7, #2 // Address for r7#2 - character.
80034b2: 781b ldrb r3, [r3, #0] // r3=character (* r7#2)
80034b4: 3b37 subs r3, #55 ; 0x37 // r3-= '0'+7;
80034b6: 001a movs r2, r3 // r2=r3=character-'0'+7;
80034b8: 1cfb adds r3, r7, #3 // Address for r7#3 - ledpos
80034ba: 781b ldrb r3, [r3, #0] // r3=ledpos (* r7#2)
80034bc: 005b lsls r3, r3, #1 // r3*=2 (r3=2*ledpos=ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES)
80034be: 4909 ldr r1, [pc, #36] ; (80034e4 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x74>) // r1=Pointer to AlphaNum
80034c0: 5c89 ldrb r1, [r1, r2] // r1=AlphaNum[r2]
80034c2: 687a ldr r2, [r7, #4] // r2=this
80034c4: 18d3 adds r3, r2, r3 // r3=this +2*ledpos
80034c6: 1c0a adds r2, r1, #0 // r2=r1=AlphaNum[character-'0'+7]
80034c8: 72da strb r2, [r3, #11] // store r2 in screen[2*ledpos]
Working code:
/**
* Write a 1 ASCII Character from position 0.
*/
void
HT16K33_SevenSegment::writeASCII(const uint8_t ledPos, const char character) {
8003470: b580 push {r7, lr} // Store current stack pointer
8003472: b084 sub sp, #16 // Reserve stack
8003474: af00 add r7, sp, #0 // Get new stack pointer
8003476: 6078 str r0, [r7, #4] // Store 'this' on stack Location #4
8003478: 0008 movs r0, r1 // r0(ledpos)=r1(ledpos)
800347a: 0011 movs r1, r2 // r1(character)=r0(character)
800347c: 1cfb adds r3, r7, #3 // r3=Address for Location #3
800347e: 1c02 adds r2, r0, #0 // r2(ledpos)=r0(ledpos)
8003480: 701a strb r2, [r3, #0] // Store r2(ledpos) in Location #3
8003482: 1cbb adds r3, r7, #2 // r3=address for Location #2
8003484: 1c0a adds r2, r1, #0 // r2(character)=r1(character)
8003486: 701a strb r2, [r3, #0] // Store r2(character) in Location #2
{
int charIdx=character-'0'-7;
80034b0: 1cbb adds r3, r7, #2 // Address for r7#2 - character.
80034b2: 781b ldrb r3, [r3, #0] // r3=character (* r7#2)
80034b4: 3b37 subs r3, #55 ; 0x37 // r3-= '0'+7;
80034b6: 60fb str r3, [r7, #12] // Store r3 in "charIdx" #12 on stack
char charVal=AlphaNum[charIdx];
80034b8: 200b movs r0, #11 // r0=11
80034ba: 183b adds r3, r7, r0 // r3=r7#11
80034bc: 490c ldr r1, [pc, #48] ; (80034f0 <_ZN20HT16K33_SevenSegment10writeASCIIEhc+0x80>) // r1=ptr to AlphaNum
80034be: 68fa ldr r2, [r7, #12] // r2 = *r7"12 = charIdx
80034c0: 188a adds r2, r1, r2 // r2=ptr to AlphaNum[charIdx]
80034c2: 7812 ldrb r2, [r2, #0] // r2=AlphaNum[charIdx]
80034c4: 701a strb r2, [r3, #0] // *r7#11(charVal)=r2=AlphaNum[charIdx]
screen[ledPos*HT16K33_DISPLAY_NB_COLUMN_BYTES]=charVal;
80034c6: 1cfb adds r3, r7, #3 // r3=ptr to ledpos (r7#3)
80034c8: 781b ldrb r3, [r3, #0] // r3=ledpos (*r7#3)
80034ca: 005b lsls r3, r3, #1 // r3*=2 (2*ledpos)
80034cc: 687a ldr r2, [r7, #4] // r2=ptr to this.
80034ce: 18d3 adds r3, r2, r3 // r2+=2*ledpos
80034d0: 183a adds r2, r7, r0 // r2=r7+r0==r7#11=charVal
80034d2: 7812 ldrb r2, [r2, #0] // r2=charVal
80034d4: 72da strb r2, [r3, #11] // store r2 in screen[2*ledpos]
}
2019-10-12 01:50 AM
Why don't you single-step through the disasm? You'd probably see immediately where the strange results come from.
JW
2019-10-12 03:55 AM
2019-10-12 09:11 AM
We've moved on in the mean time, so still have to see if we can reproduce it next Monday.
Before being able to step throught ha ASM efficiently, the code had to be annotated.
2019-10-12 12:50 PM
Well, I haven't reported any for GCC/G++ yet, and this seems to be a ST specific build.