pymavlink icon indicating copy to clipboard operation
pymavlink copied to clipboard

C implementation: hard fault on Cortex-M0 due to unaligned access to packed structure field

Open kevinmehall opened this issue 9 years ago • 5 comments

The mavlink_message_t structure is packed, and mavlink_finalize_message_chan and other functions take the address of checksum and other fields within the structure. This is illegal because those pointers may not have the required alignment for their type, and it results in hard-faults on Cortex-M0, which does not support unaligned loads.

I've worked around this on Cortex-M0 by removing the packing from the structure. There is no internal padding, so the only effect of packing is the alignment of the entire structure. However, mavlink_helpers.h hard-codes offsets into the structure, and assumes that the layout of the structure is the same as wire format in several places, which is also not guaranteed. The ideal fix would be to keep wire-format data in byte arrays, and native-format messages as un-packed structures, and convert between the two explicitly instead of by pointer cast.

kevinmehall avatar Sep 19 '16 19:09 kevinmehall

Another report / workaround: https://groups.google.com/forum/#!topic/mavlink/DrN9LasXumA

kevinmehall avatar Sep 19 '16 19:09 kevinmehall

interesting. The addition of the aligned(4) does look like a simple solution. Have you looked at any regressions that may cause?

tridge avatar Sep 19 '16 22:09 tridge

That happens to work in this case because the checksum field in question is the first field and only field that should require alignment. Interestingly, without packed, the struct has 8-byte alignment because of the uint64_t payload64[]. I think taking the address of payload64 in the _MAV_PAYLOAD macro may still technically be undefined behavior with aligned(4) since it creates a misaligned pointer, but since packed isn't part of the standard, it's unclear how it interacts with the alignment rules.

kevinmehall avatar Sep 19 '16 23:09 kevinmehall

I believe this issue is now coming back as a warning in GCC 9:

mavlink_msg_attitude_quaternion_cov.h:147:144: error: taking address of packed member of ‘__mavlink_attitude_quaternion_cov_t’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
  147 |     return mavlink_msg_attitude_quaternion_cov_pack(system_id, component_id, msg, attitude_quaternion_cov->time_usec, attitude_quaternion_cov->q, attitude_quaternion_cov->rollspeed, attitude_quaternion_cov->pitchspeed, attitude_quaternion_cov->yawspeed, attitude_quaternion_cov->covariance);
      |                                                                                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~^
compilation terminated due to -Wfatal-errors.
cc1plus: all warnings being treated as errors

julianoes avatar May 14 '19 14:05 julianoes

Was this issue resolved? I'm getting what I think is the same issue when including mavlink.h in a custom PX4 driver.

/home/user/git/p100_px4/build/cubepilot_cubeorangeplus_default/mavlink/common/../protocol.h: In function 'uint16_t _MAV_RETURN_uint16_t(const mavlink_message_t*, uint8_t)':
/home/user/git/p100_px4/build/cubepilot_cubeorangeplus_default/mavlink/common/../protocol.h:271:11: error: cast from 'const char*' to 'const uint16_t*' {aka 'const short unsigned int*'} increases required alignment of target type [-Werror=cast-align]
  271 | { return *(const TYPE *)(&_MAV_PAYLOAD(msg)[ofs]);}
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/user/git/p100_px4/build/cubepilot_cubeorangeplus_default/mavlink/common/../protocol.h:273:1: note: in expansion of macro '_MAV_MSG_RETURN_TYPE'
  273 | _MAV_MSG_RETURN_TYPE(uint16_t)

KnightHawk06 avatar May 28 '25 18:05 KnightHawk06