• AVR Freaks

Hot!XC32 Bug: Pointer accessing shifted member of packed struct

Page: 12 > Showing page 1 of 2
Author
Paul PortSol
Super Member
  • Total Posts : 620
  • Reward points : 0
  • Joined: 2015/07/03 11:52:03
  • Location: Newfoundland, Canada
  • Status: offline
2020/04/03 08:16:45 (permalink)
0

XC32 Bug: Pointer accessing shifted member of packed struct

Target crashes (exception) when use a pointer to access member of a packed struct, when that member has been shifted off alignment.
Direct access to shifted members of packed struct is OK, issue is only when use pointers to access those members. 
 
See attached project:
- PointerStructCrashV06PR_ICD4=PGx2_CrashWhenPointerAccessStructMemberShiftedByPacking.zip
- XC32v240 and earlier  (bug has been around for years but I only just narrowed it down to packed structs)
- PIC32MZ with ICD4 on PGx2
- Harmonyv206
- MPLABXv530 
 
typedef struct __attribute__ ((packed)) {//packed struct (no sub-structs)
    uint8_t _uA;
    uint8_t _uB;
    uint16_t _u16aligned;//Packing doesn't shift alignment of this element so 16bit still on 16bit boundary
    uint8_t _uC;
    uint16_t _u16shifted;//Packing shifts alignment of this element so 16bit isn't on 16bit boundary
} S_PackedStruct;
typedef struct {//simple struct (no sub-structs)
 
    uint8_t _uA;
    uint8_t _uB;
    uint8_t _uC;
    uint16_t _u16aligned;
} S_Struct;
 
//...
 
    uint16_t *puSimple, *puS, *puPSa, *puPSs; 
 
    uSimple = 0xA001;
    sS._u16aligned = 0xB001;
    sPS._u16aligned = 0xC001;
    sPS._u16shifted = 0xD001;//OK even when access 16bit shifted by packing
 
    puSimple = &uSimple;
    puS = &sS._u16aligned;
    puPSa = &sPS._u16aligned;//aligned to 16bit boundary
    puPSs = &sPS._u16shifted;//shifted by packing

    *puSimple = 0xA002;//OK
    *puS = 0xB002;//OK
    *puPSa = 0xC002;//OK
    *puPSs = 0xD002;//#### Crash when use pointer to access 16bit shifted by packing ####

 
I will submit ticket unless someone points out where I got this wrong.
Paul
 
 
 
post edited by Paul PortSol - 2020/04/03 09:13:22

Attachment(s)

Attachments are not available: Download requirements not met
#1

