4

In GCC C, how can I get an array of packed structs, where each entry in the array is aligned?

(FWIW, this is on a PIC32, using the MIPS4000 architecture).

I have this (simplified):

  typedef struct __attribute__((packed))
  {
      uint8_t     frameLength; 
      unsigned    frameType       :3;
      uint8_t     payloadBytes;  
      uint8_t     payload[RADIO_RX_PAYLOAD_WORST];
  } RADIO_PACKET;

RADIO_PACKETs are packed internally.

Then I have RADIO_PACKET_QUEUE, which is a queue of RADIO_PACKETs:

typedef struct
{
    short           read;               // next buffer to read
    short           write;              // next buffer to write
    short           count;              // number of packets stored in q
    short           buffers;            // number of buffers allocated in q
    RADIO_PACKET q[RADIO_RX_PACKET_BUFFERS];
} RADIO_PACKET_QUEUE;

I want each RADIO_PACKET in the array q[] to start at an aligned address (modulo 4 address).

But right now GCC doesn't align them, so I get address exceptions when trying to read q[n] as a word. For example, this gives an exception:

RADIO_PACKET_QUEUE rpq;
int foo = *(int*) &(rpq.q[1]);

Perhaps this is because of the way I declared RADIO_PACKET as packed.

I want each RADIO_PACKET to remain packed internally, but want GCC to add padding as needed after each array element so each RADIO_PACKET starts at an aligned address.

How can I do this?

