cancel
Showing results for 
Search instead for 
Did you mean: 

Have an issue with reading from union of bitfield structures. The compilation of the program is successful, however the execution interrupts and ends up in the default interrupt handler, which by default is an endless loop. Could this be a compiler issue?

Softronic Solutions
Associate III

To pars binary information I created a series of bitfield structures

typedef struct PD_SOURCE_PDO_CAPABILITY {
    uint32_t MAX_CURRENT_UNITS : 10;	
    uint32_t VOLTAGE_UNITS : 10;
    uint32_t PEAK_CURRENT : 2;
    uint32_t RESERVED_BITS22_23 : 2;
    uint32_t EXTENDED_MSG_SUPPORT : 1;
    uint32_t DUAL_ROLE_DATA_ABLE : 1;
    uint32_t USB_CAPABLE : 1;
    uint32_t UNCONSTRAINED_POWER    : 1;
    uint32_t USB_SUSPEND_SUPPORT : 1;
    uint32_t DUAL_ROLE_POWER_ABLE : 1;
    uint32_t PDO_TYPE : 2;	
} PD_SOURCE_PDO_CAPABILITY_T;
 
typedef struct PD_SOURCE_PDO_FIXED {
    uint32_t MAX_CURRENT_UNITS : 10;
    uint32_t VOLTAGE_UNITS : 10;
    uint32_t PEAK_CURRENT : 2;
    uint32_t RESERVED_BITS22_29 : 8;
    uint32_t PDO_TYPE : 2;
} PD_SOURCE_PDO_FIXED_T;
 
typedef struct PD_SOURCE_PDO_BATTERY
{
    uint32_t MAX_POWER_UNITS : 10;
    uint32_t MIN_VOLTAGE_UNITS : 10;
    uint32_t MAX_VOLTAGE_UNITS : 10;
    uint32_t PDO_TYPE : 2;
} PD_SOURCE_PDO_BATTERY_T;
 
typedef struct PD_SOURCE_PDO_VARIABLE
{
    uint32_t MAX_CURRENT_UNITS : 10;
    uint32_t MIN_VOLTAGE_UNITS : 10;
    uint32_t MAX_VOLTAGE_UNITS : 10;
    uint32_t PDO_TYPE : 2;
} PD_SOURCE_PDO_VARIABLE_T;
 
typedef struct PD_SOURCE_PDO_AUGMENTED
{
    uint32_t MAX_CURRENT_UNITS	: 7;
    uint32_t RESERVED_BIT7 : 1;
    uint32_t MIN_VOLTAGE_UNITS : 8;
    uint32_t RESERVED_BIT16 : 1;
    uint32_t MAX_VOLTAGE_UNITS : 8;
    uint32_t RESERVED_BITS25_26 : 2;
    uint32_t POWER_LIMITED : 1;
    uint32_t RESERVED_BITS28_29 : 2;
    uint32_t PDO_TYPE : 2;
} PD_SOURCE_PDO_AUGMENTED_T;

Because a stream of data can contain either of above bitfields I created a union of bitfield structs.

typedef union PD_SOURCE_PDO
{
    uint32_t SCALAR;
    PD_SOURCE_PDO_CAPABILITY_T CAPABILITY_PDO;
    PD_SOURCE_PDO_FIXED_T FIXED_PDO;
    PD_SOURCE_PDO_BATTERY_T BATTERY_PDO;
    PD_SOURCE_PDO_VARIABLE_T VARIABLE_PDO;
    PD_SOURCE_PDO_AUGMENTED_T AUGMENTED_PDO;
} PD_SOURCE_PDO_T;

the code that is actually handling the data streams is as follows

