iamandi / nanopb

Automatically exported from code.google.com/p/nanopb
zlib License
0 stars 0 forks source link

Processor goes to data abort vector when accessing the ptr field in the struct pb_field_t because it's unaligned due to struct packing #136

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Compile the project using the arm-none-eabi (I compiled it for a Cortex A9 
processor: Freescale iMX6)
2. Decode a packet so it tries to access the ptr field

What is the expected output? What do you see instead?
It must decode the packet, but instead the processor goes to the data abort 
interrupt vector because of an unaligned access.

What version of the product are you using? On what operating system?
NanoPB is compiled on Ubuntu 14.04 using the gcc arm-none-eabi compilers. I'm 
using nanopb 0.3.1

Please provide any additional information below.

I've read the comments in pb.h and it says struct packing is not necessary. 
However, there is no way to disable it (I think). I have attached a patch file 
which adds a check if PB_PACK_STRUCTS is defined. Only when this define is 
available, it will enable struct packing. This way we can add compile 
parameters and enable or disable struct packing depending if you prefer speed 
access, memory consumption or if the processor can't access unaligned data.

Another way to solve this problem is to move the ptr field in front of the 
struct so the struct is as follows:

PB_PACKED_STRUCT_START
typedef struct pb_field_s pb_field_t;
struct pb_field_s {
    /* Field definitions for submessage
     * OR default value for all other non-array, non-callback types
     * If null, then field will zeroed. */
    const void *ptr;

    pb_size_t tag;
    pb_type_t type;
    pb_size_t data_offset; /* Offset of field data, relative to previous field. */
    pb_ssize_t size_offset; /* Offset of array size or has-boolean, relative to data */
    pb_size_t data_size; /* Data size in bytes for a single item */
    pb_size_t array_size; /* Maximum number of entries in array */
} pb_packed;
PB_PACKED_STRUCT_END

However I think it doesn't always work. Think of a packed struct that uses two 
of these structs, then the second struct is again badly aligned:

PB_PACKED_STRUCT_START
struct exmaple {
    pb_field_t field1;
    pb_field_t field2;
} pb_packed;
PB_PACKED_STRUCT_END

So I prefer the solution where you can enable/disable the struct packing. 
However, moving the ptr field to the front also helps reducing memory usage a 
bit when structs aren't packed, but it needs a lot of more changes.

Original issue reported on code.google.com by philip.l...@gmail.com on 9 Dec 2014 at 9:42

Attachments:

GoogleCodeExporter commented 9 years ago
Hmm, usually GCC inserts extra instructions to access unaligned data when 
__attribute__((packed)) is used.

But I will add a compilation option. Not sure about rearranging fields, it is 
more logical in current order.

Original comment by Petteri.Aimonen on 9 Dec 2014 at 3:54

GoogleCodeExporter commented 9 years ago
This issue was updated by revision cfc517f36b32.

Original comment by Petteri.Aimonen on 22 Dec 2014 at 6:53

GoogleCodeExporter commented 9 years ago
Added compilation option "PB_NO_PACKED_STRUCTS". Default is off, because 
suddenly increasing RAM usage would cause trouble for existing applications.

Original comment by Petteri.Aimonen on 22 Dec 2014 at 6:54

GoogleCodeExporter commented 9 years ago
Fix released in nanopb-0.3.2.

Original comment by Petteri.Aimonen on 24 Jan 2015 at 3:53