Sparse warnings on GENMASK + arm32

Sparse warnings on GENMASK + arm32

[Stephen Boyd]

I see sparse warning when I check a clk driver file in the kernel
on a 32-bit ARM build.

drivers/clk/sunxi/clk-sun6i-ar100.c:65:20: warning: cast truncates bits from constant value (3ffffffff becomes ffffffff)

The code in question looks like:

static const struct factors_data sun6i_ar100_data = {
	.mux = 16,
	.muxmask = GENMASK(1, 0),
	.table = &sun6i_ar100_config,
	.getter = sun6i_get_ar100_factors,
};

where factors_data is

struct factors_data {
	int enable;
	int mux;
	int muxmask;
	const struct clk_factors_config *table;
	void (*getter)(struct factors_request *req);
	void (*recalc)(struct factors_request *req);
	const char *name;
};


and sparse seems to be complaining about the muxmask assignment
here. Oddly, this doesn't happen on arm64 builds. Both times, I'm
checking this on an x86-64 machine.

 $ sparse --version
 v0.5.1-rc4-1-gfa71b7ac0594

Is there something confusing to sparse in the GENMASK macro?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] arm: fix sparse flags for build on 64bit machines

[Luc Van Oostenryck]

By default sparse uses the characteristics of the build
machine to infer things like the wordsize.
This is fine when doing native builds but for ARM it's,
I suspect, very rarely the case and if the build are done
on a 64bit machine we get a bunch of warnings like:
  'cast truncates bits from constant value (... becomes ...)'

Fix this by adding the -m32 flags for sparse.

Reported-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 arch/arm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 65f4e2a4e..121fd65f8 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -131,7 +131,7 @@ endif
 KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(CFLAGS_ISA) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
 KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_ISA) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
 
-CHECKFLAGS	+= -D__arm__
+CHECKFLAGS	+= -D__arm__ -m32
 
 #Default value
 head-y		:= arch/arm/kernel/head$(MMUEXT).o
-- 
2.13.2

Re: Sparse warnings on GENMASK + arm32

[Lance Richardson]

> From: "Stephen Boyd" <sboyd@codeaurora.org>
> To: linux-sparse@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Sent: Tuesday, 25 July, 2017 9:30:20 PM
> Subject: Sparse warnings on GENMASK + arm32
> 
> I see sparse warning when I check a clk driver file in the kernel
> on a 32-bit ARM build.
> 
> drivers/clk/sunxi/clk-sun6i-ar100.c:65:20: warning: cast truncates bits from
> constant value (3ffffffff becomes ffffffff)
> 
> The code in question looks like:
> 
> static const struct factors_data sun6i_ar100_data = {
> 	.mux = 16,
> 	.muxmask = GENMASK(1, 0),
> 	.table = &sun6i_ar100_config,
> 	.getter = sun6i_get_ar100_factors,
> };
> 
> where factors_data is
> 
> struct factors_data {
> 	int enable;
> 	int mux;
> 	int muxmask;
> 	const struct clk_factors_config *table;
> 	void (*getter)(struct factors_request *req);
> 	void (*recalc)(struct factors_request *req);
> 	const char *name;
> };
> 
> 
> and sparse seems to be complaining about the muxmask assignment
> here. Oddly, this doesn't happen on arm64 builds. Both times, I'm
> checking this on an x86-64 machine.
> 
>  $ sparse --version
>  v0.5.1-rc4-1-gfa71b7ac0594
> 
> Is there something confusing to sparse in the GENMASK macro?
> 

Hmm, it seems sparse is incorrectly taking ~0UL to be a 64-bit value
while BITS_PER_LONG is (correctly) evaluated to be 32.

#define GENMASK(h, l) \
	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Sparse warnings on GENMASK + arm32

[Luc Van Oostenryck]

On Wed, Jul 26, 2017 at 09:33:01AM -0400, Lance Richardson wrote:
> > From: "Stephen Boyd" <sboyd@codeaurora.org>
> > I see sparse warning when I check a clk driver file in the kernel
> > on a 32-bit ARM build.
> > 
> > drivers/clk/sunxi/clk-sun6i-ar100.c:65:20: warning: cast truncates bits from
> > constant value (3ffffffff becomes ffffffff)
> 
> Hmm, it seems sparse is incorrectly taking ~0UL to be a 64-bit value
> while BITS_PER_LONG is (correctly) evaluated to be 32.
> 
> #define GENMASK(h, l) \
> 	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

It's the kernel CHECKFLAGS that should be using -m32/-m64 if built
on a machine with a different wordsize tht the arch.

I sent earlier a patch for ARM, I just forgot to CC the mailing list here.

-- Luc

Re: Sparse warnings on GENMASK + arm32

[Christopher Li]

