Re: [PATCH BlueZ 06/12] tools: Fix unaligned memory access on smp-tester

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johan,

On Mon, Jan 6, 2014 at 3:25 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
> This code was essentially a direct copy of what the kernel side SMP
> implementation does. Does this mean we have a problem on the kernel side
> too? Could you send a patch for that (after confirming that we do have a
> problem there too).

Checking the kernel compilation parameters, I see -Wcast-align (which
enables these warnings both on gcc and clang) is only passed if you
compile the kernel with W=2 (i.e. "make W=2 ..."). But it then enables
a ton of other warnings. I can see why they don't enable these
warnings by default.

So I did a small experiment to confirm that the code snippet you
copied from the kernel will give the same warnings with -Wcast-align
when building the kernel. I first compiled bluetooth.ko using the GCC
on my x86-64 system:

rm -rf build_test
mkdir build_test
make O=build_test allmodconfig modules_prepare
make O=build_test W=2 warning-2=-Wcast-align net/bluetooth/bluetooth.ko

This gave no warnings (maybe because x86 has no such alignment
issues?). I then downloaded the latest codesourcery ARM crosscompiler
and built bluetooth.ko for ARM with -Wcast-align (adding "ARCH=arm
CROSS_COMPILE=/opt/arm-2013.11/bin/arm-none-linux-gnueabi-" to the two
make commands). It gives 1.2MB of warnings, amongst them the ones
related to the copied code:


/home/lizardo/trees/linux.git/net/bluetooth/smp.c: In function 'smp_c1':
/home/lizardo/trees/linux.git/net/bluetooth/smp.c:103:11: warning:
cast increases required alignment of target type [-Wcast-align]
  u128_xor((u128 *) res, (u128 *) r, (u128 *) p1);
           ^
/home/lizardo/trees/linux.git/net/bluetooth/smp.c:103:25: warning:
cast increases required alignment of target type [-Wcast-align]
  u128_xor((u128 *) res, (u128 *) r, (u128 *) p1);
                         ^
/home/lizardo/trees/linux.git/net/bluetooth/smp.c:103:37: warning:
cast increases required alignment of target type [-Wcast-align]
  u128_xor((u128 *) res, (u128 *) r, (u128 *) p1);
                                     ^
/home/lizardo/trees/linux.git/net/bluetooth/smp.c:113:11: warning:
cast increases required alignment of target type [-Wcast-align]
  u128_xor((u128 *) res, (u128 *) res, (u128 *) p2);
           ^
/home/lizardo/trees/linux.git/net/bluetooth/smp.c:113:25: warning:
cast increases required alignment of target type [-Wcast-align]
  u128_xor((u128 *) res, (u128 *) res, (u128 *) p2);
                         ^
/home/lizardo/trees/linux.git/net/bluetooth/smp.c:113:39: warning:
cast increases required alignment of target type [-Wcast-align]
  u128_xor((u128 *) res, (u128 *) res, (u128 *) p2);
                                       ^

But note that there are lots of similar warnings on other kernel code,
specially from common headers. So I can only conclude that, at least
for kernel code, having these alignment requirements increased due to
cast is not considered an issue. So my suggestion is to keep the
kernel code as is, unless there is some indication that this may cause
problems.

On the other hand, on BlueZ we have traditionally taken care of fixing
these warnings (specially when people try to build for ARM and get
errors due to -Werror), so It's a good thing that clang detects it
even when building for x86. Given that it was the only place I got
this warning on BlueZ, it seemed fine to fix it like we did in the
past.

PS: I attached the compressed build log (when building only
bluetooth.ko for ARM with -Wcast-align). Unfortunately, it is too
polluted to attempt to analyze manually.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Attachment: build.log.xz
Description: application/xz


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux