We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.
I catch a C51 compiler bug when debugging some functions to calculate time differrence. The main function's prints out shows that function DifferExStamp(T1,T2) give error time differrence, but DifferJDxTime(djT1,djT2) give correct results. When I check into disassembly code in uvision IDE, i can see that the "data overlaying" for function DifferExStamp(...) -> DifferJDxTime(...) is incorrect, leading the argument djT1 (a return value) be destroyed by the function call to calculate djT2. The C51 compiler version is 7.08, code optimization level is 8. The codes are attached below. Is the Compiler wrong, or my wrong ? Please Help me. Thanks. //==========================================================
#include <reg52.h> // special function register 8052 #endif #include <stdio.h> #include <stdlib.h> /* For randomize function */ typedef unsigned char BYTE; typedef unsigned int WORD; typedef unsigned long ULONG; typedef bit BOOL; typedef struct tagExDate { BYTE cCent; BYTE cYear; BYTE cMon; BYTE cDay; } CExDate; typedef struct tagExTime { BYTE cHour; BYTE cMin; BYTE cSec; BYTE cX100ms; } CExTime; typedef struct tagExStamp { CExDate Date; CExTime Time; } CExStamp; typedef struct tagJDxTime { long lDays; ULONG dwX100ms; } CJDxTime; CJDxTime ExStampToJDxTime(CExStamp oExT1); CExStamp JDxTimeToExStamp(CJDxTime oJDxT1); CJDxTime DifferExStamp(CExStamp oExT1, CExStamp oExT2); CJDxTime DifferJDxTime(CJDxTime oJDxT1, CJDxTime oJDxT2); BOOL IsBadExStamp(CExStamp oExT); void ExStampToString(CExStamp oExTime, char *szTime); #define SECONDS_PER_MINUTE (60) #define SECONDS_PER_HOUR (60*60) #define SECONDS_PER_DAY (24*60*60L) #define BAUD9600_TH1 0xFA /*9600 BAUD in 22.1184MHz.T1@Mode2,bit SMOD=0.*/ #define BAUD_TH1 BAUD9600_TH1 void main(void) { CExStamp xdata n_oExT1, xdata n_oExT2, xdata n_oNewExT; CJDxTime xdata n_oJDxT1, xdata n_oJDxT2, xdata n_oNewJDxT, xdata n_oJDxDletaT; BYTE xdata n_szBuf[32]; char cResult; TMOD = (TMOD & 0x0F) | 0x20; TH1=BAUD_TH1; /*Setup initial baudrate.*/ TL1=BAUD_TH1; SCON = 0x52; /*(0x52) initialize UART to mode 1 */ TR1 = 1; // Open T1 as baudrate generator printf("Please Input 1 Date (as 1902-03-22):"); scanf("%2bd%2bd-%2bd-%2bd",&n_oExT1.Date.cCent,&n_oExT1.Date.cYear, &n_oExT1.Date.cMon,&n_oExT1.Date.cDay); printf("\nPlease Input 1 Time (as 20:02:23.4):"); scanf("%2bd:%2bd:%2bd.%1bd",&n_oExT1.Time.cHour,&n_oExT1.Time.cMin,&n_oExT1.Time.cSec,&n_oExT1.Time.cX100ms); if (IsBadExStamp(n_oExT1)) { printf("\nYou Input a BAD DateTime !\n"); return; } else { ExStampToString(n_oExT1,n_szBuf); printf("\nYou Input a GOOD DateTime:%s,",n_szBuf); n_oJDxT1 = ExStampToJDxTime(n_oExT1); printf("\nThe DaysTime is:%ld-%lu. ",n_oJDxT1.lDays,n_oJDxT1.dwX100ms); } printf("Please Input 2 Date (as 1902-03-22):"); scanf("%2bd%2bd-%2bd-%2bd",&n_oExT2.Date.cCent,&n_oExT2.Date.cYear, &n_oExT2.Date.cMon,&n_oExT2.Date.cDay); printf("\nPlease Input 2 Time (as 20:02:23.5):"); scanf("%2bd:%2bd:%2bd.%1bd",&n_oExT2.Time.cHour,&n_oExT2.Time.cMin,&n_oExT2.Time.cSec,&n_oExT2.Time.cX100ms); if (IsBadExStamp(n_oExT2)) { printf("\nYou Input a BAD DateTime !\n"); return; } else { ExStampToString(n_oExT2,n_szBuf); printf("\nYou Input a GOOD DateTime:%s, ",n_szBuf); n_oJDxT2 = ExStampToJDxTime(n_oExT2); printf("\nThe DaysTime is:%ld-%lu. ",n_oJDxT2.lDays,n_oJDxT2.dwX100ms); } //====return error "difference time" !!!============= n_oJDxDletaT = DifferExStamp(n_oExT1, n_oExT2); printf("\nThe Difference DateTime (T1-T2) is:%ld-%lu.\n",n_oJDxDletaT.lDays,n_oJDxDletaT.dwX100ms); // //====return correct "difference time" !!!============= n_oJDxDletaT = DifferJDxTime(n_oJDxT1, n_oJDxT2); printf("The Difference DaysTime (T1-T2) is:%ld-%lu.\n",n_oJDxDletaT.lDays,n_oJDxDletaT.dwX100ms); } CJDxTime ExStampToJDxTime(CExStamp oExT1) { CJDxTime n_oJDxTime; oExT1.Date.cMon = (oExT1.Date.cMon + 9)%12; #define wYear *((WORD*)&(n_oJDxTime.dwX100ms)) wYear = (WORD)oExT1.Date.cCent*100 + oExT1.Date.cYear; if (oExT1.Date.cMon/10) { wYear -= 1; } n_oJDxTime.lDays = (ULONG)wYear*365L + wYear/4 - wYear/100 + wYear/400 + (oExT1.Date.cMon*306 + 5)/10 + (oExT1.Date.cDay - 1); #undef wYear n_oJDxTime.dwX100ms = (ULONG)oExT1.Time.cHour*((ULONG)SECONDS_PER_HOUR*10) +(WORD)oExT1.Time.cMin*((WORD)SECONDS_PER_MINUTE*10) +(WORD)oExT1.Time.cSec*10 +oExT1.Time.cX100ms; return n_oJDxTime; } CJDxTime DifferExStamp(CExStamp oExT1, CExStamp oExT2) { //===Data overlay improperly !!! return DifferJDxTime((ExStampToJDxTime(oExT1)),(ExStampToJDxTime(oExT2))); } CJDxTime DifferJDxTime(CJDxTime oJDxT1, CJDxTime oJDxT2) { if (oJDxT1.dwX100ms < oJDxT2.dwX100ms) { oJDxT1.lDays --; oJDxT1.dwX100ms += (ULONG)SECONDS_PER_DAY*10L; } oJDxT1.dwX100ms -= oJDxT2.dwX100ms ; oJDxT1.lDays -= oJDxT2.lDays ; return oJDxT1; } BOOL IsBadExStamp(CExStamp oExT) { if ((oExT.Time.cSec > 59) || (oExT.Time.cMin > 59) || (oExT.Time.cHour > 23) || (oExT.Date.cCent < 19) || (oExT.Date.cCent > 99) || (oExT.Date.cYear > 99) || (oExT.Date.cMon == 0) || (oExT.Date.cMon > 12) || (oExT.Date.cDay == 0) || (oExT.Date.cDay > 31)) return 1; return 0; } void ExStampToString(CExStamp oExTime, char *szTime) { sprintf(szTime,"%02bd%02bd-%02bd-%02bd %02bd:%02bd:%02bd.%1bd",oExTime.Date.cCent,oExTime.Date.cYear, oExTime.Date.cMon,oExTime.Date.cDay,oExTime.Time.cHour,oExTime.Time.cMin,oExTime.Time.cSec,oExTime.Time.cX100ms); }
You don't show compiled code (i.e. *.lst file fragments) to back your claim, which makes this a bit harder than it strictly has to be. But anyway: my prime suspect would be those pointer casting hacks in ExStampToJDxTime(). They're not just needlessly ugly, they may also have managed to confuse the call graph analysis done by Keil. I suggest you get rid of them and try again.
"But anyway: my prime suspect would be those pointer casting hacks in ExStampToJDxTime()." Besides, if I recall correctly, casting like that on an lvalue is a no-no.
The cast itself is legal, though it can be a bit risky on processors that are picky about their integer alignment. Perhaps more of a problem is that the cast turns a pointer into a signed 32-bit integer into a pointer to an unsigned 16-bit one. Since Keil stores multi-byte integers MSB first, this code will be working on the high half of the long, which probably isn't what is intended. I'm also dubious about the passing of entire structures as parameters and as return values. But that's more of an efficiency problem. The Hungarian notation makes my head hurt, so I can only stare at the code for a limited amount of time.
Hi, Broeker,Henry and Davis, Thank you. By taking your suggestion, i rewote the function DifferExStamp()and have worked round the problem. My opinion is that the problem is more likely a 'compiler bug'. When i traced into disassembly code of DifferExStamp(), i found that the first parameter of DifferJDxTime(), which is the return value of the first ExStampToJDxTime() call, was overwritten by the second call to ExStampToJDxTime(). As for the casting and structur parameter passing, I consider it be legal. No warning, no error is reported on compiling output. The *.lst file is too long to post. The posted source code is complete and can be built. Thanks for your further inspection. Best regards.
I had said "...if I recall correctly, casting like that on an lvalue is a no-no". Then, Drew and Tony respectively replied "The cast itself is legal..." and "As for the casting and structur parameter passing, I consider it be legal". Yes, it is legal. Traveling in my way-back time machine, I now realize that what I had not recalled that introduced the no-no, was a more complex attempt at a pointer arithmetic operation on a casted pointer subexpression in part of a larger dereferencing expression appearing on the lefthand side of the assignment operator. So nevermind; my previous comment does not apply here for this simple cast.
The *.lst file is too long to post. I didn't ask for all of it --- just the relevant fragment, i.e. the implementation of function DifferExStamps(), should do. Actually, I did try to build your code, by the version of the compiler I have here (C51 7.06) actually refused to compile it, giving me an error C215: invalid cast. I had to expand DifferExStamp() longhand (local variables for all intermediates and the return value), and then it seemed to work reasonably.
Hi, Broeker. Thanks. On debugging, i met the problem in the implementation of function DifferExStamp(). The assembly code lines of function DifferDJxTime() seem good. The following are the relevant *.lst fragments. Best regards.
155 CJDxTime DifferExStamp(CExStamp oExT1, CExStamp oExT2) 156 { 157 1 //===Data overlay improperly !!! 158 1 return DifferJDxTime((ExStampToJDxTime(oExT1)),(ExStampToJDxTime(oExT2))); 159 1 } 160 161 CJDxTime DifferJDxTime(CJDxTime oJDxT1, CJDxTime oJDxT2) 162 { 163 1 if (oJDxT1.dwX100ms < oJDxT2.dwX100ms) 164 1 { 165 2 oJDxT1.lDays --; 166 2 oJDxT1.dwX100ms += (ULONG)SECONDS_PER_DAY*10L; 167 2 } 168 1 169 1 oJDxT1.dwX100ms -= oJDxT2.dwX100ms ; 170 1 oJDxT1.lDays -= oJDxT2.lDays ; 171 1 172 1 return oJDxT1; 173 1 } 174 ; FUNCTION DifferExStamp (BEGIN) ; SOURCE LINE # 155 ; SOURCE LINE # 156 ; SOURCE LINE # 158 0000 7800 R MOV R0,#LOW ?ExStampToJDxTime?BYTE 0002 7C00 R MOV R4,#HIGH ?ExStampToJDxTime?BYTE 0004 7D00 MOV R5,#00H 0006 7B00 MOV R3,#00H 0008 7A00 R MOV R2,#HIGH oExT1 000A 7900 R MOV R1,#LOW oExT1 000C 120000 R LCALL L?0020 000F 7800 R MOV R0,#LOW ?DifferJDxTime?BYTE 0011 7C00 R MOV R4,#HIGH ?DifferJDxTime?BYTE 0013 7D00 MOV R5,#00H 0015 7E00 MOV R6,#00H 0017 7F08 MOV R7,#08H 0019 120000 E LCALL ?C?COPY 001C 7800 R MOV R0,#LOW ?ExStampToJDxTime?BYTE 001E 7C00 R MOV R4,#HIGH ?ExStampToJDxTime?BYTE 0020 7D00 MOV R5,#00H 0022 7B00 MOV R3,#00H 0024 7A00 R MOV R2,#HIGH oExT2 0026 7900 R MOV R1,#LOW oExT2 0028 120000 R LCALL L?0020 002B 7800 R MOV R0,#LOW ?DifferJDxTime?BYTE+08H 002D 7C00 R MOV R4,#HIGH ?DifferJDxTime?BYTE+08H 002F 7D00 MOV R5,#00H 0031 120000 R LCALL L?0023 ; SOURCE LINE # 159 0034 ?C0008: 0034 22 RET ; FUNCTION DifferExStamp (END) ; FUNCTION L?0023 (BEGIN) 0000 7E00 MOV R6,#00H 0002 7F08 MOV R7,#08H 0004 120000 E LCALL ?C?COPY ; FUNCTION DifferJDxTime (BEGIN) ; SOURCE LINE # 161 ; SOURCE LINE # 162 ; SOURCE LINE # 163 0007 AF00 R MOV R7,oJDxT2+07H 0009 AE00 R MOV R6,oJDxT2+06H 000B AD00 R MOV R5,oJDxT2+05H 000D AC00 R MOV R4,oJDxT2+04H 000F AB00 R MOV R3,oJDxT1+07H 0011 AA00 R MOV R2,oJDxT1+06H 0013 A900 R MOV R1,oJDxT1+05H 0015 A800 R MOV R0,oJDxT1+04H 0017 C3 CLR C 0018 120000 E LCALL ?C?ULCMP 001B 502E JNC ?C0009 ; SOURCE LINE # 164 ; SOURCE LINE # 165 001D 74FF MOV A,#0FFH 001F 2500 R ADD A,oJDxT1+03H 0021 F500 R MOV oJDxT1+03H,A 0023 E500 R MOV A,oJDxT1+02H 0025 34FF ADDC A,#0FFH 0027 F500 R MOV oJDxT1+02H,A 0029 E500 R MOV A,oJDxT1+01H 002B 34FF ADDC A,#0FFH 002D F500 R MOV oJDxT1+01H,A 002F E500 R MOV A,oJDxT1 0031 34FF ADDC A,#0FFH 0033 F500 R MOV oJDxT1,A ; SOURCE LINE # 166 0035 E4 CLR A 0036 2500 R ADD A,oJDxT1+07H 0038 F500 R MOV oJDxT1+07H,A 003A E500 R MOV A,oJDxT1+06H 003C 342F ADDC A,#02FH 003E F500 R MOV oJDxT1+06H,A 0040 E500 R MOV A,oJDxT1+05H 0042 340D ADDC A,#0DH 0044 F500 R MOV oJDxT1+05H,A 0046 E4 CLR A 0047 3500 R ADDC A,oJDxT1+04H 0049 F500 R MOV oJDxT1+04H,A ; SOURCE LINE # 167 004B ?C0009: ; SOURCE LINE # 169 004B C3 CLR C 004C E500 R MOV A,oJDxT1+07H 004E 9500 R SUBB A,oJDxT2+07H 0050 F500 R MOV oJDxT1+07H,A 0052 E500 R MOV A,oJDxT1+06H 0054 9500 R SUBB A,oJDxT2+06H 0056 F500 R MOV oJDxT1+06H,A 0058 E500 R MOV A,oJDxT1+05H 005A 9500 R SUBB A,oJDxT2+05H 005C F500 R MOV oJDxT1+05H,A 005E E500 R MOV A,oJDxT1+04H 0060 9500 R SUBB A,oJDxT2+04H 0062 F500 R MOV oJDxT1+04H,A ; SOURCE LINE # 170 0064 C3 CLR C 0065 E500 R MOV A,oJDxT1+03H 0067 9500 R SUBB A,oJDxT2+03H 0069 F500 R MOV oJDxT1+03H,A 006B E500 R MOV A,oJDxT1+02H 006D 9500 R SUBB A,oJDxT2+02H 006F F500 R MOV oJDxT1+02H,A 0071 E500 R MOV A,oJDxT1+01H 0073 9500 R SUBB A,oJDxT2+01H 0075 F500 R MOV oJDxT1+01H,A 0077 E500 R MOV A,oJDxT1 0079 9500 R SUBB A,oJDxT2 007B F500 R MOV oJDxT1,A ; SOURCE LINE # 172 007D 7B00 MOV R3,#00H 007F 7A00 R MOV R2,#HIGH oJDxT1 0081 7900 R MOV R1,#LOW oJDxT1 ; SOURCE LINE # 173 0083 ?C0010: 0083 22 RET ; FUNCTION DifferJDxTime (END)
Sorry, Broeker. In last message, I missed a fragment. Here it is.
0008 L?0020: 0008 7E00 MOV R6,#00H 000A 7F08 MOV R7,#08H 000C 120000 E LCALL ?C?COPY ; FUNCTION ExStampToJDxTime (BEGIN) ; SOURCE LINE # 134 ; SOURCE LINE # 135 ; SOURCE LINE # 138 000F E500 R MOV A,oExT1+02H 0011 2409 ADD A,#09H 0013 75F00C MOV B,#0CH 0016 84 DIV AB 0017 85F000 R MOV oExT1+02H,B ; SOURCE LINE # 141 001A AF00 R MOV R7,oExT1 001C 7E00 MOV R6,#00H 001E 7C00 MOV R4,#00H 0020 7D64 MOV R5,#064H 0022 120000 E LCALL ?C?IMUL 0025 EF MOV A,R7 0026 2500 R ADD A,oExT1+01H 0028 F500 R MOV n_oJDxTime+05H,A 002A EC MOV A,R4 002B 3E ADDC A,R6 002C F500 R MOV n_oJDxTime+04H,A ...... ; SOURCE LINE # 152 017E 7B00 MOV R3,#00H 0180 7A00 R MOV R2,#HIGH n_oJDxTime 0182 7900 R MOV R1,#LOW n_oJDxTime ; SOURCE LINE # 153 0184 ?C0007: 0184 22 RET ; FUNCTION ExStampToJDxTime (END)
The actual problem is in the linker, then, and it's for real. I managed to reproduce it now. The parameters: * C51 7.09 (--> everything current) * optimization >=2 (data overlaying) triggers it. According to the map file, the data sections for ExStamptoJDxTime() and DifferJDxTime() are indeed overlayed, so the objects
?DifferJDxTime?BYTE ?ExStampToJDxTime?BYTE
From your explanation, I'm quite clear on the problem now. Thank you very much.