On Wed, Jul 26, 2017 at 9:33 AM, Lance Richardson <lrichard@redhat.com> wrote:
> Hmm, it seems sparse is incorrectly taking ~0UL to be a 64-bit value
> while BITS_PER_LONG is (correctly) evaluated to be 32.
>
> #define GENMASK(h, l) \
>         (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
>
What is the sizeof(unsigned long) in ARM 32 bit world?

~0UL has the type of "unsigned long", I assume BITS_PER_LONG
is just plain "int"? Using sparse -E should be able to get the expression
after the macro expression.

The kernel compile invoke sparse directly. That is the assumption
that the host gcc has the same type size as the target gcc.
That is no longer true if you have cross compiler.

If you want to have sparse understand the proper architecture difference,
the current practices is using cgcc to handle the architecture specific
macros.

you can try to invoke the kernel building with: CHECK="cgcc -no-compile".
Warning: I haven't try that myself, it might not work as expected.

In the long run, I do wish sparse can implement the proper handling of
the architecture specific stuff by itself without go through of cgcc.

Chris

[PATCH] arm: fix sparse flags for build on 64bit machines

[Stephen Boyd]

On 07/26/2017 01:43 AM, Luc Van Oostenryck wrote:
> By default sparse uses the characteristics of the build
> machine to infer things like the wordsize.
> This is fine when doing native builds but for ARM it's,
> I suspect, very rarely the case and if the build are done
> on a 64bit machine we get a bunch of warnings like:
>   'cast truncates bits from constant value (... becomes ...)'
>
> Fix this by adding the -m32 flags for sparse.
>
> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Tested-by: Stephen Boyd <sboyd@codeaurora.org>

Thanks.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] arm: fix sparse flags for build on 64bit machines

[Stephen Boyd]

On 07/26/2017 11:26 AM, Stephen Boyd wrote:
> On 07/26/2017 01:43 AM, Luc Van Oostenryck wrote:
>> By default sparse uses the characteristics of the build
>> machine to infer things like the wordsize.
>> This is fine when doing native builds but for ARM it's,
>> I suspect, very rarely the case and if the build are done
>> on a 64bit machine we get a bunch of warnings like:
>>   'cast truncates bits from constant value (... becomes ...)'
>>
>> Fix this by adding the -m32 flags for sparse.
>>
>> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
>> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> Tested-by: Stephen Boyd <sboyd@codeaurora.org>
>
>

Russell, is this good to put in the patch tracker?


Luc, can you submit this via Russell's patch tracking system (see
http://www.arm.linux.org.uk/developer/patches/info.php)?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] arm: fix sparse flags for build on 64bit machines

[Stephen Boyd]

On 08/07, Stephen Boyd wrote:
> On 07/26/2017 11:26 AM, Stephen Boyd wrote:
> > On 07/26/2017 01:43 AM, Luc Van Oostenryck wrote:
> >> By default sparse uses the characteristics of the build
> >> machine to infer things like the wordsize.
> >> This is fine when doing native builds but for ARM it's,
> >> I suspect, very rarely the case and if the build are done
> >> on a 64bit machine we get a bunch of warnings like:
> >>   'cast truncates bits from constant value (... becomes ...)'
> >>
> >> Fix this by adding the -m32 flags for sparse.
> >>
> >> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
> >> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> > Tested-by: Stephen Boyd <sboyd@codeaurora.org>
> >
> >
> 
> Russell, is this good to put in the patch tracker?
> 
> 
> Luc, can you submit this via Russell's patch tracking system (see
> http://www.arm.linux.org.uk/developer/patches/info.php)?
> 

It's been a while, and this isn't in linux-next or the patch
tracker from what I can tell. I'll forward it along if nothing
happens in the next couple days.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] arm: fix sparse flags for build on 64bit machines

[Luc Van Oostenryck]

On Fri, Sep 29, 2017 at 8:37 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 08/07, Stephen Boyd wrote:
>> On 07/26/2017 11:26 AM, Stephen Boyd wrote:
>> > On 07/26/2017 01:43 AM, Luc Van Oostenryck wrote:
>> >> By default sparse uses the characteristics of the build
>> >> machine to infer things like the wordsize.
>> >> This is fine when doing native builds but for ARM it's,
>> >> I suspect, very rarely the case and if the build are done
>> >> on a 64bit machine we get a bunch of warnings like:
>> >>   'cast truncates bits from constant value (... becomes ...)'
>> >>
>> >> Fix this by adding the -m32 flags for sparse.
>> >>
>> >> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
>> >> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
>> > Tested-by: Stephen Boyd <sboyd@codeaurora.org>
>> >
>> >
>>
>> Russell, is this good to put in the patch tracker?
>>
>>
>> Luc, can you submit this via Russell's patch tracking system (see
>> http://www.arm.linux.org.uk/developer/patches/info.php)?
>>
>
> It's been a while, and this isn't in linux-next or the patch
> tracker from what I can tell. I'll forward it along if nothing
> happens in the next couple days.

Sorry, I had forgot about it.
It's now added as patch 8701/1 in Russell's system.

-- Luc