Structure packing -- a concern?

Go To Last Post
13 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This additional header added 22/Aug/2009. Please note that some of the people reading this post thought the original question was about bitfields. This was *NOT* the case. The original question was of byte-alignments of odd-byte length members (char and int8u).

Please keep this in mind, when you read the subsequent posts following my original question.

Hi,
I've looked through the forums on AVR-GCC, but could not see anything likely. An element (percentFreeUp) one byte above a sub-structure is being trashed.

Should I be concerned about aligning elements in a structure, particulary if it contains odd bytes?

#define   COMMENT_LEN 20 

typedef struct
{
  int16u  crc;             
  int32u  indexHead;       
  int32u  indexEmptyTail;  
  int32u  recIDTail;       
  int32u  recIDHead;       
} Coords_t;

typedef struct // Bump VERSION, if you change this structure.
{
  int16u  crc;              
  int16u  version;          
  int16u  saneLen;          

  int32u  instrSerialNo;
  int32u  slotCapacity;     
  int8u   recordSize;       <<<<<<<<<<< one byte !!!!!!!
  char    comment[COMMENT_LEN+1]; <<<<<<< odd length !!!!!!!

  // The following is *not* included in the CRC
  int8u   good;             
  int8u   percentFreeUp;   <<<<<<<<<<<< being trashed !!!!!!!

  // These have their own CRCs
  Coords_t  toggle[2];  

} Dir_t;

static Dir_t dsdDir;

What I see is that if toggle[0] is updated, then percentFreeUp is trashed. Is it the way I'm referencing the elements of toggle? I've found that this

  int8u         good          = dsdDir.good;
  int8u         next          = NEXT(good, 2);             
  Coords_t     *pToggleNext   = &dsdDir.toggle[next];

  pToggleNext->indexEmptyTail = indexEmptyTail;
  pToggleNext->indexHead      = indexHead;
  pToggleNext->recIDTail      = recIDTail;
  pToggleNext->recIDHead      = recIDHead;
  pToggleNext->crc            = DsdMemToggleCrc(next); <<<<<<<<<<< this trashes percentFreeUp
  dsdDir.good                 = next;

produces much smaller code than

  int8u         next          = NEXT(dsdDir.good, 2);             

  dsdDir.toggle[next].indexEmptyTail = indexEmptyTail;
  dsdDir.toggle[next].indexHead      = indexHead;
  dsdDir.toggle[next].recIDTail      = recIDTail;
  dsdDir.toggle[next].recIDHead      = recIDHead;
  dsdDir.toggle[next].crc            = DsdMemToggleCrc(next); 
  dsdDir.good                        = next;

Should I be concerned that pToggleNext->crc trashes percentFreeUp? Unfortunately, it is not occurring in the test app, which I can debug, but in a 'production app' build which I cannot.

Regards,
Alf Lacis
http://alfredo4570.customer.netspace.net.au/src

Last Edited: Sat. Aug 22, 2009 - 01:32 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Alf,

I can see no structure alignment or packing problems, because everything is even anyway. The AVR has no alignment issues with SRAM at all, so the auto variables should be OK.

I would immediately suspect your comment[ ] buffer is overwriting other variables.

Allocate a slightly larger comment[ ] buffer, write a magic "ALF" at the end of the buffer. Then check periodically to see if it ever gets overwritten.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
The AVR has no alignment issues with SRAM at all, so the auto variables should be OK.

I can say, though, that avr-gcc 4.x seems to have serious problems when you use bitfield structs whose contents don't add up to byte boundaries.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Quote:
The AVR has no alignment issues with SRAM at all, so the auto variables should be OK.

I can say, though, that avr-gcc 4.x seems to have serious problems when you use bitfield structs whose contents don't add up to byte boundaries.

Can you give an example where it goes wrong?
Because I think i have hit the same light post there.

A structure with loads of seriously odd mixed 8, 16 and 32 bit integers. which whree forming byte spanning variables ended up in total useless code.
it was something along these lines (implementing the csd structure of a mmc/sd card):
struct {

 int8 hello:5;
 int16 there5:   //<== that didn't work as planned
 int16 howare:6; // from there i somewhat lost track if it didnt work or not.
 int8 you:3;
 int8 doing:5;
}

MY MICROCONTROLLER CAN BEAT THE HELL OUT OF YOUR MICROCONTROLLER /ATMEL

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Unfortunately, I do not have an example of the failure. At the time that it was troubling me, I was on a very pressed schedule and I didn't take the time to document the cause of the problem, only the solution. I know the problem arose as a consequence of moving from AVR-GCC 3.x over to AVR-GCC 4.x.

