On 13/09/2021 11:51, Rafał Pietrak via Gcc wrote:
> Hi,
> 
> Thenk you for very prompt reply.


(I'm not sure how much this is a "gcc development list" matter, rather
than perhaps a "gcc help list" or perhaps comp.arch.embedded Usenet
group discussion, but I'll continue here until Jonathan or other gcc
developers say stop.)

> 
> W dniu 13.09.2021 o 10:44, Jonathan Wakely pisze:
>> On Mon, 13 Sept 2021 at 07:55, Rafał Pietrak via Gcc <gcc@gcc.gnu.org> wrote:
> [-----------------]
>>> #elif VARIANT_WORKING
>>>         struct cr_s a = (struct cr_s) {
>>>                         .re = 1,
>>>                         .te = 1,
>>>                         .rxneie = 1,
>>>                         .txeie = 1,
>>>                         .ue = 1 };
>>>         int *b = (int *) &a;
>>>         d->cr1 &= ~(*b);
>>
>> This is a strict aliasing violation. You should either use a union or
>> memcpy to get the value as an int.
> 
> Yes, I know. I know, this is a "trick" I use (I had to use to missleed
> gcc).
> 

Don't think of it as a "trick" - think of it as a mistake.  A completely
unnecessary mistake, that will likely give you unexpected results at times :

    union {
        struct cr_s s;
        uint32_t raw;
    } a = {(struct cr_s) {
        .re = 1,
        .te = 1,
        .rxneie = 1,
        .txeie = 1,
        .ue = 1 }
    };
    d->cr1 &= ~(a.raw);


I hope you also realise that the "VARIANT_TRADITIONAL" and
"VARIANT_WORKING" versions of your code do different things.  The ARM
Cortex-M devices (like the STM-32) are little-endian, and the bitfields
are ordered from the LSB.  So either your list of #define's is wrong, or
your struct cr_s bitfiled is wrong.  (I haven't used that particular
device myself, so I don't know which is wrong.)

Also - perhaps beside the point, but good advice anyway - for this kind
of work you should always use the fixed-size <stdint.h> types such as
"uint32_t", not home-made types like "uint".  And you always want
unsigned types when doing bit operations such as bitwise complement.

Using a union of the bitfield struct with a "raw" combined field is a
common idiom, and gives you exactly what you need here.  It is
guaranteed to work in C, unlike your code (which has undefined
behaviour).  If it is important to you, you should note that it is not
defined behaviour in C++ (though maybe gcc guarantees it will work - the
documentation of "-fstrict-aliasing" is not clear on the matter).

As Jonathan says, a small inline function or macro (using gcc's
extension of declarations and statements inside an expression) can be
used for a wrapper that will work simply and efficiently while being
safe for both C and C++ :

#define raw32(x) \
        ({ uint32_t raw; memcpy(&raw, &x, sizeof(uint32_t)); raw;})


    struct cr_s a = (struct cr_s) {
        .re = 1,
        .te = 1,
        .rxneie = 1,
        .txeie = 1,
        .ue = 1 }
    };
    d->cr1 &= ~raw32(a);


mvh.,

David

Reply via email to