linux-4.4-rc8/arch/arm64/kernel/module.c:78: 32/64 bit problem ?

[dcb314@hotmail.com (David Binderman)]

Hello there Will,

----------------------------------------
> Curious, but how are you seeing this warning? GCC is silent for me...

Static analyser called cppcheck, available from sourceforge.

GCC will be silent for all sorts of dodgy code. clang produces more warnings, 
but I am not sure how much of the linux kernel it can compile. 
Hence use of cppcheck.


Regards

David Binderman

>
>> Source code is
>>
>>     u64 imm_mask = (1 << len) - 1;
>>     s64 sval = do_reloc(op, place, val);
>>
>>     switch (len) {
>>     case 16:
>>         *(s16 *)place = sval;
>>         break;
>>     case 32:
>>         *(s32 *)place = sval;
>>         break;
>>     case 64:
>>
>> So it seems that len can be 64. Suggest new code
>>
>>     u64 imm_mask = (1UL << len) - 1;
>
> That still ends up shifting by the width of the type when len == 64,
> which is potentially still broken. We're better off using GENMASK.
>
> Will
>
> --->8
>
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index f4bc779e62e8..266e7490e85c 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -75,7 +75,7 @@ static u64 do_reloc(enum aarch64_reloc_op reloc_op, void *place, u64 val)
>
> static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
> {
> - u64 imm_mask = (1 << len) - 1;
> + u64 imm_mask = GENMASK(len - 1, 0);
> s64 sval = do_reloc(op, place, val);
>
> switch (len) {