[PATCHES v4] Another round of FreeBSD developer build fixes

[Uri Simchoni]

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
    address.

    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
    this.

    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
       sockaddr_storage.

    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
    build).

    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>

Thanks,
Uri.