[PATCHES v4] Another round of FreeBSD developer build fixes
uri at samba.org
Fri Nov 24 22:37:28 UTC 2017
On 11/24/2017 02:09 AM, Andrew Bartlett via samba-technical wrote:
> I don't really like
> [PATCH v4 01/13] s4-cli: increment once in the for loop
What do you have in mind? I agree with the compile that incrementing
both in the for-loop and inside the loop is not very good - people
looking at the code might miss the increment. Shall we transform it into
a while loop?
> I would like some broader comment on:
> [PATCH v4 08/13] build: remove -Wcast-align from developer build
That commit message was already considerably longer than the average
commit message. Following is a broader commit message, please tell if
this is closer to what you had in mind.
build: remove -Wcast-align from developer build
-Wcast-align is a warning that warns against casting to a pointer
with greater alignment. Prior to this patch it was enabled in
picky developer builds, and this patch disables it.
On some CPU architectures (e.g. ARM, MIPS), a load/store instruction
is required to be from an address whose alignment matches the
load/store size, e.g. a 32-bit load has to access a 32-bit aligned
Compilers generally emit code that is guaranteed to comply with this
requirement. As part of keeping that guarantee, each data type has
an alignment associated with it, where a variable of that type is
placed at an address which is a multiple of that alignment.
Now, suppose a pointer to type A, with alignment of 2, is cast to a
pointer to B, whose alignment is 4. Suddenly we lose the guarantee
that our pointer is properly aligned, which can cause the program to
crash on some architectures. The -Wcast-align warning warns against
That would suggest that -Wcast-align is a good idea.
However, casting to a pointer of greater alignment is not
necessarily a bug. The pointer might be known to possess the desired
alignment. For example:
1. The original pointer got its value by casting from a greater
alignment. That is, we cast B* to A* and back to B*. An example
is casting from sockaddr * to sockaddr_in * - we know the
sockaddr pointer really points to a sockaddr_in or a
2. A pointer is known to point to a member of a B structure
with greater alignment, then calculating the address of the B*
from the A* is legal, although it involves a cast from A* to
B* (this BTW has a workaround of casting to void * somewhere
along the way - see container_of macro in the Linux kernel).
The Samba code base has some correct code that triggers the
-Wcast-align warning if enabled. The reason we don't get it all the
time is that we usually run developer builds with GCC and on
x86/x86_64 architectures. Those architectures don't have alignment
requirements, and GCC chooses not to emit a warning in this case.
That makes the -Wcast-align a no-op on Samba's autobuild.
OTOH, clang does emit warnings (turned to errors in developer
So, this warning buys us nothing with x86_64+GCC, and breaks the
build with clang (and probably on ARM+GCC in developer mode).
In order to enable this warning properly, we need to map the valid
uses and disable it there (e.g. there are some C files that
manipulate sockets, disable the warning on those files. Also have a
container_of macro that doesn't trigger the warning. Until this is
done, in order to allow building developer build with clang, this
warning is not enabled.
Signed-off-by: Uri Simchoni <uri at samba.org>
More information about the samba-technical