for (PD_ACTIVE_OBJECT_T Object = 0; Object < NumOfObjects; Object++)  {
    PD_SOURCE_PDO_T* SOURCE = (PD_SOURCE_PDO_T*)(pData + (Object * sizeof(PD_SOURCE_PDO_T)));
    PD_POWER_OBJECT_T* POWER = (PD_POWER_OBJECT_T*)&pInstance->PowerObject[Object];
    if (Object == 0 ) {
        pInstance->DataEnabled = SOURCE->CAPABILITY_PDO.USB_CAPABLE;
        pInstance->Unconstrained = SOURCE->CAPABILITY_PDO.UNCONSTRAINED_POWER;
        pInstance->ExtMsgSupport = SOURCE->CAPABILITY_PDO.EXTENDED_MSG_SUPPORT;
        pInstance->DualRoleData = SOURCE->CAPABILITY_PDO.DUAL_ROLE_DATA_ABLE;
        pInstance->DualRolePower = SOURCE->CAPABILITY_PDO.DUAL_ROLE_POWER_ABLE;
        pInstance->SuspendEnabled = SOURCE->CAPABILITY_PDO.USB_SUSPEND_SUPPORT;
    }
    POWER->PowerSource = SOURCE->CAPABILITY_PDO.PDO_TYPE;
    //POWER->PowerSource = READ_MASKED(SOURCE->SCALAR, 2, 30);
    if (POWER->PowerSource == PD_POWER_SOURCE_BATTERY)  {
        //POWER->MaxVoltage = SOURCE->BATTERY_PDO.MAX_VOLTAGE_UNITS * 50;
        //POWER->MinVoltage = SOURCE->BATTERY_PDO.MIN_VOLTAGE_UNITS * 50;
        //POWER->Power = SOURCE->BATTERY_PDO.MAX_POWER_UNITS * 250;
        //POWER->Current = (POWER->Power/POWER->MinVoltage)*1000;
    } else if (POWER->PowerSource == PD_POWER_SOURCE_VARIABLE)  {
        //POWER->MaxVoltage = SOURCE->VARIABLE_PDO.MAX_VOLTAGE_UNITS * 50;
        //POWER->MinVoltage = SOURCE->VARIABLE_PDO.MIN_VOLTAGE_UNITS * 50;
        //POWER->Current = SOURCE->VARIABLE_PDO.MAX_CURRENT_UNITS * 10;
        //POWER->Power = ((POWER->MaxVoltage*POWER->Current)/1000);
    } else if (POWER->PowerSource == PD_POWER_SOURCE_AUGMENTED)  {
        //POWER->MaxVoltage = SOURCE->AUGMENTED_PDO.MAX_VOLTAGE_UNITS * 100;
        //POWER->MinVoltage = SOURCE->AUGMENTED_PDO.MIN_VOLTAGE_UNITS * 100;
        //POWER->Current = SOURCE->AUGMENTED_PDO.MAX_CURRENT_UNITS * 50;
        //POWER->Power = ((POWER->MaxVoltage*POWER->Current)/1000);
    } else { //PD_POWER_SOURCE_FIXED
        //PD_VOLTAGE_T VoltUnits = READ_MASKED(SOURCE->SCALAR, 10, 10);
        PD_VOLTAGE_T VoltUnits = SOURCE->FIXED_PDO.VOLTAGE_UNITS;
        //PD_CURRENT_T AmpUnits = READ_MASKED(SOURCE->SCALAR, 10, 0);
	PD_CURRENT_T AmpUnits = SOURCE->FIXED_PDO.MAX_CURRENT_UNITS;
        POWER->MaxVoltage = VoltUnits * 50;
        POWER->MinVoltage = POWER->MaxVoltage;
        POWER->Current =  AmpUnits * 10;
        POWER->Power = ((POWER->MaxVoltage*POWER->Current)/1000);
    }
}

The issue is triggered when reading from a pointer variable SOURCE with is a union of bitfield type.

This seams to be working

POWER->PowerSource = SOURCE->CAPABILITY_PDO.PDO_TYPE;
08002a78:   ldr     r3, [r7, #28]
08002a7a:   ldrb    r3, [r3, #3]
08002a7c:   lsls    r3, r3, #24
08002a7e:   lsrs    r3, r3, #30
08002a80:   uxtb    r3, r3
08002a82:   sxtb    r2, r3
08002a84:   ldr     r3, [r7, #24]
08002a86:   strb    r2, [r3, #16]

But this fails to execute

PD_VOLTAGE_T VoltUnits = SOURCE->FIXED_PDO.VOLTAGE_UNITS;
08002aa6:   ldr     r3, [r7, #28]
08002aa8:   ldr     r3, [r3, #0]
08002aaa:   lsls    r3, r3, #12
08002aac:   lsrs    r3, r3, #22
08002aae:   uxth    r3, r3
08002ab0:   str     r3, [r7, #20]

Causing a jump to

.section .text.Default_Handler,"ax",%progbits
Default_Handler:
Infinite_Loop:
  b Infinite_Loop
  .size Default_Handler, .-Default_Handler

At this point I'm clue less of what's going on?

Even stranger if I compile the code as below the previous assignment statement is now also failing?

POWER->PowerSource = READ_MASKED(SOURCE->SCALAR, 2, 30);
08002a78:   ldr     r3, [r7, #28]
08002a7a:   ldr     r3, [r3, #0]
08002a7c:   lsrs    r3, r3, #30
08002a7e:   sxtb    r2, r3
08002a80:   ldr     r3, [r7, #24]
08002a82:   strb    r2, [r3, #16]

What's up doc?

11 REPLIES 11

Well the compiler assumes that the structure is aligned.

The problem here is that you cast it from pData which is unaligned.

On the old ARM7/ARM9 platforms it was generally advisable to memcpy() to an aligned auto-variable copy when dealing with memory or file packed/byte aligned structures.

The alternative is for the compile to know something is unaligned, or potentially, and then construct 32-bit words by reading as 4 individual byte accesses.

One could also test the pointer alignment, and optionally copy to an aligned buffer when it is not.

Tips, Buy me a coffee, or three.. PayPal Venmo
Up vote any posts that you find helpful, it shows what's working..

> different and more intelligent approach

Then you have to invent it yourself - either write in assembler, or write yourself a compiler from whatever higher level description (a.k.a. language) you wish. This is not trolling, I mean it.

The traditional standpoint of programming languages is to provide you with convenience like math-like operations and portability - a certain level of abstraction, in exchange for some inefficiency and certain loss of control. If you wish to regain that control - e.g. by prescribing particular layout of data - you must give up the covenience - i.e. the high-level language does not work anymore. In particular, in C, there are two ways to prescribe data layout - pointer cast, and unions. Both are directly warned against in the standard (according to standard, both result in undefined behaviour). That they are widely used (and I use them, too) does not make them any less wrong.

Packed bitfields with assumed arrangement are in fact the third such way - and you've conveniently omitted the fact that you've packed the bitfields - and it's even more evil than those traditional two. And I do use them, too. Extensively. But then I also try to avoid Cortex-M0/M0+ as far as I can (and on those, I don't use any of the abovementioned techniques). Cortex-M3/M4 luckily has much less alignment constraints (there are some, and are source of painful gotchas too).

The only true portable method is, if you have buffer of bytes, treat them as bytes, and rearrange them manually, in that most painful way.

JW