Received-SPF: pass (google.com: domain of linux-kernel+bounces-18750-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 6 Jan 2024 16:42:31 -0800
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: kernel test robot <lkp@intel.com>, Arnd Bergmann <arnd@arndb.de>,
	linux-sparse@vger.kernel.org
Cc: Chris Morgan <macromorgan@hotmail.com>, oe-kbuild-all@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: include/asm-generic/unaligned.h:119:16: sparse: sparse: cast
 truncates bits from constant value (aa01a0 becomes a0)
Message-ID: <ZZnzd3s2L-ZwGOlz@google.com>
References: <202401070147.gqwVulOn-lkp@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202401070147.gqwVulOn-lkp@intel.com>

On Sun, Jan 07, 2024 at 01:41:34AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   95c8a35f1c017327eab3b6a2ff5c04255737c856
> commit: 66603243f5283f7f28c795f09e7c2167233df0bd Input: add driver for Hynitron cstxxx touchscreens
> date:   1 year, 2 months ago
> config: x86_64-randconfig-x051-20230705 (https://download.01.org/0day-ci/archive/20240107/202401070147.gqwVulOn-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240107/202401070147.gqwVulOn-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202401070147.gqwVulOn-lkp@intel.com/
> 
> sparse warnings: (new ones prefixed by >>)
>    drivers/input/touchscreen/hynitron_cstxxx.c: note: in included file (through arch/x86/include/generated/asm/unaligned.h):
> >> include/asm-generic/unaligned.h:119:16: sparse: sparse: cast truncates bits from constant value (aa01a0 becomes a0)
>    include/asm-generic/unaligned.h:120:20: sparse: sparse: cast truncates bits from constant value (aa01 becomes 1)
> >> include/asm-generic/unaligned.h:119:16: sparse: sparse: cast truncates bits from constant value (ab00d0 becomes d0)
>    include/asm-generic/unaligned.h:120:20: sparse: sparse: cast truncates bits from constant value (ab00 becomes 0)
> 
> vim +119 include/asm-generic/unaligned.h
> 
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  116  
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  117  static inline void __put_unaligned_le24(const u32 val, u8 *p)
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  118  {
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08 @119  	*p++ = val;
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  120  	*p++ = val >> 8;
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  121  	*p++ = val >> 16;
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  122  }
> 803f4e1eab7a89 Arnd Bergmann 2021-05-08  123  

This is not really a kernel/driver bug, just sparse being over-eager
with truncation detection. I wonder if we could make sparse skip this
check on forced casts like this:

diff --git a/expand.c b/expand.c
index f14e7181..5487e8b3 100644
--- a/expand.c
+++ b/expand.c
@@ -96,6 +96,7 @@ static long long get_longlong(struct expression *expr)
 
 void cast_value(struct expression *expr, struct symbol *newtype, struct expression *old)
 {
+	enum expression_type cast_type = expr->type;
 	struct symbol *oldtype = old->ctype;
 	int old_size = oldtype->bit_size;
 	int new_size = newtype->bit_size;
@@ -133,7 +134,7 @@ Int:
 	expr->value = value & mask;
 
 	// Stop here unless checking for truncation
-	if (!Wcast_truncate || conservative)
+	if (cast_type == EXPR_FORCE_CAST || !Wcast_truncate || conservative)
 		return;
 	
 	// Check if we dropped any bits..

and then in the kernel we would do this:

diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 699650f81970..034237d12d70 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -104,9 +104,9 @@ static inline u32 get_unaligned_le24(const void *p)
 
 static inline void __put_unaligned_be24(const u32 val, u8 *p)
 {
-	*p++ = val >> 16;
-	*p++ = val >> 8;
-	*p++ = val;
+	*p++ = (__force u8)(val >> 16);
+	*p++ = (__force u8)(val >> 8);
+	*p++ = (__force u8)val;
 }
 
 static inline void put_unaligned_be24(const u32 val, void *p)
@@ -116,9 +116,9 @@ static inline void put_unaligned_be24(const u32 val, void *p)
 
 static inline void __put_unaligned_le24(const u32 val, u8 *p)
 {
-	*p++ = val;
-	*p++ = val >> 8;
-	*p++ = val >> 16;
+	*p++ = (__force u8)val;
+	*p++ = (__force u8)(val >> 8);
+	*p++ = (__force u8)(val >> 16);
 }
 
 static inline void put_unaligned_le24(const u32 val, void *p)
@@ -128,12 +128,12 @@ static inline void put_unaligned_le24(const u32 val, void *p)
 
 static inline void __put_unaligned_be48(const u64 val, u8 *p)
 {
-	*p++ = val >> 40;
-	*p++ = val >> 32;
-	*p++ = val >> 24;
-	*p++ = val >> 16;
-	*p++ = val >> 8;
-	*p++ = val;
+	*p++ = (__force u8)(val >> 40);
+	*p++ = (__force u8)(val >> 32);
+	*p++ = (__force u8)(val >> 24);
+	*p++ = (__force u8)(val >> 16);
+	*p++ = (__force u8)(val >> 8);
+	*p++ = (__force u8)val;
 }
 
 static inline void put_unaligned_be48(const u64 val, void *p)

What do you think?

Thanks.

-- 
Dmitry