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

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