Bit-packing Problem in a Union (Register Mapping)

Question

I am trying to get a union together to map out some bit fields in a register map. The code I have is the following:

typedef union __attribute__((packed)) {
    struct {
    uint8_t     MODE:3;
    uint8_t     VSHCT:3;
    uint8_t     VBUSCT:3;
    uint8_t     AVG:3;
    uint8_t     RSVD:3;
    uint8_t     RST:1;
    };
    
    struct {
    uint8_t     lsbyte:8;
    uint8_t     msbyte:8;
    };
    uint16_t    w;
    
} CON_MAP_t;

I am initializing the fields with:

CON_MAP_t map = {
    .RST =      0,
    .RSVD =     4,
    .AVG =      0,
    .VBUSCT =   4,
    .VSHCT =    4,
    .MODE =     7
}

So far this is all fine, no compiler issues or warnings.

I expect the binary/hex representation to be 01000001_00100111 / 0x4127.

However, in the debugger I end up with a value for 'w' of: 00000100_00100111 The least significant byte is correct, but the msb(yte) is not.

I am not sure if I'm missing something fundamental here and I've just been staring at it too long, but any insight would be highly appreciated!

I am using: MPLABX v6.05 Latest XC32 Compiler

Device is a PIC32MX130F064D debugging with a PICKIT4.

Answer 1

As noted in comments and in other posts such as Why bit endianness is an issue in bitfields?, bit-fields are so poorly defined by the C standard that it's not even funny. Put a nasty reputation compiler like MPLAB on top of that mess and you'll have a recipe for disaster.

My advise is to forget that you ever heard of bit-fields and write the code using 100% portable, standardized integer constants through macros. In this specific case there's no obvious need to type pun between word and byte - why would you ever need to byte access this since the bit-fields are all over the place?

Assuming your hardware register is named CON_MAP and the CPU is little endian (endianess matters for your bit-field code but not for my solution below) then:

#define CON_MAP (*(volatile uint16_t*)0x12345678u) // physical address

#define CON_MAP_MODE_MASK   0x3u
#define CON_MAP_MODE_BIT    0u
#define CON_MAP_MODE(val)   ( ((val) & CON_MAP_MODE_MASK) << CON_MAP_MODE_BIT )
#define CON_MAP_VSHCT_MASK  0x3u
#define CON_MAP_VSHCT_BIT   2u
#define CON_MAP_VSHCT(val)  ( ((val) & CON_MAP_VSHCT_MASK) << CON_MAP_VSHCT_BIT )
...

Usage:

CON_MAP = CON_MAP_MODE(2u)   | 
          CON_MAP_VSHCT(3u)  |
          CON_MAP_VBUSCT(0u) |
          ...                ;

The values 2u, 3u etc should ideally be replaced by named constants in case you have some meaningful names for them. Like: CON_MAP_MODE(FANCY_MODE | SPECIAL_MODE).

The above is one of several common industry standard ways to implement hardware registers in embedded systems. More info here: How to access a hardware register from firmware?

Answer 2

It's implementation-defined whether bit-fields can span across the boundary of the datatype on which they're defined. There are various other details too. See following excerpts from the reference:

The following properties of bit-fields are implementation-defined:

• Whether types other than int, signed int, unsigned int, and _Bool (since C99) are permitted
• Whether a bit-field can straddle an allocation unit boundary
• The order of bit-fields within an allocation unit (on some platforms, bit-fields are packed left-to-right, on others right-to-left)

In this case, you have an 8-bit datatype into which you're trying to pack 3 groups of 3 bits. The third group straddles a boundary and it seems your compiler implementation does not support that. It looks like the value has been moved to the next byte, which probably means sizeof(CON_MAP_t) is greater than 2.

So at the very least, change your types used in the first struct to uint16_t, but be aware that support for this is also implementation-defined (as per excerpts shown earlier).

As a side-note, there's no need to define bit-fields in that second struct (where you've specified 8 bits for each uint8_t).

typedef union __attribute__((packed)) {
    struct {
        uint16_t    MODE    : 3;
        uint16_t    VSHCT   : 3;
        uint16_t    VBUSCT  : 3;
        uint16_t    AVG     : 3;
        uint16_t    RSVD    : 3;
        uint16_t    RST     : 1;
    };
    
    struct {
        uint8_t     lsbyte;
        uint8_t     msbyte;
    };

    uint16_t    w;
} CON_MAP_t;

It should be noted that using bit-fields for this kind of thing is heavily platform- and compiler-dependent. And using them for type-punning violates strict aliasing.

For any kind of portability, you should look to other solutions. And even targeting one specific platform, it's wise to at least create plenty of unit tests for this structure to ensure sanity.

Answer 3

When it comes to using compilers that supports some additional features over standard C compilers, like XC32 Compiler, it always is better to refer to its guides and manuals for special cases like this one.

The XC32 does fully support bit fields in structures and also guarantee the order as the first defined bit as to be the Least Significant bit. Here is how it described in section 8.6.2 Bit Fields in Structures of XC32 C Compiler User's Guide:

MPLAB XC32 C/C++ Compiler fully supports bit fields in structures. Bit fields are always allocated within 8-bit storage units, even though it is usual to use the type unsigned int in the definition. Storage units are aligned on a 32-bit boundary, although this can be changed using the packed attribute.

The first bit defined will be the Least Significant bit of the word in which it will be stored. When a bit field is declared, it is allocated within the current 8-bit unit if it will fit; otherwise, a new byte is allocated within the structure. Bit fields can never cross the boundary between 8-bit allocation units.

For example, the declaration:

struct {
     unsigned lo    : 1;
     unsigned dummy : 6;
     unsigned hi    : 1;
} foo;

will produce a structure occupying 1 byte.

So according to the guide description you should see the same order as per your bit definition.

Also note that packed attribute is meant to use only if you want to alter the 32-bit boundary. But it is not necessary since yours is only 16-bits.

Here is a demo showing the expected result following with the code:

Demo code shown in the screenshot:

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>

typedef union  {
    struct {
        unsigned     MODE:3;
        unsigned     VSHCT:3;
        unsigned     VBUSCT:3;
        unsigned     AVG:3;
        unsigned     RSVD:3;
        unsigned     RST:1;
    };
    
    struct {
        uint8_t     lsbyte:8;
        uint8_t     msbyte:8;
    };
    uint16_t    w;
    
} CON_MAP_t;

int main(int argc, char** argv) {
    
    CON_MAP_t map = {
        .RST =      0,
        .RSVD =     4,
        .AVG =      0,
        .VBUSCT =   4,
        .VSHCT =    4,
        .MODE =     7
    };
    
    if(map.lsbyte == 0x27 && map.msbyte == 0x41)
        return (EXIT_SUCCESS);
    else
        return (EXIT_FAILURE);
}

Answer 4

I think I may have figured this out by doing the last thing I could think of. I changed from the macro type definitions to standard "int" and also added a bit count to the "w" field:

typedef union __attribute__((packed)) {
    struct {
    int     MODE:3;
    int     VSHCT:3;
    int     VBUSCT:3;
    int     AVG:3;
    int     RSVD:3;
    int     RST:1;
    };
    
    struct {
    int     lsbyte:8;
    int     msbyte:8;
    };
    int w:16;
    
} CON_MAP_t;

This seems to have fixed the problem in the hardware implementation.

Let me know if there's any other fundamental understanding here that I may have missed.

Thanks!