On Mon, Jul 01, 2013 at 12:02:45PM +0200, Hans de Goede wrote: > Unaligned accesses on architectures which don't support it, either get > trapped and emulated (with a fat warnign logged somewhere), or get trapped > and terminate the program. So I don't think this may cause "subtle" bugs.
I agree that it's unlikely to be an issue in real life for libusb, but I was tricked by exactly the same problem with uIP on an older ARM microcontroller (ARM7/ARMv4T), it wasn't easy to debug at all. Triggering subtle bugs here would require running uCLinux (i.e. MMU-less) with libusb. This [1] document has an elaborate description about the matters (in particular, LDR doing a "rotated load"). [1] http://www.heyrick.co.uk/armwiki/Unaligned_data_access > Notice that actually getting the alignment wrong is pretty hard, most ABI's > guarantee some stack pointer alignment when entering a function, so having > an unaligned buffer would require declaring vars like ie this: > > unsigned char unalign[3]; > unsigned char buffer[16]; Yep, certainly. And malloc() guarantees maximum alignment. > Since you're actually getting the warnings can you provide a patch to silence > them ? Follows (against the current libusbx HEAD, but should be applicable to libusb as well), TIA From: Paul Fertser <fercer...@gmail.com> Date: Mon, 1 Jul 2013 20:47:18 +0400 Subject: [PATCH] Clarify alignment requirements for the control transfer buffer Since the buffer pointer will later be casted to ``struct libusb_control_setup *'', it should point to memory aligned to at least 2 bytes boundary as that's the strictest requirement of the struct fields. Also, use a (void *) casting trick to convince the compiler the cast is safe, to fix warnings such as: /usr/local/include/libusb-1.0/libusb.h: In function 'libusb_control_transfer_get_setup': /usr/local/include/libusb-1.0/libusb.h:1435:9: error: cast increases required alignment of target type [-Werror=cast-align] /usr/local/include/libusb-1.0/libusb.h: In function 'libusb_fill_control_setup': /usr/local/include/libusb-1.0/libusb.h:1464:39: error: cast increases required alignment of target type [-Werror=cast-align] /usr/local/include/libusb-1.0/libusb.h: In function 'libusb_fill_control_transfer': /usr/local/include/libusb-1.0/libusb.h:1509:39: error: cast increases required alignment of target type [-Werror=cast-align] cc1: all warnings being treated as errors This actually can lead to failure to build from the sources for certain projects which use -Werror=cast-align on ARM. Signed-off-by: Paul Fertser <fercer...@gmail.com> --- libusb/io.c | 2 +- libusb/libusb.h | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/libusb/io.c b/libusb/io.c index b27e082..add2328 100644 --- a/libusb/io.c +++ b/libusb/io.c @@ -731,7 +731,7 @@ void cb(struct libusb_transfer *transfer) void myfunc() { struct libusb_transfer *transfer; - unsigned char buffer[LIBUSB_CONTROL_SETUP_SIZE]; + unsigned char buffer[LIBUSB_CONTROL_SETUP_SIZE] __attribute__ ((aligned (2))); int completed = 0; transfer = libusb_alloc_transfer(0); diff --git a/libusb/libusb.h b/libusb/libusb.h index 5e1c23c..2c98f52 100644 --- a/libusb/libusb.h +++ b/libusb/libusb.h @@ -1428,7 +1428,7 @@ static inline unsigned char *libusb_control_transfer_get_data( static inline struct libusb_control_setup *libusb_control_transfer_get_setup( struct libusb_transfer *transfer) { - return (struct libusb_control_setup *) transfer->buffer; + return (struct libusb_control_setup *)(void *) transfer->buffer; } /** \ingroup asyncio @@ -1437,6 +1437,7 @@ static inline struct libusb_control_setup *libusb_control_transfer_get_setup( * be given in host-endian byte order. * * \param buffer buffer to output the setup packet into + * This pointer must be aligned to at least 2 bytes boundary. * \param bmRequestType see the * \ref libusb_control_setup::bmRequestType "bmRequestType" field of * \ref libusb_control_setup @@ -1457,7 +1458,7 @@ static inline void libusb_fill_control_setup(unsigned char *buffer, uint8_t bmRequestType, uint8_t bRequest, uint16_t wValue, uint16_t wIndex, uint16_t wLength) { - struct libusb_control_setup *setup = (struct libusb_control_setup *) buffer; + struct libusb_control_setup *setup = (struct libusb_control_setup *)(void *) buffer; setup->bmRequestType = bmRequestType; setup->bRequest = bRequest; setup->wValue = libusb_cpu_to_le16(wValue); @@ -1493,6 +1494,7 @@ void LIBUSB_CALL libusb_free_transfer(struct libusb_transfer *transfer); * \param dev_handle handle of the device that will handle the transfer * \param buffer data buffer. If provided, this function will interpret the * first 8 bytes as a setup packet and infer the transfer length from that. + * This pointer must be aligned to at least 2 bytes boundary. * \param callback callback function to be invoked on transfer completion * \param user_data user data to pass to callback function * \param timeout timeout for the transfer in milliseconds @@ -1502,7 +1504,7 @@ static inline void libusb_fill_control_transfer( unsigned char *buffer, libusb_transfer_cb_fn callback, void *user_data, unsigned int timeout) { - struct libusb_control_setup *setup = (struct libusb_control_setup *) buffer; + struct libusb_control_setup *setup = (struct libusb_control_setup *)(void *) buffer; transfer->dev_handle = dev_handle; transfer->endpoint = 0; transfer->type = LIBUSB_TRANSFER_TYPE_CONTROL; -- 1.7.9 ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
