|
|
View previous topic :: View next topic |
Author |
Message |
romanrdgz
Joined: 28 Jan 2011 Posts: 16
|
Corrupt memory issues |
Posted: Thu Nov 24, 2011 2:17 am |
|
|
I have an to store data collected from UART interrupts. I also use an struct declared in the SD/MMC library for CCS mmcsd.c.
Writing didn't worked well until debugging I realised that struct FILE contains a buffer called buf, which was being corrupted with data stored in the buffer used for UART.
I have had this kind of problem before, and I was taught that it was because I wasn't indicating the size of the buffers when declaring them (I don't understand why). Until now I have been declaring their size always, and yet the problem has appeared again.
In fact I'll have to debug more, but I think some int8 variables are getting corrupted too in some way.
So, the question is: why CCS compiler overlaps info in available memory like this? Is there a way to avoid this things happening? Is there a way to manually tell the compiler where to store the data instead of letting him decide where to write it? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19499
|
|
Posted: Thu Nov 24, 2011 3:11 am |
|
|
Start at the beginning.
_Any_ local variable in a function, that is _not_ declared as 'static', uses memory, that is _only_ reserved for the function, while that function is 'in use'. As soon as you exit the function, the memory can and will be used by other things. This applies in compilers like Microsoft C, as well as in CCS. However in general, on machines with more memory, re-use is rare, and on the PC, the memory area will generally be held as 'in use', while the parent program is operating, not just while the routine is operating. On the PIC, because memory is scarce, CCS is aggressive on the re-use.
So first thing is, if you want a variable to remain holding it's contents between successive calls to a function, declare it as static. Cost, more RAM used.
Second thing though is memory corruption from _adjacent_ variables. If (for instance), you declare an array in a PC, and then access a value beyond the end of the memory area that array is located in, the PC _hardware_ will complain. The PC has a memory management system, controlling what code can access what memory. The PIC doesn't. So if you declare an array with 128bytes of storage in the PIC, and start writing to indexes beyond the end of the array, you _will_ destroy the contents of other variables, which may be local, or even in completely unconnected parts of the code. Hence 'limit test' your address. In the case of the buffer, you must ensure that the index can never exceed 127. Also though beware of unsigned arithmetic, and remember that if you have an int8 'index', and subtract '1' from an address of '0', you will be accessing address 255.....
In C, you _always_ have to 'indicate the size of buffers when declaring them'. 'int8 buffer[]', declares a _pointer_ to a 'buffer', with no storage allocated. Exactly the same as 'int8 * buffer'. These are useful constructs, since you could then assign them to point to an area of memory you have already declared elsewhere, or could use a dynamic memory allocation function like malloc, to get the memory. Actual storage space is _only_ allocated when you declare a size (the exception to this, is if you instead declare the contents, when the size will be calculated for you).
Best Wishes |
|
|
RF_Developer
Joined: 07 Feb 2011 Posts: 839
|
Pointers, pointers, pointers. Probably... |
Posted: Thu Nov 24, 2011 3:31 am |
|
|
Let the compiler put things where it wants. It can sort out where things should go far better than you or I can.
So what is the problem? I suspect its to do with the way you write into the buffer. Its not where the buffer *is*, its where you are actually writing most likely. Its probable that the buffer is being written using a pointer. One of the long standing serious weaknesses of C (and also one of its greatest strengths) is pointers, which are wonderfully flexible and allow you to do all sorts of great stuff, but they also allow you to trash memory and write over anything and everything. My guess is that soemwhere the buffer pointer is either getting messed up or is beign used wrongly so that you buffer code is not writing where you think its writing, and instead you are over writing something else, in this case the stream structure. This is exactly the sort of problem that happens a lot with pointers and is why pointers are disliked by a lot of language designers.
You should ALWAYS declare how big your buffers are, other wise yes, the compiler will place stuff in the "wrong" place. C arrays and pointers are very closely related.
So, the anwsers are: CCS compiler doesn't overlap info in memory. Yes, there is a way of stopping this happening: by getting your pointers correct through good code writing and careful debugging, especially at all the special cases, such as where buffer pointers wrap round and when the buffer is full. Yes there is a way of manually telling the compiler where to store data (hint: its very occasionally used to place data structures over SPRs), but this is NOT the way out of your problem.
RF Developer |
|
|
romanrdgz
Joined: 28 Jan 2011 Posts: 16
|
|
Posted: Thu Nov 24, 2011 3:31 am |
|
|
Thanks for your answer. It was very clarifying.
The int8 Buffer[128] from my code was already static, even when I didn't knew it was being kept in memory because of that. I though it only initialised its values to 0!
Anyway it is declared in main.h so it can be used at the interrupt routines and at the main function. So being a global, I guess it would be kept in memory even if it wasn't static, right?
Problem may be caused by the struct FILE then. I was wront, it is declared in fat.c. Its declaration is:
Code: |
struct iobuf
{
fatpos_t
Bytes_Until_EOF, // how many bytes until the stream's end of file
Cur_Char, // the current byte that the stream is pointing at
Entry_Addr, // the entry address of the file that is associated with the stream
Parent_Start_Addr, // the parent's start adddress of the file that is associated with the stream
Size, // the size of the file that is associated with the stream
Start_Addr; // the beginning of the data in the file that is associated with the stream
enum filetype File_Type; // the type of file that is associated with the stream
enum ioflags Flags; // any associated input/output flag
int Buf[STREAM_BUF_SIZE]; // this is a buffer so that during fatputc() or fatgetc()
// the media won't have to be read at every character
};
typedef struct iobuf FILE;
|
So... may I declare and static struct, or make all its components static to avoid the problem then? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19499
|
|
Posted: Thu Nov 24, 2011 4:58 am |
|
|
That code declares a 'type', it doesn't declare the variable.
The variable is declared, where you have 'FILE stream'.
Now, variables declared in 'main', are not inherently 'global'. Ones declared outside _any_ function are.
Have a look at the .sym file. If you declare 'fred' outside any function, you get a variable 'fred' in the sym file. A global variable, that anyone can address as 'fred'. However if you declare 'fred' in main, you instead get a variable 'main.fred'. Inside main, you can address this locally as 'fred'. A bit like using the local code when dialing a number. The memory is held so long as main is alive (so in most standard CCS programs, for as long as the code is running), but other functions inside, can't talk directly to 'fred', so it is not 'global'. If you are talking to 'buffer' in the interrupt, and it is declared in main, and still being found, then it implies some other code has a global 'buffer', and it is _this_ that you are talking to, not the one you think you are addressing. Might explain what is wrong.
Best Wishes |
|
|
romanrdgz
Joined: 28 Jan 2011 Posts: 16
|
|
Posted: Thu Nov 24, 2011 6:06 am |
|
|
So, if I declare FILE as static, should that do the trick? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19499
|
|
Posted: Thu Nov 24, 2011 8:11 am |
|
|
I doubt it.
I strongly suspect you have an issue with how your variables are declared, and how you are addressing them. It may 'hide' the problem for a while, but the real solution is to look at your variable declarations....
Best Wishes |
|
|
romanrdgz
Joined: 28 Jan 2011 Posts: 16
|
|
Posted: Thu Nov 24, 2011 8:31 am |
|
|
So, how will I know when they are wrong declared? I'd like to learn more about this topic. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19499
|
|
Posted: Thu Nov 24, 2011 10:06 am |
|
|
First, I'd start by using a lot of logic. I tend to call character buffers for example 'csomething_buffer', where 'something' reflects what it does. So you have:
char crs232_buffer[32];
Similarly, I tend to reserve small names like 'i', and 'n' for locally declared integers.
Then use fp, for floating point values, and 'ptr' for pointers, so you have things like:
float fpintermediate_result;
char * ptrremote_array;
Basically character names can become comments. It costs you nothing.
You can even call local variables names like 'local_ctr'.
Straight away you 99% remove the 'risk' of accessing something else by mistake.
Then go back to my comments about limit checking. _Whenever_ you use a pointer or an array, make sure you know how it is going to increment (remember incrementing a pointer adds the size of the object it is pointing to), and that your code either inherently limits the offsets to the size allowed, or that _you_ check the limits.
Then if you suspect a memory leak over something, add 'dummy' global variables in front of and after the affected object. Set these to a known value, and debug print them at points in your code. When they change you have identified 'where' the problem is...
You need to remember that it is perfectly 'legal' for instance to have a global variable called 'buffer', and a local variable with the same name. In the routine that has buffer declared locally, talking to buffer will address the local version, while everywhere else using buffer will address the global version. I suspect this is what is happening to you accidentally with your buffer, since if buffer was only declared in main it would _not_ be visible in the ISR. This implies there must be a global 'buffer' being declared somewhere else. You can generally see this by looking at the .sym file, which will then show both a buffer, and a main.buffer. Memory space for variables is initially at least used in the order in which the variables are declared, so looking at the names of the variables each side, can then help you to find where a particular variable is defined.
Key thing to understand is that there is no memory management hardware, and no intrinsic bounds checking in the compiler, so a simple error like forgetting to make a string variable large enough to contain the terminating null, as well as the number of characters you expect _will_ result in variables being overwritten.
RF_Developer has already harked onto the same point, and his comments are right.
Best Wishes |
|
|
romanrdgz
Joined: 28 Jan 2011 Posts: 16
|
|
Posted: Thu Nov 24, 2011 11:33 am |
|
|
I'll try to follow your advice to get this working. I hope I find where the problem is. Thank you very much |
|
|
romanrdgz
Joined: 28 Jan 2011 Posts: 16
|
No luck |
Posted: Mon Nov 28, 2011 4:06 pm |
|
|
I have followed all you advices, but it isn't working yet. The problem is the same: the buffer called 'Buf' into the struct FILE called stream is being overwriten with gata received from uart interruption stored in a buffer called bufferGPS.
bufferGPS is global, and each character is stored increasing an index. The char is only stored if index is < 128-1, being 128 the size of the buffer.
FILE stream (struct defined in fat.c library) is declared locally at the main function, and as I need it in other functions, I'm passing its addresslike this: myFunction(&stream.
That function is defined as int8 myFunction(FILE *stream), and then I'm passing it to other functions yet, and I do it withoout the ampersand. I have checked the address I'm passing and it seems always correct.
Again, the only problem is the same as at the beginning: data from bufferGPS apears into the FILE's buffer.
I even declared a pair of int8 with 0x00 value before anf after declaring the FILE at main, so they get together in memory. Then I checked their values and they still are 0x00.
FILE is quite big in memory, so it appears only a part of it is being corrupted. One of the corrupted parts is the buffer which it's supposed to store the path to a file, so then I'm getting troubles when trying to open, write or closing a file.
The only thing I have thought about, it that int8 bufferGPS[128] is stored between 0AE-12D, and FILE stream is stored between 354-38D. So the last byte match up. I wonder if I'm doing something wrong at the compiler so it doesn't get rid of the first 4 bits, and only of the last 8. Sounds weird for me, but it's the only explanation I have right now.
I need more ideas. Also, if someone could have a look at my code I could upload it anywhere. I would be so grateful.
Please, at least give me some ideas, I'm very lost right now! |
|
|
|
|
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
|