| View previous topic :: View next topic |
| Author |
Message |
WS_notLogged
Guest
|
| Dynamic memory - malloc - pic24 |
 Posted: Mon Jan 04, 2010 12:50 pm |
 |
|
Hi.
I have one weird problem.
When I run the code below, on my pic 18F it works without problem.
It has been tested for over 3 weeks.
Now, when I port this code to pic24, program is resetting all the time.
Any idea what can be wrong ?
| Code: |
void addData(cap_t g) {
cap_t *temp1,*temp2;
if((temp1=(cap_t *)malloc(sizeof(cap_t)))==NULL) {
printf("\f\nOut of memory");
}
else {
temp1->id=g.id;
temp1->cap_mode=g.cap_mode;
strcpy(temp1->dsc,g.dsc);
temp1->next_cap=NULL;
}
if(start_cap==NULL)
start_cap=temp1;
else {
temp2=start_cap;
while(temp2->next_cap!=NULL) {
temp2=temp2->next_cap;
}
temp2->next_cap=temp1;
}
}
//***********************************************************
void loadData() {
int8 u;
cap_t kkam;
for(u=0;u<caps_msg.num_caps;u++) {
kkam=caps_msg.caps[u];
addData(kkam);
}
}
|
The program is resetting in full meaning of this word.
It jumps right at the beginning.
W. |
|
 |
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Mon Jan 04, 2010 5:00 pm |
|
|
As a preliminary remark, I didn't use yet malloc() with PCD and don't know if it's working correctly. Generally,
I think it's of limited use for embedded computing. It can be meaningful however, if the application structure
suggests concurrent usage of part of the RAM. Of course, the respective memory area must be freed before it
can be used for other purposes. But in most cases, static assignment of memory is more effective in my opinion.
Popular candidates for spontaneous processor resets are address error and stack error trap. There has been
already a forum discussion about the too small default PCD stack allocation. It should be better doubled.
| Code: | | #build(stack = 256) |
http://www.ccsinfo.com/forum/viewtopic.php?t=40083
Address errors can be easily produced, either by accesses of non-existing memory or word accesses at
odd addresses. The shown code is incomplete in so far, that the structure definition and (hopefully) preceeding
variable initialization isn't shown. So possible selfmade problems can't be seen. I previously experienced
also address errors caused by PCD compiler bugs, some of these bugs are possibly continuing until most
recent PCD version.
The PIC24 address error trap can be used to reveal coding errors.
http://www.ccsinfo.com/forum/viewtopic.php?t=36479 |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Tue Jan 05, 2010 9:02 am |
|
|
| Quote: | | As a preliminary remark, I didn't use yet malloc() with PCD and don't know if it's working correctly. |
It doesn't at all. I found that with all PCD versions up to most recent V4.104, the below init code was included at the begin of main, if either
#pragma USE DYNAMIC_MEMORY or #include <stdlibm.h> is specified.
CLR CB9 is an illegal word access to an odd memory location and causes a processor reset.
CLR.B CB9 would be correct.
| Code: | 0056E: MOV #CB8,W4
00570: MOV W4,806
00572: MOV.B #C5,W0L
00574: MOV.B W0L,CB8
00576: CLR CB9 |
P.S.: Why we always find PCD features, that obviously never have been tested after coding? |
|
|
Guest
|
|
| Posted: Wed Jan 06, 2010 6:01 am |
|
|
Hi
Problem is with the assignment of __DYNAMIC_HEAD that is aligned to an odd memory address. Also the position of __DYNAMIC_HEAD is often in the same as that used by locals. I suggest assigning your own and making it a global array. This not only aligns the address but ensures the compiler understands that that memory is not available for other variables.
Remove #USE DYNAMIC_MEMORY
add
| Code: |
#elif defined(__PCD__)
typedef struct nodet {
unsigned int16 size;
unsigned int16 next; }node_t;
#define DMEMSIZE 0x800 // words
unsigned int16 dynamic_mem[DMEMSIZE];
unsigned int16 *__DYNAMIC_HEAD;
#endif
#include "memmgmt.c"
void start_dm()
{
__DYNAMIC_HEAD = dynamic_mem;
*__DYNAMIC_HEAD = DMEMSIZE*2;
*(__DYNAMIC_HEAD+1)= 0;
}
|
and include start_dm(); in your initialisation. |
|
|
Guest
|
|
| Posted: Wed Jan 06, 2010 6:08 am |
|
|
| Sorry forgot to say that code should be added to STDLIBM.H |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Wed Jan 06, 2010 8:55 am |
|
|
Thank you for the suggestion, it seems to remove at least the obvious errors. Basically it's a PCD bug that
will be hopefully fixed by CCS in time.
I didn't check however the full functionality of malloc(). I also wonder, what's the PCD standard
method to assign dynamic memory? Ideally, it should allocate all free RAM. |
|
|
Guest
|
|
| Posted: Wed Jan 06, 2010 9:34 am |
|
|
Yes it should but there is no heap used by the CCS compiler so it calculates where the locals are. If it was able to calculate the usage of the locals correctly it should work, but as programs get bigger and bigger for large PIC24's the smallest error gets multiplied.
This also stops recursion and pointers to functions that are nice features of ansi C. Of course these functions don't have much use in PIC12/16/18 and in fact accounts for the very efficient code for these small devices, but in the much more powerful PIC24/33's the compiler should have been rewritten to allow full C' features instead of just bolting on to the old compiler design.
This is feel is the main reason that there have been so many bugs in the PCD.
BTW I used dynamic allocation implementing a FAT file system to allow for unknown number of files open at the same time and for path structures for folders to any depth. These PIC24's are sooo good. |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Wed Jan 06, 2010 10:33 am |
|
|
| So you have some hope for the original poster, that malloc is working correctly with the modification? |
|
|
Guest
|
|
| Posted: Wed Jan 06, 2010 10:58 am |
|
|
| Yes it did work. I did have some problems with calloc and free that for some reason had to have name changes. I used f_ree and c_alloc. I didn't investigate. The code has been working now for a few weeks and seems to be stable. The application is 24/7 active. |
|
|
W_Schauberg
Joined: 17 Sep 2009
Posts: 16
|
|
| Posted: Mon Jan 11, 2010 2:01 am |
|
|
Hello FVM and "guest".
I have added the changes to the code and this is the situation:
- I get less error traps ( one or two )
- If I use pointers program generates much more traps
- I can't use some parts of the program
- Program is big but the reduced version makes the same errors.
-------------------------------------------------------------------
@FVM
I will send you new very small version of the code, so when you have time, please take a look.
@guest
Please be so kind and PM me or send me some email, where I can send you test code?. It's quite important to me and I can't describe this part here. The code is small and easy to read... |
|
|
WS_notLogged
Guest
|
|
| Posted: Thu Jan 14, 2010 5:51 am |
|
|
Hi guys.
Any idea regarding my original post ?
I send this problem to ccs support before few days but i still wait for answer.
W. |
|
|
FvM
Joined: 27 Aug 2008
Posts: 2337
Location: Germany
|
|
| Posted: Thu Jan 14, 2010 4:37 pm |
|
|
I finally had some time to look into the code. I found, that only one dataword rather than the full structure is
copied to the function parameter storage
| Code: | kkam=caps_msg.caps[u];
addData(kkam);
00732 F8190E push.w 0x190e
00734 F91910 pop.w 0x1910
00736 020654 call 0x000654
|
Generally, PCD is supporting structures in assignments, kkam is e.g. assigned correctly. Also the parameter
storage for the function is correctly implemented.
| Code: | | void addData(cap_t g); |
Personally, I would never use a construct like this, because it's a huge waste of precious memory. Using a
pointer instead makes the code work correctly, as far as I see.
| Code: | void addData(cap_t *g) {
cap_t *temp1,*temp2;
if((temp1=(cap_t *)malloc(sizeof(cap_t)))==NULL) {
printf("\f\nOut of memory");
}
else {
temp1->id=g->id;
temp1->cap_mode=g->cap_mode;
strcpy(temp1->dsc,g->dsc);
temp1->next_cap=NULL;
}
if(start_cap==NULL)
start_cap=temp1;
else {
temp2=start_cap;
while(temp2->next_cap!=NULL) {
temp2=temp2->next_cap;
}
temp2->next_cap=temp1;
}
}
void loadData() {
int8 u;
cap_t *kkam;
for(u=0;u<caps_msg.num_caps;u++) {
kkam=&caps_msg.caps[u];
addData(kkam);
}
} |
Trying to replace the structure element copy in addData() by a more compact structure assignment reveals
another PCD V4.104 bug.
| Code: | *temp1=*g;
00684 80C895 mov.w 0x1912,0x000a
00686 80C887 mov.w 0x1910,0x000e
00688 780086 mov.w 0x000c,0x0002 !!!SHOULD BE 0x000a !!!!
0068A 780107 mov.w 0x000e,0x0004
0068C 09001B repeat #27
0068E 7858B2 mov.b [0x0004++],[0x0002++] |
As many PCD users already experienced, it wants to be spoon-feed and doesn't like complex constructs 
Regards,
Frank |
|
|
miketwo
Guest
|
| Also |
| Posted: Thu Feb 25, 2010 1:12 pm |
|
|
I was also having problems of this sort.
In addition to the fix above (removing #USE DYNAMIC MEMORY and coding an alternate), I went to the stddef.h file and changed the definition of size_t, which is used in the malloc code. In mine it was set to int8, and really it should be int16.
| Code: | #define size_t unsigned int16
|
Now I don't get any reboots so long as I'm not trying to allocate more than about 2k... That's pretty good for what I need. Though I wish the malloc function would die more gracefully (just return NULL) if the memory isn't available rather than resetting the processor.
Cheers,
Michael |
|
|
miketwo
Guest
|
| Even more developments... |
| Posted: Thu Feb 25, 2010 8:13 pm |
|
|
Don't want to spam the boards, but have been working on this nonstop, and found a decent workaround.
It basically boiled down to this:
malloc(even number) was ok.
malloc(odd number) caused a reboot.
According to someone who knows this way more than me, this is common knowledge. 
But I'm a n00b at this, so I'm posting this for the other new guys out there that stumble across this page when they're faced with "random" reboots.
Workaround:
| Code: | if(size%2==0) store=(char*)malloc(size); //even
else store=(char*)malloc(size+1); //odd
|
|
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
