 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
| Compiler bug or me ? |
 Posted: Tue Feb 24, 2009 11:04 am |
 |
|
Hi all,
Can somebody check this out for me.
PCW 4.086, PIC 18F6722
It's a proof of bug (i think) hacked down code so it looks a bit weird but it runs.
In the full program there is a const stucture containing amongst other things the addresses of lots of variables, most in another stucture
But i was getting corruption of the variables if they were refered to by the stored addresses.
It seems the compiler isn't correctly storing addresses as constants.
Jim1 should be the same as jim2.
Is it me or a bug ?
I am looking at the results in the debugger so no display output.
Thanks,
Jim
| Code: | #include <18f6722.h>
#device adc=10 *=16
#device ICD=true
#use delay(clock=16000000)
#fuses WDT, WDT512, INTRC_IO, PROTECT, BROWNOUT, PUT, STVREN, NODEBUG, NOMCLR, NOIESO ,NOFCMEN
#fuses NOLVP, NOWRT, NOWRTD, NOWRTB, WRTC, NOCPD, NOCPB, NOEBTR, NOEBTRB, BORV27, NOXINST, NOLPT1OSC
struct
{
int8 other_data[100];
int8 more_data[150];
int8 bar;
}foo;
const int16* address=&foo.bar; // store address of foo so it can be changed later.
int16 jim1=0;
int16 jim2=0;
void main()
{
setup_oscillator(OSC_16MHZ | OSC_NORMAL);
setup_adc_ports(NO_ANALOGS);
setup_adc(ADC_OFF);
while(1)
{
jim2=&foo.bar; // this works, gets address of foo.bar
jim1=address; // this doesn't
restart_wdt();
}
} |
|
|
 |
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Tue Feb 24, 2009 12:29 pm |
|
|
| const int16* address is inferring a flash constant pointer, that can't be changed later and possibly don't work in regular assignments to a ram pointer. Omitting the const type modifier should create a regular pointer and hopefully work. |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
|
| Posted: Tue Feb 24, 2009 1:37 pm |
|
|
| FvM wrote: | | const int16* address is inferring a flash constant pointer, that can't be changed later and possibly don't work in regular assignments to a ram pointer. Omitting the const type modifier should create a regular pointer and hopefully work. |
Thanks for the reply.
I intend it to be a const pointer, in the real program there are nearly 300 variables referenced from a very large const structure that contains pointers to the variables plus there size, min & max values, decimal point position, units etc. All part of a flexible menu and data entry system.
It was all working quite happily with several 1000 units in the field until the customer wanted some more options added.
Then the problems started.
Jim |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue Feb 24, 2009 1:59 pm |
|
|
Historically, CCS has a different meaning for 'const' than MSVC++ does.
From the manual:
| Quote: |
const
Data is read-only. Depending on compiler configuration, this qualifier
may just make the data read-only -AND/OR- it may place the data into
program memory to save space.
Constant Data:
The CONST qualifier will place the variables into program memory.
If the keyword CONST is used before the identifier, the identifier is
treated as a constant. Constants should be initialized and may not be
changed at run-time. This is an easy way to create lookup tables.
CONST=READ_ONLY
Uses the ANSI keyword CONST definition, making CONST variables
read only, rather than located in program memory.
CONST=ROM
Uses the CCS compiler traditional keyword CONST definition, making
CONST variables located in program memory. This is the default mode.
|
|
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Tue Feb 24, 2009 5:10 pm |
|
|
| I was assuming from the code comments, that you have used unintentionally a ROM constant for the pointer. If ROM constant pointers are required for your application, there should be a way. But they are handled specially by the compiler and some usage restrictions apply. |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
| More clarification. |
| Posted: Wed Feb 25, 2009 5:59 am |
|
|
Thanks for the replys,
I'm not sure people are following exactly how my code has been working so here is more of a replica of my code which shows the bug more clearly (i think).
I'm not even sure if it's the same problem as above as this bug only shows up if a structrure crosses over 0x0100 in ram.
I don't understand Pic assembler to follow what it's doing, can somebody have a look at it please.
Many thanks,
Jim
| Code: | #include <18f6722.h>
#device adc=10 *=16
#device ICD=true
#use delay(clock=16000000)
#fuses WDT, WDT512, INTRC_IO, PROTECT, BROWNOUT, PUT, STVREN, NODEBUG, NOMCLR, NOIESO ,NOFCMEN
#fuses NOLVP, NOWRT, NOWRTD, NOWRTB, WRTC, NOCPD, NOCPB, NOEBTR, NOEBTRB, BORV27, NOXINST, NOLPT1OSC
// symbols for units.
const enum {NONE, U_DATE, U_TIME, U_VOLUME, U_TIME_HR_MIN, U_TIME_HR_MIN_D, U_TIME_MIN_SEC};
const enum {DATA_UP_DOWN, DATA_DIGIT, DATA_DISPLAY_ONLY};
struct
{
int8 padding[231]; // 220 and code works ok, 231 and it gives the wrong address to jim3 (230 gives an internal compiler error !!, another bug)
int8 date_format;
int8 time_format;
int8 liquid_units;
int32 range_4_20ma_4ma_adj;
int16 pin_number;
int16 some_stuff[8];
int8 lcd_contrast; // lcd contrast value
// many more here normally
}nvram;
struct
{
int8 seconds;
int8 minutes;
int8 hours;
int8 days;
int8 weekdays;
int8 weeks;
int8 months;
int8 years;
int16 time;
int32 date;
} rtc_decoded;
typedef struct data_entry_struct
{
int8 string1; // Description, index from string table
int8 string2; // Description2, index from string table
int8 entry_method; // data_entry method
int8 var_size; // var size (1=int8,2=16,4=32 etc), easiest to use sizeof(var)
int16 *var_ptr; // pointer to number location
int8 dp; // position of DP on display
int8 units; // uS, time, pin etc
int32 min; // minimum value
int32 max; // maximum value
int32 deflt; // default value for variable if required.
};
struct data_entry_struct const data_lines[3]=
{
//description1 description2, entry method, size var dp units min max default
{ 28, /* Contrast */ 0, /* */ DATA_UP_DOWN, sizeof(nvram.lcd_contrast), &nvram.lcd_contrast, 0, NONE, 0, 63, 26, },
{ 35, /* Date */ 38, /* dd/mm/yy */ DATA_DIGIT , sizeof(rtc_decoded.date), &rtc_decoded.date, 0, U_DATE, 0, 999999, 131103, },
{ 108,/* Time */ 54, /* hh:mm */ DATA_DIGIT , sizeof(rtc_decoded.time), &rtc_decoded.time, 0, U_TIME_HR_MIN, 0, 9999, 1200, },
};
int16 jim1=0;
int16 jim2=0;
int16 jim3=0;
int8 index=0;
void main()
{
setup_oscillator(OSC_16MHZ | OSC_NORMAL);
setup_adc_ports(NO_ANALOGS);
setup_adc(ADC_OFF);
while(1)
{
jim1=&nvram.lcd_contrast; // this works
jim2=data_lines[0].var_ptr; // this doesn't as the compiler hard codes jim2 to equal start address of nvram
jim3=data_lines[index].var_ptr; // this doesn't work if padding on line 17 means the struct nvram crosses over 0x0100 in ram
// so 220 works ok but 231 doesn't as jim3 only gets a 8 bit version of the address.
restart_wdt();
}
}
|
Part of the symbol map when padding = 220 and code works.
| Code: |
000 @SCRATCH
001 @SCRATCH
001 _RETURN_
002 @SCRATCH
003 @SCRATCH
004 @SCRATCH
005-0FA nvram
0FB-108 rtc_decoded
109-10A jim1
10B-10C jim2
10D-10E jim3
10F index
|
Part of the symbol map when padding = 231 and code doesn't work.
| Code: |
000 @SCRATCH
001 @SCRATCH
001 _RETURN_
002 @SCRATCH
003 @SCRATCH
004 @SCRATCH
005-105 nvram
106-113 rtc_decoded
114-115 jim1
116-117 jim2
118-119 jim3
11A index
|
Part of the assembly listing when padding = 220 and code works.
| Code: |
00627 ....................... jim1=&nvram.lcd_contrast; // this works
0008C 6B0A 00628 CLRF jim1+1
0008E 0EFA 00629 MOVLW nvram+245
00090 6F09 00630 MOVWF jim1
00631 ....................... jim2=data_lines[0].var_ptr; // this doesn't as the compiler hard codes jim2 to equal start address of nvram
00092 6B0C 00632 CLRF jim2+1
00094 0E05 00633 MOVLW nvram
00096 6F0B 00634 MOVWF jim2
00635 ....................... jim3=data_lines[index].var_ptr; // this doesn't work if padding on line 17 means the struct nvram crosses over 0x0100 in ram
00098 510F 00636 MOVF index,W
0009A 0D14 00637 MULLW 14
0009C 50F3 00638 MOVF PRODL,W
0009E 6B11 00639 CLRF @@x11
000A0 6F10 00640 MOVWF @@x10
000A2 0E04 00641 MOVLW 04
000A4 2510 00642 ADDWF @@x10,W
000A6 6E01 00643 MOVWF @01
000A8 0E00 00644 MOVLW 00
000AA 2111 00645 ADDWFC @@x11,W
000AC 6E03 00646 MOVWF @03
000AE 5001 00647 MOVF @01,W
000B0 CFF2 F112 00648 MOVFF INTCON,@@112
000B4 9EF2 00649 BCF INTCON.GIE
000B6 0100 00650 MOVLB 0
000B8 DFA5 00651 RCALL 0004
000BA 0009 00652 TBLRD*+
000BC CFF5 F003 00653 MOVFF TABLAT,03
000C0 0101 00654 MOVLB 1
000C2 BF12 00655 BTFSC @@x12.7
000C4 8EF2 00656 BSF INTCON.GIE
000C6 6F0D 00657 MOVWF jim3
000C8 C003 F10E 00658 MOVFF 03,jim3+1
|
Part of the assembly listing when padding = 231 and code doesn't work.
| Code: |
00627 ........................ jim1=&nvram.lcd_contrast; // this works
0008C 0E01 00628 MOVLW nvram+-4
0008E 6F15 00629 MOVWF jim1+1
00090 0E05 00630 MOVLW nvram
00092 6F14 00631 MOVWF jim1
00632 ........................ jim2=data_lines[0].var_ptr; // this doesn't as the compiler hard codes jim2 to equal start address of nvram
00094 6B17 00633 CLRF jim2+1
00096 6F16 00634 MOVWF jim2
00635 ........................ jim3=data_lines[index].var_ptr; // this doesn't work if padding on line 17 means the struct nvram crosses over 0x0100 in ram
00098 511A 00636 MOVF index,W
0009A 0D14 00637 MULLW 14
0009C 50F3 00638 MOVF PRODL,W
0009E 6B1C 00639 CLRF @@x1C
000A0 6F1B 00640 MOVWF @@x1B
000A2 0E04 00641 MOVLW 04
000A4 251B 00642 ADDWF @@x1B,W
000A6 6E01 00643 MOVWF @01
000A8 0E00 00644 MOVLW 00
000AA 211C 00645 ADDWFC @@x1C,W
000AC 6E03 00646 MOVWF @03
000AE 5001 00647 MOVF @01,W
000B0 CFF2 F11D 00648 MOVFF INTCON,@@11D
000B4 9EF2 00649 BCF INTCON.GIE
000B6 0100 00650 MOVLB 0
000B8 DFA5 00651 RCALL 0004
000BA 0009 00652 TBLRD*+
000BC CFF5 F003 00653 MOVFF TABLAT,03
000C0 0101 00654 MOVLB 1
000C2 BF1D 00655 BTFSC @@x1D.7
000C4 8EF2 00656 BSF INTCON.GIE
000C6 6F18 00657 MOVWF jim3
000C8 C003 F119 00658 MOVFF 03,jim3+1
|
Last edited by Jim Hearne on Wed Feb 25, 2009 7:29 am; edited 1 time in total |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Wed Feb 25, 2009 6:42 am |
|
|
I just realized, that you are assigning a pointer value to an integer in the said cases, also in the previously posted code. Generally, these variables can't be considered assignment compatible, so the compilers action is basically undefined. Can you please try again with a code according to the C standard?
You are also mentioning problems with arrays crossing memory space boundaries, but it's unclear if theses are related to the other observations. If you feel that these additional restrictions are causing the problem, can you retry with a memory layout that avoids them? |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
|
| Posted: Wed Feb 25, 2009 7:28 am |
|
|
I don't think theres a problem, CCS seem to treat pointers the same as a normal variables. As long as it's big enough it should be fine.
Just to be sure i've recomplied with jim1, 2 and 3 declared as pointers, the problem is still there and the assembly code is still the same.
In the real program it is treated as a pointer in code like
| Code: | for(l=0;l<NO_OF_DATA_LINES;l++) // init all nvram values {
size=data_entry[l].var_size;
if(size==1)
*(int8 *)data_entry[l].var_ptr=(int8)data_entry[l].deflt;
else if(size==2)
*(int16 *)data_entry[l].var_ptr=(int16)data_entry[l].deflt;
else if(size==4)
*(int32 *)data_entry[l].var_ptr=data_entry[l].deflt;
} |
Which initalizes all the variables refered to in the data_entry structure.
The problem with the array crossing the boundry is the same problem, it only occurs when the array crosses 0x0100 in ram.
My original example at the top simplified things too much and seems to show up another problem which doesn't matter at the moment.
Thanks,
Jim |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
|
| Posted: Wed Feb 25, 2009 8:11 am |
|
|
Further testing reveals that the problem continues at 256 byte boundaries.
So if an extra array of
int8 more_padding[260];
is added before the nvram structure is declared then both jim2 and jim3 get the incorrect address of 0x0109 and only jim1 gets the correct result of 0x0209
This means i can't get round the problem by that method as in the real program structure nvram is larger than 255 bytes.
Jim |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Wed Feb 25, 2009 10:02 am |
|
|
I remember, that a 256 byte offset restriction could be observed in some situations, but it think, there have been workarounds. Unfortunately I'm busy with some projects now, but I would like to look into it, unless other forum members have solved the issue in between.
Sorry for leading you on a wrong track with the pointer versus integer topic, but I learned from experience, that a compiler sometimes can be very strict in this regard. |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Wed Feb 25, 2009 3:23 pm |
|
|
I tried to figure what's actually happening with your code and found two similar, but different bugs, both described correctly in your comments to the assembly code.
The first, more general is regarding pointers to structure members with offsets above 0xff. They are only evaluated correctly in some situation, e. g. simple assignments as jim1=&nvram.lcd_contrast; but incorrectly when calculating pointer constants as used to fill data_lines. This bug isn't particularly related to ROM constants, it also happens when data_lines is located in RAM.
In your example, a possible workaround is to split nvram in several partial arrays, that are placed consecutively in RAM and thus can be accessed also by a continuous index.
A more special bug reveals with jim2=data_lines[0].var_ptr; . It only affects ROM constants, and is caused apparently by a not well designed CCS C shortcut, intended to save the TBLRD overhead, when the constant value can be easily calculated at compile time. Unfortunately the shortcut seems to ignore structure member offsets. I didn't yet find a workaround other than your index=0 construct for jim3.
Don't forget to report the bug at CCS and in win a free update when it has been finally fixed.
P.S.: I saw that you stated splitting nvram doesn't work, but I saw it working, can you try a new with partial arrays below 0x100 size? |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
|
| Posted: Thu Feb 26, 2009 2:35 am |
|
|
Many thanks FvM for taking the time to confirm the bugs.
A pity CCS aren't so quick at replying, i emailed them several days ago before i posted the first message here and again yesterday after i narrowed it down.
PCW ver 8.086 then stopped working completly, crashing at startup and CCS have been helping with that problem, the difference being i emailed [email protected] directly instead of going through the support request option in the compiler.
I'll try the direct address with these bugs, i've not had any support via the normal route in the last 6 months or more despite a current subscription.
Splitting nvram was my next attempt to get round the problem, it's a bit of a pain as the struct nvram is read and written from a loop to backup all the variables in it to EEPROM. So i'll have to duplicate that code and the checksums on it to handle 2 stuctures but it can be done.
Thanks again for your help.
Jim |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Thu Feb 26, 2009 3:37 am |
|
|
My idea with splitting nvram was to use an alias integer array spanning the full range, e.g. for copy operations or checksum generation, but I don't know, if this fits your application.
I'm waiting for several PCD bug fixes since october 2008, I'll decide about a software maintenance plan, when I see substantial progress in this regard. Unfortunately, most bug reports have a status of "we have not had time to further review your e-mail" or at best "A priority has not yet been determined for this issue". However, some arbitray bug reports have been processed rather quickly.
So these guys are apparently very busy, but working somehow.
Frank |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
|
| Posted: Thu Feb 26, 2009 5:58 am |
|
|
Alias ? Do you mean union ?, i wonder if that would work.....
I'm hanging on for today in the hope that CCS will reply with a bug fix but i guess they have only just got up over in the US
Out of interest how can i find out where in rom the compiler stores const data, i can't find it in the list lising or symbol files.
In debugging mode do eval on const variables just gives an "unable to read RAM error" !
Mine you even the mouse over has never worked on const's, either giving garbage values or a similar ram error.
Jim |
|
|
Jim Hearne
Joined: 22 Dec 2003
Posts: 109
Location: West Sussex, UK
|
|
| Posted: Thu Feb 26, 2009 10:19 am |
|
|
Using a union of the struct nvram split into 2 plus an array the size of them both has given me a work around.
Abit at the expense of making the code a bit messy.
I now have to use nv.ram.a.lcd_contrast instead of just nvram.lcd_contrast but i can live with it.
Thanks,
Jim |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
