protobuf-c generated code causes alignment compile warning on mips64

SUMMARY
=======

protobuf-c generated code causes alignment compile warning on mips64

DETAILED DESCRIPTION
====================

Consider the following test.proto file

-----< START >----------------------------------------------
package test;
message test {
  required uint32 x = 1;
  required uint64 y = 2;
};
-----< END >------------------------------------------------

Consider that we generate test.pc-c.c and test.pc-c.h files as follows:

proto-c --c_out=. test.proto

Consider that we have the following test program:

-----< START >----------------------------------------------
#include <stdlib.h>
#include <stdio.h>
#include "test.pb-c.h"

int
main (int argc, char** argv)
{
    uint32_t x_val = UINT32_MAX - 111;
    uint64_t y_val = UINT64_MAX - 222;

    Test__Test test = TEST__TEST__INIT;
    test.x = x_val;
    test.y = y_val;

    /* Pack */
    const ProtobufCMessage *message = (const ProtobufCMessage *) &test;
    size_t size = protobuf_c_message_get_packed_size(message);
    void *memory = malloc(size);
    protobuf_c_message_pack(message, memory);
    printf("Pack success\n");

    /* Unpack */
    Test__Test *unpacked_test = test__test__unpack(NULL, size, memory);
    assert(unpacked_test->x == x_val);
    assert(unpacked_test->y == y_val);
    printf("Unpack success\n");

    free(memory);
    test__test__free_unpacked(unpacked_test, NULL);

    return 0;
}
-----< END >------------------------------------------------

When you compile the generated files and the test program for the MIPS64 target 
processes you get the following compile warning:

In file included from test.c:2:
test.pb-c.c: In function 'test__test__unpack':
test.pb-c.c:43: warning: cast increases required alignment of target type

The 'offending' code is caused by the typecast in the following return 
statement in test.pb-c.c:

-----< START >----------------------------------------------
Test__Test *
       test__test__unpack
                     (ProtobufCAllocator  *allocator,
                      size_t               len,
                      const uint8_t       *data)
{
  return (Test__Test *)
     protobuf_c_message_unpack (&test__test__descriptor,
                                allocator, len, data);
}
-----< END >------------------------------------------------

The Test__Test structure is 64-bit alligned because it contains an uint64_t. 
Here is the definition in test.pb-c.h:

-----< START >----------------------------------------------
typedef struct _Test__Test Test__Test;
struct  _Test__Test
{
  ProtobufCMessage base;
  uint32_t x;
  uint64_t y;
};  
-----< END >------------------------------------------------

The function protobuf_c_message_unpack returns a "ProtobufCMessage *". Here is 
the declaration in protobuf-c.h:

-----< START >----------------------------------------------
PROTOBUF_C_API ProtobufCMessage *
          protobuf_c_message_unpack         (const ProtobufCMessageDescriptor *,
                                             ProtobufCAllocator  *allocator,
                                             size_t               len,
                                             const uint8_t       *data);
-----< END >------------------------------------------------

The ProtobufCMessage structure is NOT 64-bit alligned because it does not 
contain any uint64_t. Here is the definition in protobuf-c.h:

-----< START >----------------------------------------------
typedef struct _ProtobufCMessage ProtobufCMessage;
typedef struct _ProtobufCMessageUnknownField ProtobufCMessageUnknownField;
struct _ProtobufCMessage
{
  const ProtobufCMessageDescriptor *descriptor;
  unsigned n_unknown_fields;
  ProtobufCMessageUnknownField *unknown_fields;
};
-----< END >------------------------------------------------

PROPOSED SOLUTION
=================

OPTION 1 (CHOSEN)
-----------------

To avoid this warning the ProtobufCMessage structure should be aligned 
according to the most stringent possible alignment criteria, that is: aligned 
to a 64-bit boundary.

One way to achieve this is to use the "__attribute__ aligned" compiler 
directive.

-----< START >----------------------------------------------
struct _ProtobufCMessage
{
  const ProtobufCMessageDescriptor *descriptor;
  unsigned n_unknown_fields;
  ProtobufCMessageUnknownField *unknown_fields;
} __attribute__ ((aligned (8)));
-----< END >------------------------------------------------

Even better, if you specify the aligned attribute without a value, it means 
"the largest alignment which is ever used for any data type on the target 
machine":

-----< START >----------------------------------------------
struct _ProtobufCMessage
{
  const ProtobufCMessageDescriptor *descriptor;
  unsigned n_unknown_fields;
  ProtobufCMessageUnknownField *unknown_fields;
} __attribute__ ((aligned));
-----< END >------------------------------------------------

Note: you can use "aligned (__BIGGEST_ALIGNMENT__)" instead, but that is not 
supported as widely in gcc versions.

The problem with this is that the code becomes compiler dependent and hence 
less portable.

Note that protobuf-c.h already contains various compiler-dependent macros, for 
example:

-----< START >----------------------------------------------
#if !defined(PROTOBUF_C_NO_DEPRECATED) && (__GNUC__ > 3 || (__GNUC__ == 3 && 
__GNUC_MINOR__ >= 1))
#define PROTOBUF_C_DEPRECATED __attribute__((__deprecated__))
#else
#define PROTOBUF_C_DEPRECATED
#endif 
-----< END >------------------------------------------------

In conclusion, we propose to solve the issue as follows:

-----< START >----------------------------------------------
#if !defined(PROTOBUF_C_ALIGNED) && (__GNUC__ > 2 || (__GNUC__ == 2 && 
__GNUC_MINOR__ >= 7))
#define PROTOBUF_C_ALIGNED __attribute__((aligned))
#else
#define PROTOBUF_C_ALIGNED
#endif 

struct _ProtobufCMessage
{
  const ProtobufCMessageDescriptor *descriptor;
  unsigned n_unknown_fields;
  ProtobufCMessageUnknownField *unknown_fields;
} PROTOBUF_C_ALIGNED;
-----< END >------------------------------------------------

Note: the website http://ohse.de/uwe/articles/gcc-attributes.html contains 
information about which attributes are supported in which version of gcc. It 
says that the aligned attribute is supporte in gcc 2.7 and later.

OPTION 2
--------

Another way to achieve this is to make sure that the ProtobufCMessage structure 
contains at least on uint64_t member.

-----< START >----------------------------------------------
struct _ProtobufCMessage
{
  const ProtobufCMessageDescriptor *descriptor;
  unsigned n_unknown_fields;
  ProtobufCMessageUnknownField *unknown_fields;
  uint64_t align; /*NEW*/
};
#define PROTOBUF_C_MESSAGE_INIT(descriptor) { descriptor, 0, NULL, 0 }
-----< END >------------------------------------------------

OPTION 3
--------

The problem with this is that it increases the size of the ProtobufCMessage 
structure and hence the size of each message.

Yet another way to achieve this is to use a union to add the uint64_t member 
field to the ProtobufCMessage structure. The problem with this is that it has a 
ripple effect and causes many other required changes in the code.
Original issue reported on code.google.com by brunorij...@gmail.com on 28 Dec 2011 at 8:19
shiyilei / protobuf-c

protobuf-c generated code causes alignment compile warning on mips64 #75
