Ignore "-Wcast-align" warnings

Use this forum for requesting small changes in ChibiOS. Large changes should be discussed in the development forum. This forum is NOT for support.
User avatar
stertingen
Posts: 21
Joined: Mon Jan 13, 2020 10:55 pm
Has thanked: 7 times
Been thanked: 7 times

Ignore "-Wcast-align" warnings

Postby stertingen » Thu Oct 29, 2020 6:16 pm

Using GCC, I have enabled the "-Wcast-align" warnings enabled.

Unfortunately, 1. the ChibiOS code performs such casting operations which increase the required alignment and 2. the ChibiOS Makefile structure does not easily allow to disable warnings for ChibiOS source files while keeping them enabled for the project source files.

I see a few ways to approach this issue:

1. Add something like '#pragma GCC diagnostic ignored "-Wcast-align"'

2. Separate project source files and ChibiOS source files in the Makefile, so the ChibiOS files are compiled without warnings

3. Try not to use such cast operations and use memcpy instead.

See also: http://blog.antiblau.de/2016/07/13/cc-d ... ast-align/

User avatar
stertingen
Posts: 21
Joined: Mon Jan 13, 2020 10:55 pm
Has thanked: 7 times
Been thanked: 7 times

Re: Ignore "-Wcast-align" warnings

Postby stertingen » Fri Nov 13, 2020 4:26 pm

Some thoughts on how to prevent unaligned memory access:

Perform pointer arithmetic on maximum alignment
Instead of

Code: Select all

uint32_t * x = (uint32_t)((uint8_t *)y + n)

do

Code: Select all

uint32_t * x = (uint32_t *)y + n / sizeof (uint32_t)

So we guarantee that x has the correct alignment. Of course we have to ensure that

Code: Select all

(n / sizeof (uint32_t)) * sizeof (uint32_t) == n


Compare pointers on minimum alignment
Instead of

Code: Select all

uint32_t * a; uint8_t * b;
a == (uint32_t *)b

do

Code: Select all

(uint8_t *)a == b

or maybe even

Code: Select all

(void *)a == (void *)b

as we do not want to dereference the pointers. However, I sometimes feel uncomfortable when using void pointers.

Use memcpy to copy data into or from a buffer at arbitrary alignment
Instead of

Code: Select all

uint8_t * buf = ...;
uint32_t x = ...;
x = *((uint32_t *)buf);
*((uint32_t *)buf) = x;

do

Code: Select all

memcpy(&x, buf, sizeof (uint32_t));
memcpy(buf, &x, sizeof (uint32_t));

because it might be possible that three or five bytes have been already written to buf, so buf might not be aligned.

I have tried to refactor some affected functions, however, they are not tested:
cast-align.zip
(2.48 KiB) Downloaded 169 times

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: Ignore "-Wcast-align" warnings

Postby Giovanni » Fri Nov 13, 2020 6:55 pm

Hi,

In general OK but the use of memcpy() would impact performance in some pretty critical places, see USB code, that code implicitly relies on pointers being correctly aligned.

Giovanni

User avatar
stertingen
Posts: 21
Joined: Mon Jan 13, 2020 10:55 pm
Has thanked: 7 times
Been thanked: 7 times

Re: Ignore "-Wcast-align" warnings

Postby stertingen » Sat Nov 14, 2020 8:59 pm

Giovanni wrote:Hi,

In general OK but the use of memcpy() would impact performance in some pretty critical places, see USB code, that code implicitly relies on pointers being correctly aligned.

Giovanni


GCC is smart enough to replace calls to memcpy() with other copy operations when appropriate with -O2. I looked through the disassembly of hal_buffers.o. While hal_buffers.c contains 9 calls to memcpy, hal_buffers.o has only 2 calls to memcpy (in ibqReadTimeout and obqWriteTimeout).
hal_buffers.zip
(63.17 KiB) Downloaded 169 times


However, I have not looked at other compilers.

I wonder if there's a way to tell the compiler "this pointer is correctly aligned" other than making it a uint32_t* (or stkalign_t* or ...). The only way I see to do this is by declaring it as uint32_t* and cast it down to uint8_t* if byte-wise operations are needed.

User avatar
stertingen
Posts: 21
Joined: Mon Jan 13, 2020 10:55 pm
Has thanked: 7 times
Been thanked: 7 times

Re: Ignore "-Wcast-align" warnings

Postby stertingen » Sun Nov 15, 2020 11:17 am

stertingen wrote:I wonder if there's a way to tell the compiler "this pointer is correctly aligned" other than making it a uint32_t* (or stkalign_t* or ...). The only way I see to do this is by declaring it as uint32_t* and cast it down to uint8_t* if byte-wise operations are needed.


Actually, there is __builtin_assume_aligned: https://gcc.gnu.org/onlinedocs/gcc-5.4. ... ltins.html

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: Ignore "-Wcast-align" warnings

Postby Giovanni » Sun Nov 15, 2020 12:07 pm

stertingen wrote:
stertingen wrote:I wonder if there's a way to tell the compiler "this pointer is correctly aligned" other than making it a uint32_t* (or stkalign_t* or ...). The only way I see to do this is by declaring it as uint32_t* and cast it down to uint8_t* if byte-wise operations are needed.


Actually, there is __builtin_assume_aligned: https://gcc.gnu.org/onlinedocs/gcc-5.4. ... ltins.html


That is GCC-specific, cannot use that in portable code.

Giovanni

User avatar
stertingen
Posts: 21
Joined: Mon Jan 13, 2020 10:55 pm
Has thanked: 7 times
Been thanked: 7 times

Re: Ignore "-Wcast-align" warnings

Postby stertingen » Sun Nov 15, 2020 2:58 pm

Giovanni wrote:
stertingen wrote:
stertingen wrote:I wonder if there's a way to tell the compiler "this pointer is correctly aligned" other than making it a uint32_t* (or stkalign_t* or ...). The only way I see to do this is by declaring it as uint32_t* and cast it down to uint8_t* if byte-wise operations are needed.


Actually, there is __builtin_assume_aligned: https://gcc.gnu.org/onlinedocs/gcc-5.4. ... ltins.html


That is GCC-specific, cannot use that in portable code.

Giovanni


Yeah, you're right. ((uint32_t *)((void *)(ptr))) does also silent the warning because of the intermediate cast to void*. __builtin_assume_aligned silences the warning because it returns void*, not because it informs the compiler about the pointer alignment. The warning still disappears for (uint32_t*)__builtin_assume_aligned(ptr, 1) although it shouldn't.

If we are sure that (1.) the pointer is correctly aligned OR (2.) the platform supports unaligned memory access, the warning could be silenced using an intermediate cast to void*.


Return to “Small Change Requests”

Who is online

Users browsing this forum: No registered users and 6 guests