I'm using an AT91SAM7 uC and have inherited some C code which is giving me fits. When I run this code I get a data abort error. I've tracked down the offending line of code, and it appears to be caused by something like this:
n = (UINT8) ((UINT16*)(pObject->pVar))[0];
pObject->pVar points to an instance of a structure like this:
typedef __packed struct{ UINT16 v1; UINT32 v2; UINT32 v3; UINT32 v4; UINT32 v5; }OBJTYPE; OBJTYPE xxx = {0,0,0,0,0};
When I stop the debugger on the line that causes the data abort error, I can see that pObject->pVar is pointing to 0x00200173. Is the data abort error happening because that object is not on a 32-bit boundary in memory? If so, how can I [easily] fix that? I've been going through all the online doc's reading about __packed, #pragma pack(n), adding unions to typedefs, etc.; but all the things I've been trying have not fixed the problem.
Is there an easy way (besides the __at__ attribute) to get variables like the above structure to be aligned on 32-bit boundaries?
Can anyone give me a suggestion for how to resolve this problem?
Help... Dave.
That's roughly equivalent to asking if there's an easy way to make triangular wheels roll smoothly, short of making the road a sequence of perfectly shaped arcs fitting triangular wheels of just that size.
The answer to both questions is: "Don't do that, then!". Packing structures and pointer gymnastics like those in the OP are two concepts that just don't mix, period. For the pointer voodoo to work, the standard semantics of C have to be in action. Stuff like __packed breaks that premise.
Thanks for all the suggestions.
The bottom line is that there's too much of this inherited code, and it's full of stuff like this. The problem is that the pObjEntry pointer points to many different kinds of structures, that are all part of an object dictionary for an EtherCAT slave controller. I probably could remove the typecasting to get the first structure element, because it defines the number of entries in this object's dictionary entry. However, I'd need to use typecasts for the remaining elements because they're different for different objects. I'll try your suggestions out and see if that gets me past this roadblock.
The object dictionary is "fixed" once the code has been written and built. It's not dynamic. So, another possible fix is to use a #define to build the code without, and then with the __packed attribute. I could then parse the map file to get the absolute addresses of all these objects, and then use the "at" attribute to hard code the addresses of the object dictionary elements to addresses that are aligned on 32-bit boundaries, and then change the #define to use the __packed attribute.
I know it sounds ugly, but it sounds like the easiest way to be able to use __packed and be able to use typecasts to get stuff out of all these objects at runtime.
The bottom line is that there's too much of this inherited code, and it's full of stuff like this.
So you've got yourselves wedged between a rock and a hard place. That means soft measures will no longer work. You have to bite the bullet and get into gear to fix things in earnest. Yes, that'll hurt. But you'll be better off having done it in the medium term. It's already quite amazing that this cruft ever appeared to work correctly --- but that's no excuse to leave it in that state now you've diagnosed it.
but it sounds like the easiest way to be able to use __packed and be able to use typecasts to get stuff out of all these objects at runtime.
No. The easiest way would be to remove any and all appearances of __packed from that source code. They're causing you nothing but problems the rest of the code is ill-prepared to handle, and with the method you've described you would be losing any advantages it promises anyway. So just lose __packed completely, and never look back.
However, I'd need to use typecasts for the remaining elements because they're different for different objects.
Isn't it something unions can be used for?
That, or if one somehow feels locked into using pointer casts, at least make them struct pointer casts, i.e. instead of
*(first_element *)(&one_struct)
make it
((otherstruct *)(& one_struct))->first_element
Among other things, this has the advantage of seamlessly working for other than the first element, too, and of continuing to work if the type of some elements has to be changed.
Okay... I know you're right... there's no easy way out. Hope is always the last thing to go. Thanks for all the advice and encouragement. Wish me luck!
Mmmmmm......
(Sorry for my limited English ability.)
To help myself to clarify some concepts, I hope that I can confirm one thing with the help from you all.
If the processor is executing
n = (UINT8) ((UINT16*)(0x00200173))[0];
And 0x00200173 is somewhere in RAM, Why Data Abort Exception is triggered?
A 16-bit access (UINT16*) should be made to an even address. Had the compiler known that the data was packed, it would have performed two 8-bit accesses.
To just perform one 8-bit access, you should have had (UINT8*).
nice to c some revent information on following web pages. microcontroller51.blogspot.com/ and see also http://picinf.blogspot.com/
Dr Gull? Don't you mean Dr Spam?
Hi Per,
Many thanks to your help.
I found a page: http://www.keil.com/support/docs/3194.htm
So, I guess, the "evil solution" for the menthioned problem might be?
n = (UINT8) ((__packed UINT16*)(pObject->pVar))[0];
I don't understand why you typecast to a pointer to a 16-bit value if your goal is to throw away the upper 8 bits. Why not use a pointer to a 8-bit value in the first place?
It is only when the pointer points to a "magic" memory address - such as in some cases a SFR - that you may have to read out both bytes at the same time, even if the goal is just to keep one of them.
Why not use a pointer to a 8-bit value in the first place?
As the OP said, the code is inherited, so I guess in practical term nobody can provide the answer. It could be anything ranging from some strange coding style convention to plain incompetence...
Well... here's something funny...
I used a #define and an edit macro to create two copies of the 1st line of each typedef. The goal was to allow conditionally deleting __packed from the typedef's for a little experimentation.
As expected, after removing them, the data abort error went away. What I didn't expect was that the code didn't break. In other words, all the other stuff that used these objects still worked. The really good thing was that the part I enabled (that made the data abort error show up in the first place) just started working.
Who says nice things don't happen once in a while.
Thanks for urging me to remove __packed from all these typedefs. Now I'm really curious, though, why were they there in the first place? They surely don't look like they were needed.
Just to close the loop on using the 8-bit cast to access the 1st 16-bit element in each object definition, I have no idea why that casting was done that strange way. But, this code is full of really odd stuff that would seem to violate any style/coding convention I've ever seen.
Thanks again. Dave.
I can see three reasons for packing.
1) You need to match the memory layout of a memory-mapped device.
2) You need to conserve space.
3) You are lazy and want to access packed structures in file systems or files directly, instead of creating a #define or a helper function for reading/writing the potentially unaligned values.
Note that for 1 and 3, you may also have to take byte order into account, in which case it may not help to pack.
An ARM chip is normally fast enough that you can write code to explicitly pack/unpack data, instead of having the compiler pack the structures. That will help if moving to another compiler or processor.