And as a consequence of the experience, I ended up establishing these rules, after which bitfield structs have never bothered me:
1) Never directly intermingle bitfield and non-bitfield types within a single struct.
1a) It is permissible to invoke structs containing bitfields as explicit sub-members of larger structs involving regular integer types, provided rule (2) below is followed.

2) Always make sure that all structs containing bitfields always have a total size that adds up to a multiple of 8 bits. Insert padding bits if necessary to ensure this rule is always observed.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

Firstly, there's no bitfields.

Second, the top part of the Dir_t structure from int16u crc; to char comment[COMMENT_LEN+1]; is fixed once on format (& written to FRAM), and never rewritten: just read on bootup.

Things only change from int8u good; downwards.

Hope that helps with anyone's answer?
Regards,
Alf Lacis
http://au.geocities.com/lacis_alfredo/src

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Why not just single step this in the debugger/simulator and watch the registers holding the struct pointers and see what's doing the damage and if its value is really being incorrectly calculated. Obviously you'll need to switch to the interleaved C/Asm view and determine which registers are being used to hold which pointers (don't rely on 'watch' unless you have no optimisation!)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Why not just single step this in the debugger/simulator and watch the registers holding the struct pointers and see what's doing the damage...
For some processors, the debugger also allows breakpoints on memory access, selectable for read or write. Place an execution breakpoint where your structure is created, then compute where the victim is and place a write breakpoint at that location. Then let the code run. Voila - when the code that trashes your victim byte tries to do the write, you get a break and you know who the culprit is!

Of course, you probably don't have the proper processor (it always seems to be that way), so the simulator is your next best shot.

Stu

Engineering seems to boil down to: Cheap. Fast. Good. Choose two. Sometimes choose only one.

Newbie? Be sure to read the thread Newbie? Start here!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm under the impression that GCC for 8 bit AVRs doesn't do any word alignment. It shouldn't, because there are no alignment requirements for these AVRs.

On the other hand code for the 32 bit AVRs may have alignment concerns. I'm not sure because I don't have one.

You can tell how big a struct is by using sizeof and see what it says.

GCC has an attribute to force structs to have no voids, but you shouldn't have to use it on the 8 bit AVRs.
__attribute__((packed))

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The default makefile includes:

CFLAGS += -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums

I'm guessing the "-fpack-struct" in there is doing something similar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

struct { 
 int8 hello:5;
 int16 there5:   //<== that didn't work as planned
 int16 howare:6; // from there i somewhat lost track if it didnt work or not.
 int8 you:3;
 int8 doing:5;
}

I would interpret that as 3 members being created for the struct, 2 8 bit members, and one 16 bit member.

I suspect you got something like:

byte:8
 [pad:3]|[hello:5]
word:16
 [pad:5]|[howare:6]|[there:5]
byte:8
 [doing:5]|[you:3]

(I'm not sure on the allocation order that GCC uses, I assumed lsb-msb in the above)

What does sizeof(your struct) return? My guess is 4.

Either way, I would avoid bit fields, as they are inherently non-portable, because too much is left to the compiler to decide (and that may change from version to version). If you are dealing with a bit packed structure, access the members with masks and shifts, to guarantee you get things where you expect them, or need them. Only use bitfields when you don't care on how or where the bits are stored.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

1. I repeat, the structures do *not* have bitfields.
2. CPU is ATmega168.
Will get back to this in due course...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

glitch wrote:
Quote:

struct { 
 int8 hello:5;
 int16 there5:   //<== that didn't work as planned
 int16 howare:6; // from there i somewhat lost track if it didnt work or not.
 int8 you:3;
 int8 doing:5;
}

I would interpret that as 3 members being created for the struct, 2 8 bit members, and one 16 bit member.

I suspect you got something like:

byte:8
 [pad:3]|[hello:5]
word:16
 [pad:5]|[howare:6]|[there:5]
byte:8
 [doing:5]|[you:3]

(I'm not sure on the allocation order that GCC uses, I assumed lsb-msb in the above)

What does sizeof(your struct) return? My guess is 4.

Either way, I would avoid bit fields, as they are inherently non-portable, because too much is left to the compiler to decide (and that may change from version to version). If you are dealing with a bit packed structure, access the members with masks and shifts, to guarantee you get things where you expect them, or need them. Only use bitfields when you don't care on how or where the bits are stored.

I totally agree!

Its just those bitfields are used in bucketloads with GCC? [lookaway]Pic[/lookaway] compilers. Sometimes I just feel the need for copying code from one platform to a nother, thats when the $4it gets sour sometimes.

MY MICROCONTROLLER CAN BEAT THE HELL OUT OF YOUR MICROCONTROLLER /ATMEL