nerdfever.com
  • 1,652
  • 1
  • 20
  • 41
  • 5
    C mandates that arrays consist of *contiguous* sequences of objects. – Kerrek SB Nov 09 '15 at 17:09
  • Why do you declare `RADIO_PACKET` as packed when misalignment is a problem? You do realize that packed structures are not very useful on platforms where misaligned memory access is prohibited as the members of packed structures are not aligned? – fuz Nov 09 '15 at 17:10
  • `*(int*) &(anything that isn't an int)` provokes undefined behavior, and *will* be "mis-optimized" by current compilers. – zwol Nov 09 '15 at 17:11
  • @KerrekSB OK, thanks. So I think that implies I need to make sure each RADIO_PACKET is padded to a modulo 4 length. How do I tell GCC to add the necessary padding (0 to 3 bytes)? – nerdfever.com Nov 09 '15 at 17:12
  • @zwol I need a citation for this being undefined in all cases. – fuz Nov 09 '15 at 17:13
  • 1
    @nerdfever.com You could start by making `RADIO_PACKET` not packed. Do you know what a packed declaration means? (hint: it tells the compiler to leave out any padding). – fuz Nov 09 '15 at 17:13
  • 3
    @nerdfever.com: It seems like "I want padding" and "I want it packed" are two fundamentally incompatible desires. Add your own padding explicitly if you want packing, too. – Kerrek SB Nov 09 '15 at 17:14
  • @FUZxxl It's packed to match the standardized on-air packet format and the memory layout of my radio peripheral. – nerdfever.com Nov 09 '15 at 17:15
  • @nerdfever.com You can achieve that without using unportable and highly problematic packed structures. Structure packing does not belong into a proper C program. Either marshal the data manually (by copying bytes around as needed) or design your structures in a way that packing does not affect you (this shouldn't be a problem in your case). – fuz Nov 09 '15 at 17:17
  • @zwol The *(int*) &(anything that isn't an int) is meant to produce a single 32-bit (word) read instruction, for speed. As long as the address is aligned, that should be much quicker than reading each byte one-by-one. (My problem is that the address isn't aligned.) – nerdfever.com Nov 09 '15 at 17:18
  • @nerdfever.com You could also consider building yourself a macro for unaligned memory access to safely dereference the pointer you make. – fuz Nov 09 '15 at 17:18
  • @nerdfever.com `How do I tell GCC to add the necessary padding (0 to 3 bytes)?` just add array of chars at the end of structure, plus use StaticAssert to make sure that you have right size – fghj Nov 09 '15 at 17:18
  • @nerdfever.com You could add a C11 `_Alignas` attribute to the `payload` array but I don't know how well this interacts with packed structures (which aren't part of standard C). – fuz Nov 09 '15 at 17:19
  • @FUZxxl C11 [N1570] section 6.5 paragraphs 6 and 7. – zwol Nov 09 '15 at 17:21
  • @nerdfever.com Yes, I know what you were trying to do; the problem is that you flat-out *cannot achieve that effect* in standard C; older compilers would compile the code you wrote to the assembly sequence you want, but newer ones are inclined to generate almost anything *but* the assembly sequence you want. – zwol Nov 09 '15 at 17:23
  • @FUZxxl If my memory structure exactly matches what the radio hardware wants, then I can transfer a whole packet using DMA. That's way faster than copying bytes around one-by-one (in an ISR). – nerdfever.com Nov 09 '15 at 17:23
  • 1
    you can declare struct that wraps RADIO-PACKET and keep it unaligned. Then you do the array of this new struct. – Nick Nov 09 '15 at 17:25
  • @zwol Well that doesn't seem like an improvement, does it? I thought the whole point of C is to be a "high level assembler". If you can't make it produce the instructions you want, what's the point? – nerdfever.com Nov 09 '15 at 17:25
  • @Nick Thanks! That sounds like what I want (a way to get the padding between elements, without having to hard-code how many bytes are in each element). Can you post an example as an answer? – nerdfever.com Nov 09 '15 at 17:27
  • @nerdfever.com *If you can't make it produce the instructions you want, what's the point?* If you *want* the compiler to produce *aligned* 32-bit `int` loads, then *write code* for *aligned* 32-bit `int` loads and don't expect code that aliases a **packed** `struct` with an implementation-dependent bit field to produce *aligned* 32-bit `int` accesses to memory. – Andrew Henle Nov 09 '15 at 17:28
  • @AndrewHenle What if I want aligned 32-bit loads for the first elements of the structure, but not for the later ones? My approach was to pack the whole thing, then use casts to force 32-bit loads when I want them. Is there a better way? – nerdfever.com Nov 09 '15 at 17:34
  • @nerdfever.com C *used* to be usable as a portable assembler, but the evolution of the language over the past 25 years or so has been funded by people who want to use it as a replacement for FORTRAN, instead; and these two use cases are directly contradictory. (I'm being colorful, but that really is a good sound-bite explanation of the phenomenon.) Unfortunately, no language has stepped up as a replacement. – zwol Nov 09 '15 at 17:37
  • @nerdfever.com Use aligned `uint32_t` elements and handle the bits yourself. – Andrew Henle Nov 09 '15 at 17:38
  • "As long as the address is aligned, that should be much quicker than reading each byte one-by-one." If the address is aligned, wouldn't the first byte access load the full word into your L1 cache and any further accesses would just hit the cache, meaning there wouldn't be that much of a performance penalty? – JAB Nov 09 '15 at 18:16
  • @zwol The paragraphs you cite don't undermine your statement. Consider `struct { int x; double y; } foo; (int*)&foo;`. This is legal (cf. ISO 9899:2011§6.7.2.1 ¶17) but `struct foo` is not an `int`. – fuz Nov 09 '15 at 18:20
  • @FUZxxl I used to think that was legal, too, but compilers don't agree; see extensive discussion at https://stackoverflow.com/questions/3846551/structures-and-casting-in-c/3846579#3846579 – zwol Nov 09 '15 at 19:12
  • @zwol The post you link to tries something different from what I list. There, OP tries to convert between pointers to structures of different type but with equal first members. This is of course not allowed, §6.7.2.1 only allows conversion to the (transitive) first member of the structure (or to a structure of which the pointee is the first member), converting between pointers to different structures that aren't first members of one-another is not covered. – fuz Nov 09 '15 at 19:26

3 Answers3

4

Since you specify that you are using GCC, you should be looking at type attributes. In particular, if you want RADIO_PACKETs to be aligned on 4-byte (or wider) boundaries, then you would use __attribute__((aligned (4))) on the type. When applied to a struct, it describes the alignment of instances of the overall struct, not (directly) the alignment of any individual members, so it is possible to use it together with attribute packed:

typedef struct __attribute__((aligned(4), packed))
{
    uint8_t     frameLength; 
    unsigned    frameType       :3;
    uint8_t     payloadBytes;  
    uint8_t     payload[RADIO_RX_PAYLOAD_WORST];
} RADIO_PACKET;

The packed attribute prevents padding between structure elements, but it does not prevent trailing padding in the structure representation, exactly as is necessary for ensuring the required alignment for every element of an array of the specified type. You should not then need to do anything special in the declaration of RADIO_PACKET_QUEUE.

This is a lot cleaner and clearer than the alternative you came up with, but it is GCC-specific. Since you were GCC-specific already, I don't see that being a problem.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
2

You can wrap your packed structure, inside another unaligned structure. Then you do array from this unaligned structures.

Solution 2 could be, to add dummy member char[] at the end of the packed structure. In this case you will need to calculate it somehow, probably manually.

I also will suggest you to rearrange your structure, by placing longer members first and placing uint8_t members last (assuming you have 16/32 bit members and not doing some HW mapping).

Colonel Thirty Two
  • 23,953
  • 8
  • 45
  • 85
Nick
  • 9,962
  • 4
  • 42
  • 80
1

I think I've solved this, based on the hint from @Nick in the question comments.

I added a RADIO_PACKET_ALIGNED wrapper around RADIO_PACKET. This includes calculated padding.

Then I substituted RADIO_PACKET_ALIGNED for RADIO_PACKET in the RADIO_PACKET_QUEUE structure.

Seems to work:

typedef struct
{
    RADIO_PACKET packet;
    uint8_t padding[3 - (sizeof(RADIO_PACKET) + 3) % 4];
} RADIO_PACKET_ALIGNED;

typedef struct
{
    short           read;               // next buffer to read
    short           write;              // next buffer to write
    short           count;              // number of packets stored in q
    short           buffers;            // number of buffers allocated in q
    RADIO_PACKET_ALIGNED q[RADIO_RX_PACKET_BUFFERS];
} RADIO_PACKET_QUEUE;

Thanks to all the commenters!

Edit: A more portable version of the wrapper would use:

uint8_t padding[(sizeof(int) - 1) - (sizeof(RADIO_PACKET) + (sizeof(int) - 1)) % sizeof(int)];
nerdfever.com
  • 1,652
  • 1
  • 20
  • 41