Recently at my day job we stumbled upon a nasty compiler bug dealing with arrays of structures. On the PIC24 compiler we use, something as simple as this produced problematic code:
typedef struct
{
char name[100];
int value;
} ObjectStruct;
#define NUM_OBJECTS 2
// Globals
ObjectStruct objects[NUM_OBJECTS];
// Prototypes
void ShowObject (ObjectStruct object);
void main()
{
int idx;
for (idx = 0; idx < NUM_OBJECTS; idx++)
{
objects[idx].value = (100 + idx);
}
ShowObject (objects[0]);
int n = 0;
ShowObject (objects[n]);
}
void ShowObject (ObjectStruct object)
{
printf ("object.value = %d\r\n", object.value);
}
The compiler appeared to generate reasonable code when accessing the structure element by constant “[0]”:
……………….. ShowObject (objects[0]);
*
0044C: MOV #800,W0
0044E: MOV #8D2,W1
00450: REPEAT #65
00452: MOV [W0++],[W1++]
00454: CALL 39A
………………..
…but when accessing it by variable “[n]” the code got ludicrous (prepare to scroll):
………………..
……………….. int n = 0;
*
00458: CLR 8CE
……………….. ShowObject (objects[n]);
0045A: MOV 8CE,W4
0045C: MOV #66,W3
0045E: MUL.SS W4,W3,W0
00460: MOV #800,W4
00462: ADD W0,W4,W0
00464: MOV #A,W4
00466: REPEAT #32
00468: MOV [W0++],[W4++]
0046A: MOV W5,8D2
0046C: MOV W6,8D4
0046E: MOV W7,8D6
00470: MOV W8,8D8
00472: MOV W9,8DA
00474: MOV W10,8DC
00476: MOV W11,8DE
00478: MOV W12,8E0
0047A: MOV W13,8E2
0047C: MOV W14,8E4
0047E: MOV W15,8E6
00480: MOV W0,8E8
00482: MOV W1,8EA
00484: MOV W2,8EC
00486: MOV W3,8EE
00488: MOV W4,8F0
0048A: MOV W5,8F2
0048C: MOV W6,8F4
0048E: MOV W7,8F6
00490: MOV W8,8F8
00492: MOV W9,8FA
00494: MOV W10,8FC
00496: MOV W11,8FE
00498: MOV W12,900
0049A: MOV W13,902
0049C: MOV W14,904
0049E: MOV W15,906
004A0: MOV W0,908
004A2: MOV W1,90A
004A4: MOV W2,90C
004A6: MOV W3,90E
004A8: MOV W4,910
004AA: MOV W5,912
004AC: MOV W6,914
004AE: MOV W7,916
004B0: MOV W8,918
004B2: MOV W9,91A
004B4: MOV W10,91C
004B6: MOV W11,91E
004B8: MOV W12,920
004BA: MOV W13,922
004BC: MOV W14,924
004BE: MOV W15,926
004C0: MOV W0,928
004C2: MOV W1,92A
004C4: MOV W2,92C
004C6: MOV W3,92E
004C8: MOV W4,930
004CA: MOV W5,932
004CC: MOV W6,934
004CE: MOV W7,936
004D0: MOV W6,8D4
004D2: MOV W7,8D6
004D4: MOV W8,8D8
004D6: MOV W9,8DA
004D8: MOV W10,8DC
004DA: MOV W11,8DE
004DC: MOV W12,8E0
004DE: MOV W13,8E2
004E0: MOV W14,8E4
004E2: MOV W15,8E6
004E4: MOV W0,8E8
004E6: MOV W1,8EA
004E8: MOV W2,8EC
004EA: MOV W3,8EE
004EC: MOV W4,8F0
004EE: MOV W5,8F2
004F0: MOV W6,8F4
004F2: MOV W7,8F6
004F4: MOV W8,8F8
004F6: MOV W9,8FA
004F8: MOV W10,8FC
004FA: MOV W11,8FE
004FC: MOV W12,900
004FE: MOV W13,902
00500: MOV W14,904
00502: MOV W15,906
00504: MOV W0,908
00506: MOV W1,90A
00508: MOV W2,90C
0050A: MOV W3,90E
0050C: MOV W4,910
0050E: MOV W5,912
00510: MOV W6,914
00512: MOV W7,916
00514: MOV W8,918
00516: MOV W9,91A
00518: MOV W10,91C
0051A: MOV W11,91E
0051C: MOV W12,920
0051E: MOV W13,922
00520: MOV W14,924
00522: MOV W15,926
00524: MOV W0,928
00526: MOV W1,92A
00528: MOV W2,92C
0052A: MOV W3,92E
0052C: MOV W4,930
0052E: MOV W5,932
00530: MOV W6,934
00532: MOV W7,936
00534: MOV W8,938
00536: MOV W7,8D6
00538: MOV W8,8D8
0053A: MOV W9,8DA
0053C: MOV W10,8DC
0053E: MOV W11,8DE
00540: MOV W12,8E0
00542: MOV W13,8E2
00544: MOV W14,8E4
00546: MOV W15,8E6
00548: MOV W0,8E8
0054A: MOV W1,8EA
0054C: MOV W2,8EC
0054E: MOV W3,8EE
00550: MOV W4,8F0
00552: MOV W5,8F2
00554: MOV W6,8F4
00556: MOV W7,8F6
00558: MOV W8,8F8
0055A: MOV W9,8FA
0055C: MOV W10,8FC
0055E: MOV W11,8FE
00560: MOV W12,900
00562: MOV W13,902
00564: MOV W14,904
00566: MOV W15,906
00568: MOV W0,908
0056A: MOV W1,90A
0056C: MOV W2,90C
0056E: MOV W3,90E
00570: MOV W4,910
00572: MOV W5,912
00574: MOV W6,914
00576: MOV W7,916
00578: MOV W8,918
0057A: MOV W9,91A
0057C: MOV W10,91C
0057E: MOV W11,91E
00580: MOV W12,920
00582: MOV W13,922
00584: MOV W14,924
00586: MOV W15,926
00588: MOV W0,928
0058A: MOV W1,92A
0058C: MOV W2,92C
0058E: MOV W3,92E
00590: MOV W4,930
00592: MOV W5,932
00594: MOV W6,934
00596: MOV W7,936
00598: MOV W8,938
0059A: MOV W9,93A
0059C: MOV W8,8D8
0059E: MOV W9,8DA
005A0: MOV W10,8DC
005A2: MOV W11,8DE
005A4: MOV W12,8E0
005A6: MOV W13,8E2
005A8: MOV W14,8E4
005AA: MOV W15,8E6
005AC: MOV W0,8E8
005AE: MOV W1,8EA
005B0: MOV W2,8EC
005B2: MOV W3,8EE
005B4: MOV W4,8F0
005B6: MOV W5,8F2
005B8: MOV W6,8F4
005BA: MOV W7,8F6
005BC: MOV W8,8F8
005BE: MOV W9,8FA
005C0: MOV W10,8FC
005C2: MOV W11,8FE
005C4: MOV W12,900
005C6: MOV W13,902
005C8: MOV W14,904
005CA: MOV W15,906
005CC: MOV W0,908
005CE: MOV W1,90A
005D0: MOV W2,90C
005D2: MOV W3,90E
005D4: MOV W4,910
005D6: MOV W5,912
005D8: MOV W6,914
005DA: MOV W7,916
005DC: MOV W8,918
005DE: MOV W9,91A
005E0: MOV W10,91C
005E2: MOV W11,91E
005E4: MOV W12,920
005E6: MOV W13,922
005E8: MOV W14,924
005EA: MOV W15,926
005EC: MOV W0,928
005EE: MOV W1,92A
005F0: MOV W2,92C
005F2: MOV W3,92E
005F4: MOV W4,930
005F6: MOV W5,932
005F8: MOV W6,934
005FA: MOV W7,936
005FC: MOV W8,938
005FE: MOV W9,93A
00600: MOV W10,93C
00602: MOV W9,8DA
00604: MOV W10,8DC
00606: MOV W11,8DE
00608: MOV W12,8E0
0060A: MOV W13,8E2
0060C: MOV W14,8E4
0060E: MOV W15,8E6
00610: MOV W0,8E8
00612: MOV W1,8EA
00614: MOV W2,8EC
00616: MOV W3,8EE
00618: MOV W4,8F0
0061A: MOV W5,8F2
0061C: MOV W6,8F4
0061E: MOV W7,8F6
00620: MOV W8,8F8
00622: MOV W9,8FA
00624: MOV W10,8FC
00626: MOV W11,8FE
00628: MOV W12,900
0062A: MOV W13,902
0062C: MOV W14,904
0062E: MOV W15,906
00630: MOV W0,908
00632: MOV W1,90A
00634: MOV W2,90C
00636: MOV W3,90E
00638: MOV W4,910
0063A: MOV W5,912
0063C: MOV W6,914
0063E: MOV W7,916
00640: MOV W8,918
00642: MOV W9,91A
00644: MOV W10,91C
00646: MOV W11,91E
00648: MOV W12,920
0064A: MOV W13,922
0064C: MOV W14,924
0064E: MOV W15,926
00650: MOV W0,928
00652: MOV W1,92A
00654: MOV W2,92C
00656: MOV W3,92E
00658: MOV W4,930
0065A: MOV W5,932
0065C: MOV W6,934
0065E: MOV W7,936
00660: MOV W8,938
00662: MOV W9,93A
00664: MOV W10,93C
00666: MOV W11,93E
00668: MOV W10,8DC
0066A: MOV W11,8DE
0066C: MOV W12,8E0
0066E: MOV W13,8E2
00670: MOV W14,8E4
00672: MOV W15,8E6
00674: MOV W0,8E8
00676: MOV W1,8EA
00678: MOV W2,8EC
0067A: MOV W3,8EE
0067C: MOV W4,8F0
0067E: MOV W5,8F2
00680: MOV W6,8F4
00682: MOV W7,8F6
00684: MOV W8,8F8
00686: MOV W9,8FA
00688: MOV W10,8FC
0068A: MOV W11,8FE
0068C: MOV W12,900
0068E: MOV W13,902
00690: MOV W14,904
00692: MOV W15,906
00694: MOV W0,908
00696: MOV W1,90A
00698: MOV W2,90C
0069A: MOV W3,90E
0069C: MOV W4,910
0069E: MOV W5,912
006A0: MOV W6,914
006A2: MOV W7,916
006A4: MOV W8,918
006A6: MOV W9,91A
006A8: MOV W10,91C
006AA: MOV W11,91E
006AC: MOV W12,920
006AE: MOV W13,922
006B0: MOV W14,924
006B2: MOV W15,926
006B4: MOV W0,928
006B6: MOV W1,92A
006B8: MOV W2,92C
006BA: MOV W3,92E
006BC: MOV W4,930
006BE: MOV W5,932
006C0: MOV W6,934
006C2: MOV W7,936
006C4: MOV W8,938
006C6: MOV W9,93A
006C8: MOV W10,93C
006CA: MOV W11,93E
006CC: MOV W12,940
006CE: MOV W11,8DE
006D0: MOV W12,8E0
006D2: MOV W13,8E2
006D4: MOV W14,8E4
006D6: MOV W15,8E6
006D8: MOV W0,8E8
006DA: MOV W1,8EA
006DC: MOV W2,8EC
006DE: MOV W3,8EE
006E0: MOV W4,8F0
006E2: MOV W5,8F2
006E4: MOV W6,8F4
006E6: MOV W7,8F6
006E8: MOV W8,8F8
006EA: MOV W9,8FA
006EC: MOV W10,8FC
006EE: MOV W11,8FE
006F0: MOV W12,900
006F2: MOV W13,902
006F4: MOV W14,904
006F6: MOV W15,906
006F8: MOV W0,908
006FA: MOV W1,90A
006FC: MOV W2,90C
006FE: MOV W3,90E
00700: MOV W4,910
00702: MOV W5,912
00704: MOV W6,914
00706: MOV W7,916
00708: MOV W8,918
0070A: MOV W9,91A
0070C: MOV W10,91C
0070E: MOV W11,91E
00710: MOV W12,920
00712: MOV W13,922
00714: MOV W14,924
00716: MOV W15,926
00718: MOV W0,928
0071A: MOV W1,92A
0071C: MOV W2,92C
0071E: MOV W3,92E
00720: MOV W4,930
00722: MOV W5,932
00724: MOV W6,934
00726: MOV W7,936
00728: MOV W8,938
0072A: MOV W9,93A
0072C: MOV W10,93C
0072E: MOV W11,93E
00730: MOV W12,940
00732: MOV W13,942
00734: MOV W12,8E0
00736: MOV W13,8E2
00738: MOV W14,8E4
0073A: MOV W15,8E6
0073C: MOV W0,8E8
0073E: MOV W1,8EA
00740: MOV W2,8EC
00742: MOV W3,8EE
00744: MOV W4,8F0
00746: MOV W5,8F2
00748: MOV W6,8F4
0074A: MOV W7,8F6
0074C: MOV W8,8F8
0074E: MOV W9,8FA
00750: MOV W10,8FC
00752: MOV W11,8FE
00754: MOV W12,900
00756: MOV W13,902
00758: MOV W14,904
0075A: MOV W15,906
0075C: MOV W0,908
0075E: MOV W1,90A
00760: MOV W2,90C
00762: MOV W3,90E
00764: MOV W4,910
00766: MOV W5,912
00768: MOV W6,914
0076A: MOV W7,916
0076C: MOV W8,918
0076E: MOV W9,91A
00770: MOV W10,91C
00772: MOV W11,91E
00774: MOV W12,920
00776: MOV W13,922
00778: MOV W14,924
0077A: MOV W15,926
0077C: MOV W0,928
0077E: MOV W1,92A
00780: MOV W2,92C
00782: MOV W3,92E
00784: MOV W4,930
00786: MOV W5,932
00788: MOV W6,934
0078A: MOV W7,936
0078C: MOV W8,938
0078E: MOV W9,93A
00790: MOV W10,93C
00792: MOV W11,93E
00794: MOV W12,940
00796: MOV W13,942
00798: MOV W14,944
0079A: MOV W13,8E2
0079C: MOV W14,8E4
0079E: MOV W15,8E6
007A0: MOV W0,8E8
007A2: MOV W1,8EA
007A4: MOV W2,8EC
007A6: MOV W3,8EE
007A8: MOV W4,8F0
007AA: MOV W5,8F2
007AC: MOV W6,8F4
007AE: MOV W7,8F6
007B0: MOV W8,8F8
007B2: MOV W9,8FA
007B4: MOV W10,8FC
007B6: MOV W11,8FE
007B8: MOV W12,900
007BA: MOV W13,902
007BC: MOV W14,904
007BE: MOV W15,906
007C0: MOV W0,908
007C2: MOV W1,90A
007C4: MOV W2,90C
007C6: MOV W3,90E
007C8: MOV W4,910
007CA: MOV W5,912
007CC: MOV W6,914
007CE: MOV W7,916
007D0: MOV W8,918
007D2: MOV W9,91A
007D4: MOV W10,91C
007D6: MOV W11,91E
007D8: MOV W12,920
007DA: MOV W13,922
007DC: MOV W14,924
007DE: MOV W15,926
007E0: MOV W0,928
007E2: MOV W1,92A
007E4: MOV W2,92C
007E6: MOV W3,92E
007E8: MOV W4,930
007EA: MOV W5,932
007EC: MOV W6,934
007EE: MOV W7,936
007F0: MOV W8,938
007F2: MOV W9,93A
007F4: MOV W10,93C
007F6: MOV W11,93E
007F8: MOV W12,940
007FA: MOV W13,942
007FC: MOV W14,944
007FE: MOV W15,946
00800: MOV W14,8E4
00802: MOV W15,8E6
00804: MOV W0,8E8
00806: MOV W1,8EA
00808: MOV W2,8EC
0080A: MOV W3,8EE
0080C: MOV W4,8F0
0080E: MOV W5,8F2
00810: MOV W6,8F4
00812: MOV W7,8F6
00814: MOV W8,8F8
00816: MOV W9,8FA
00818: MOV W10,8FC
0081A: MOV W11,8FE
0081C: MOV W12,900
0081E: MOV W13,902
00820: MOV W14,904
00822: MOV W15,906
00824: MOV W0,908
00826: MOV W1,90A
00828: MOV W2,90C
0082A: MOV W3,90E
0082C: MOV W4,910
0082E: MOV W5,912
00830: MOV W6,914
00832: MOV W7,916
00834: MOV W8,918
00836: MOV W9,91A
00838: MOV W10,91C
0083A: MOV W11,91E
0083C: MOV W12,920
0083E: MOV W13,922
00840: MOV W14,924
00842: MOV W15,926
00844: MOV W0,928
00846: MOV W1,92A
00848: MOV W2,92C
0084A: MOV W3,92E
0084C: MOV W4,930
0084E: MOV W5,932
00850: MOV W6,934
00852: MOV W7,936
00854: MOV W8,938
00856: MOV W9,93A
00858: MOV W10,93C
0085A: MOV W11,93E
0085C: MOV W12,940
0085E: MOV W13,942
00860: MOV W14,944
00862: MOV W15,946
00864: MOV W0,948
00866: MOV W15,8E6
00868: MOV W0,8E8
0086A: MOV W1,8EA
0086C: MOV W2,8EC
0086E: MOV W3,8EE
00870: MOV W4,8F0
00872: MOV W5,8F2
00874: MOV W6,8F4
00876: MOV W7,8F6
00878: MOV W8,8F8
0087A: MOV W9,8FA
0087C: MOV W10,8FC
0087E: MOV W11,8FE
00880: MOV W12,900
00882: MOV W13,902
00884: MOV W14,904
00886: MOV W15,906
00888: MOV W0,908
0088A: MOV W1,90A
0088C: MOV W2,90C
0088E: MOV W3,90E
00890: MOV W4,910
00892: MOV W5,912
00894: MOV W6,914
00896: MOV W7,916
00898: MOV W8,918
0089A: MOV W9,91A
0089C: MOV W10,91C
0089E: MOV W11,91E
008A0: MOV W12,920
008A2: MOV W13,922
008A4: MOV W14,924
008A6: MOV W15,926
008A8: MOV W0,928
008AA: MOV W1,92A
008AC: MOV W2,92C
008AE: MOV W3,92E
008B0: MOV W4,930
008B2: MOV W5,932
008B4: MOV W6,934
008B6: MOV W7,936
008B8: MOV W8,938
008BA: MOV W9,93A
008BC: MOV W10,93C
008BE: MOV W11,93E
008C0: MOV W12,940
008C2: MOV W13,942
008C4: MOV W14,944
008C6: MOV W15,946
008C8: MOV W0,948
008CA: MOV W1,94A
008CC: SUB #66,W0
008CE: MOV #8E8,W4
008D0: REPEAT #32
008D2: MOV [W0++],[W4++]
008D4: MOV #8EA,W4
008D6: REPEAT #32
008D8: MOV [W1++],[W4++]
008DA: MOV #8EC,W4
008DC: REPEAT #32
008DE: MOV [W2++],[W4++]
008E0: MOV #8EE,W4
008E2: REPEAT #32
008E4: MOV [W3++],[W4++]
008E6: MOV #8F0,W0
008E8: REPEAT #32
008EA: MOV [W4++],[W0++]
008EC: MOV #8F2,W0
008EE: REPEAT #32
008F0: MOV [W5++],[W0++]
008F2: MOV #8F4,W0
008F4: REPEAT #32
008F6: MOV [W6++],[W0++]
008F8: MOV #8F6,W0
008FA: REPEAT #32
008FC: MOV [W7++],[W0++]
008FE: MOV #8F8,W0
00900: REPEAT #32
00902: MOV [W8++],[W0++]
00904: MOV #8FA,W0
00906: REPEAT #32
00908: MOV [W9++],[W0++]
0090A: MOV #8FC,W0
0090C: REPEAT #32
0090E: MOV [W10++],[W0++]
00910: MOV #8FE,W0
00912: REPEAT #32
00914: MOV [W11++],[W0++]
00916: MOV #900,W0
00918: REPEAT #32
0091A: MOV [W12++],[W0++]
0091C: MOV #902,W0
0091E: REPEAT #32
00920: MOV [W13++],[W0++]
00922: SUB #CC,W14
00924: MOV #904,W0
00926: REPEAT #32
00928: MOV [W14++],[W0++]
0092A: SUB #CC,W15
0092C: MOV #906,W0
0092E: REPEAT #32
00930: MOV [W15++],[W0++]
00932: MOV #908,W0
00934: REPEAT #32
00936: MOV [W0++],[W0++]
00938: MOV #90A,W0
0093A: REPEAT #32
0093C: MOV [W1++],[W0++]
0093E: MOV #90C,W0
00940: REPEAT #32
00942: MOV [W2++],[W0++]
00944: MOV #90E,W0
00946: REPEAT #32
00948: MOV [W3++],[W0++]
0094A: MOV #910,W0
0094C: REPEAT #32
0094E: MOV [W4++],[W0++]
00950: MOV #912,W0
00952: REPEAT #32
00954: MOV [W5++],[W0++]
00956: MOV #914,W0
00958: REPEAT #32
0095A: MOV [W6++],[W0++]
0095C: MOV #916,W0
0095E: REPEAT #32
00960: MOV [W7++],[W0++]
00962: MOV #918,W0
00964: REPEAT #32
00966: MOV [W8++],[W0++]
00968: MOV #91A,W0
0096A: REPEAT #32
0096C: MOV [W9++],[W0++]
0096E: MOV #91C,W0
00970: REPEAT #32
00972: MOV [W10++],[W0++]
00974: MOV #91E,W0
00976: REPEAT #32
00978: MOV [W11++],[W0++]
0097A: MOV #920,W0
0097C: REPEAT #32
0097E: MOV [W12++],[W0++]
00980: MOV #922,W0
00982: REPEAT #32
00984: MOV [W13++],[W0++]
00986: MOV #924,W0
00988: REPEAT #32
0098A: MOV [W14++],[W0++]
0098C: MOV #926,W0
0098E: REPEAT #32
00990: MOV [W15++],[W0++]
00992: MOV #928,W0
00994: REPEAT #32
00996: MOV [W0++],[W0++]
00998: MOV #92A,W0
0099A: REPEAT #32
0099C: MOV [W1++],[W0++]
0099E: MOV #92C,W0
009A0: REPEAT #32
009A2: MOV [W2++],[W0++]
009A4: MOV #92E,W0
009A6: REPEAT #32
009A8: MOV [W3++],[W0++]
009AA: MOV #930,W0
009AC: REPEAT #32
009AE: MOV [W4++],[W0++]
009B0: MOV #932,W0
009B2: REPEAT #32
009B4: MOV [W5++],[W0++]
009B6: MOV #934,W0
009B8: REPEAT #32
009BA: MOV [W6++],[W0++]
009BC: MOV #936,W0
009BE: REPEAT #32
009C0: MOV [W7++],[W0++]
009C2: CALL 39A
………………..
UPDATE: A week after reporting this issue (and a related one), the tool provider sent us an updated .DLL file that seems to resolve it. Code generation is now much nicer!
Sadly, this isn’t the only such behavior we have found with this compiler in regards to structures.
But why are we trying to pass a structure in to a function? Wouldn’t it be much more efficient to pass it in by reference, as a pointer?
Yes.
Yes it would.
But pointer problems are one of the leading causes of system crashes. Checking that a pointer is not NULL only means it points somewhere, and does not ensure it points to the expected data.
If the code is mission critical, that pointer check needs to do much more. Perhaps have some ID values in the structure that have to be verified, and add a checksum/CRC to the data:
typedef struct
{
uint16_2 id; // will be set to some value like 0x1234
// your data
// more of your data
uint16_t checksum; // will be a checksum of the id and data bytes.
} SafeStruct;
Then, when that structure is initialized/created, the ID would get set, and any time values were updated, the checksum would be updated. Any function that gets passed a pointer to a SafeStruct would validate the id and then checksum before proceeding.
If the system happens to have memory protection, and the pointer is in some off-limits memory, that attempt to access the id could cause a BUS TRAP or memory access exception. So, other checks would be needed to see if the pointer is even in the expected range of application memory.
This can get tricky, but if we all coded like this, we’d have far less crashes. I believe, anyway.
But, C allows you to pass in a variable and a copy gets made for use in the the function. This generates extra code and is slower, but it’s safer since the function only works on a COPY of the data and not a pointer that could be wrong.
MyStruct foo;
function (foo); // function() gets a copy of foo to work with
To modify this and return a value, you could do something like this:
foo = function (foo);
But, that means you are no longer able to use the return value for error checking. To work around that, you might have an error field in the structure that is set before it returns.
Yep, extra steps. Larger code. Slower code. But if you are coding as if lives depend on it working, and don’t need every last clock cycle, it’s a good habit to get into.
Unless you are using this PIC24 compiler, in which case, it causes other problems.
Until next time…