26 Replies Related Threads

    simong123
    Lab Member No. 003
    • Total Posts : 1391
    • Reward points : 0
    • Joined: 2012/02/07 18:21:03
    • Location: Future Gadget Lab (UK Branch)
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 09:44:56 (permalink)
    +3 (3)
    That is certainly the behaviour I would expect. When using pointers it is up to you to ensure alignment.
    One of the joys of RISC :)
     
    Edit: what do you get if you use:
    S_PackedStruct *psPS=&sPS;

    pSPS->_u16shifted=0xD002;
    ?
    post edited by simong123 - 2020/04/03 10:01:51
    #2
    andersm
    Super Member
    • Total Posts : 2823
    • Reward points : 0
    • Joined: 2012/10/07 14:57:44
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 12:05:04 (permalink)
    0
    The problem is that your pointer type is "uint16_t*", ie. a pointer to a regular uint16_t, which must be aligned. GCC can apply the packed attribute only structs or unions, so (AFAIK) there's no way to do what you want. Instead, make sure to only access the fields via a S_PackedStruct* pointer, and you'll be fine.
    #3
    NKurzman
    A Guy on the Net
    • Total Posts : 18799
    • Reward points : 0
    • Joined: 2008/01/16 19:33:48
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 12:21:44 (permalink)
    +1 (1)
    You can Google “unaligned pointers in GCC”.
    It looks like a method would be to use your pointer to read the individual bytes, then reassemble them into a word. it’s not as kludgy as it sounds. Since it what the CPU would have to do anyway.
    #4
    aschen0866
    Super Member
    • Total Posts : 4567
    • Reward points : 0
    • Joined: 2006/01/08 22:18:32
    • Location: San Diego
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 16:46:01 (permalink)
    0
    You can use a function like this to get a uint16_t value with no assumption about the alignment.

    uint16_t get_unaligned16(const void *p)
    {
    const struct unaligned
    {
    uint16_t retval;
    } __attribute__((packed)) * rp;
    rp = p;
    return rp->retval;
    }

    #5
    NKurzman
    A Guy on the Net
    • Total Posts : 18799
    • Reward points : 0
    • Joined: 2008/01/16 19:33:48
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 17:27:06 (permalink)
    0
    I saw that one too.
    It looks a little more elegant.
    #6
    LdB_ECM
    Super Member
    • Total Posts : 378
    • Reward points : 0
    • Joined: 2019/04/16 22:01:25
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 21:25:55 (permalink)
    +1 (1)
    It isn't a bug unless the compiler doesn't throw a warning you violate a C rule to do with uint16_t.
    uint16_t* requires 2-byte alignment

    Same for uint32_t it requires 4-byte alignment do anything else at your own peril
     
    If you have all the warnings at max it should throw a warning when you set the pointer .. that is the bug if it doesn't.
     
    If you have unaligned packs stop being cute and use the structure pointer and you will never have the issue because the compiler can work the alignment out from the struct.
    #7
    aschen0866
    Super Member
    • Total Posts : 4567
    • Reward points : 0
    • Joined: 2006/01/08 22:18:32
    • Location: San Diego
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/03 22:21:16 (permalink)
    0
    LdB_ECM
    It isn't a bug unless the compiler doesn't throw a warning you violate a C rule to do with uint16_t.

    Which C rule are you talking about? I am curious now.
    #8
    andersm
    Super Member
    • Total Posts : 2823
    • Reward points : 0
    • Joined: 2012/10/07 14:57:44
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/04 00:41:09 (permalink)
    0
    LdB_ECMIf you have all the warnings at max it should throw a warning when you set the pointer .. that is the bug if it doesn't.

    You need GCC 9 or newer to get a warning (-Waddress-of-packed-member, enabled by default). XC32 is still based on GCC 4.8.3.
    #9
    LdB_ECM
    Super Member
    • Total Posts : 378
    • Reward points : 0
    • Joined: 2019/04/16 22:01:25
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/04 10:22:40 (permalink)
    +1 (1)
    aschen0866
    Which C rule are you talking about? I am curious now.

     
    Section 6.2.8 page 48 Alignment Of Objects clause 1
    http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
    Complete object types have alignment requirements which place restrictions on the addresses at which objects of that type may be allocated. An alignment is an implementation-defined integer value representing the number of bytes between successive addresses at which a given object can be allocated. An object type imposes an alignment requirement on every object of that type: stricter alignment can be requested using the_Alignas keyword

     
    If you need the layman stupid version take the pointer MODULO it with the sizeof the type and it must be zero if it isn't then it fails 6.2.8 section 1.
     
    uint16_t alignment is 2 bytes, the pointer being requested does not have that alignment it does not use the _Alignas keyword so its behaviour is not defined.
     
    I am surprised it only got put in GCC 9 it has been in Keil and Intel compilers for as long as I can remember.
    #10
    jtemples
    عُضْوٌ جَدِيد
    • Total Posts : 11897
    • Reward points : 0
    • Joined: 2004/02/13 12:31:19
    • Location: Southern California
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/04 11:23:17 (permalink)
    +1 (1)
    It isn't a bug unless the compiler doesn't throw a warning you violate a C rule to do with uint16_t.

     
    There's nothing in the section you quote that requires a diagnostic or mentions any "rule to do with uint16_t".
    #11
    NKurzman
    A Guy on the Net
    • Total Posts : 18799
    • Reward points : 0
    • Joined: 2008/01/16 19:33:48
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/04 16:15:23 (permalink)
    0
    GCC tends to be in its own world.
    I have found their diagnostics were not very good. It doesn’t complain about a lot of things that it should. The pedantic keyword, tells you what the coders thought about the C standard.
    It appear XC8 has adopted similar diagnostics. Too bad.
    #12
    LdB_ECM
    Super Member
    • Total Posts : 378
    • Reward points : 0
    • Joined: 2019/04/16 22:01:25
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/05 01:13:00 (permalink)
    0
    jtemples
    There's nothing in the section you quote that requires a diagnostic or mentions any "rule to do with uint16_t".

    It covers EVERY TYPE of pointer no exceptions .. there is nothing special about uint16_t
    The translation of uint16_t on the pic32 is 2 bytes ERGO the the alignment must 2 bytes to meet specification as I stated above.
    There is perhaps a rare CPU around that is 7bits and uint16_t translates to 3 bytes then the alignment must be 3 bytes.
    The stupid example above fails that and crashed ... totally predictable.
    Maybe there are some compilers that by accident or CPU design get that right but the code is DORKED.
    That is why the specification is written in long form English rather than talking about particular types.
    If you are looking type x must be aligned to y it won't happen because the types are subject to compiler interpretation so you are never going to get what you seemingly want.
    post edited by LdB_ECM - 2020/04/05 01:21:47
    #13
    jtemples
    عُضْوٌ جَدِيد
    • Total Posts : 11897
    • Reward points : 0
    • Joined: 2004/02/13 12:31:19
    • Location: Southern California
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/05 13:13:02 (permalink)
    +2 (2)
    The translation of uint16_t on the pic32 is 2 bytes ERGO the the alignment must 2 bytes to meet specification as I stated above.

     
    The fact that the PIC32 has two-byte alignment for a uint16_t has nothing to do with the standard.  There is no requirement that any compiler have any particular alignment for any particular data type.  The standard you reference is the C11 draft; XC32 is a C89/some C99 compiler; _Alignas is a C11 feature.
    #14
    LdB_ECM
    Super Member
    • Total Posts : 378
    • Reward points : 0
    • Joined: 2019/04/16 22:01:25
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/05 17:47:32 (permalink)
    0
    The according to your answer it is a bug ... so the OP can open a bug request and it is obvious what the compiler is doing :-)
     
    Personally I don't give a dam I would never write that junk, as far as I am concerned I would treat it as invalid or a crash waiting for a place to happen. So I don't really care and this is now just wasting my time.
    post edited by LdB_ECM - 2020/04/05 17:52:49
    #15
    ric
    Super Member
    • Total Posts : 27716
    • Reward points : 0
    • Joined: 2003/11/07 12:41:26
    • Location: Australia, Melbourne
    • Status: online
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/05 17:52:50 (permalink)
    +2 (2)
    The great thing about standards is, there are so many to choose from...
     

    I also post at: PicForum
    Links to useful PIC information: http://picforum.ric323.co...opic.php?f=59&t=15
    NEW USERS: Posting images, links and code - workaround for restrictions.
    To get a useful answer, always state which PIC you are using!
    #16
    LdB_ECM
    Super Member
    • Total Posts : 378
    • Reward points : 0
    • Joined: 2019/04/16 22:01:25
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/05 17:55:47 (permalink)
    0
    hehe true and some code that is valid under one spec has no future going forward and will become invalid.
    #17
    andersm
    Super Member
    • Total Posts : 2823
    • Reward points : 0
    • Joined: 2012/10/07 14:57:44
    • Location: 0
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/05 21:34:42 (permalink)
    0
    Note that the _Alignas keyword can only be used to request stricter alignment.
    _Alignas(float) char c; // OK, stricter alignment
    _Alignas(char) float f; // Error

    The combined effect of all alignment attributes in a declaration shall not specify an alignment that is less strict than the alignment that would otherwise be required for the type of the object or member being declared.

    #18
    Paul PortSol
    Super Member
    • Total Posts : 620
    • Reward points : 0
    • Joined: 2015/07/03 11:52:03
    • Location: Newfoundland, Canada
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/06 08:03:00 (permalink)
    0
    Thank you all for the input.
    I would have expected a warning, but the gcc is old so that's life.
    At least I know what's happening and so have put a warning the the header file next to that special structure.
    Paul
    #19
    jdeguire
    Super Member
    • Total Posts : 567
    • Reward points : 0
    • Joined: 2012/01/13 07:48:44
    • Location: United States
    • Status: offline
    Re: XC32 Bug: Pointer accessing shifted member of packed struct 2020/04/06 10:30:15 (permalink)
    +4 (4)
    Asking for better diagnostics might be a way to convince the XC32 devs into moving up GCC versions, though that'd be a long-term investment because they have modified parts of GCC to add attributes (like "address", "coherent", and the Arm TCM ones), their "best-fit" linker, and some optimizations.  Thus moving GCC versions would not be as simple as just downloading the newest sources and building them.
     
    The MIPS compiler is GCC 4.8.3, but the Arm one is 6.2.1 and the diagnostics on that side seem notably better from what I can tell from my limited experience with the Arm devices.  At the very least, the messages are easier to understand.  I suspect that, oddly enough, we can thank Clang for that because I recall better messages being a selling point for Clang early on.
     
    Also, the Arm side should support C++14, which is nice.
    #20
    Page: 12 > Showing page 1 of 2
    Jump to:
    © 2020 APG vNext Commercial Version 4.5