linux-kernel - Re: [PATCH] mm/vmstat: Fix -Wenum-enum-conversion warning in vmstat.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241008005136.GA241099@thelio-3990X>
Date: Mon, 7 Oct 2024 17:51:36 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Aleksei Vetrov <vvvvvv@...gle.com>
Cc: Shivamurthy Shastri <shivamurthy.shastri@...utronix.de>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	akpm@...ux-foundation.org, vbabka@...e.cz, willy@...radead.org,
	hannes@...xchg.org, anna-maria@...utronix.de, tglx@...utronix.de,
	arnd@...db.de, llvm@...ts.linux.dev
Subject: Re: [PATCH] mm/vmstat: Fix -Wenum-enum-conversion warning in vmstat.h

Hi Aleksei,

On Mon, Oct 07, 2024 at 08:13:41PM +0000, Aleksei Vetrov wrote:
> Hi Shivamurthy,
> 
> On Fri, Jun 21, 2024 at 01:16:04PM +0200, Shivamurthy Shastri wrote:
> > A W=1 build with -Wenum-enum-conversion enabled, results in the
> > following build warning due to an arithmetic operation between different
> > enumeration types 'enum node_stat_item' and 'enum lru_list':
> > 
> >   include/linux/vmstat.h:514:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
> >     514 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
> >         |                               ~~~~~~~~~~~ ^ ~~~
> > 
> > Address this by casting lru to the proper type.
> > 
> > Signed-off-by: Shivamurthy Shastri <shivamurthy.shastri@...utronix.de>
> > Reviewed-by: Anna-Maria Behnsen <anna-maria@...utronix.de>
> > ---
> >  include/linux/vmstat.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> > index 735eae6e272c..72ecd46fd0c4 100644
> > --- a/include/linux/vmstat.h
> > +++ b/include/linux/vmstat.h
> > @@ -511,7 +511,7 @@ static inline const char *node_stat_name(enum node_stat_item item)
> >  
> >  static inline const char *lru_list_name(enum lru_list lru)
> >  {
> > -	return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
> > +	return node_stat_name(NR_LRU_BASE + (enum node_stat_item)lru) + 3; // skip "nr_"
> >  }
> >  
> >  static inline const char *writeback_stat_name(enum writeback_stat_item item)
> > -- 
> > 2.34.1
> > 
> 
> We have encountered the same problem after trying to update Clang to the
> latest version and this is a blocker because we use W=1 to compile the
> kernel.
> 
> Do you plan to address review comments about casting to int instead of
> enum node_stat_item? Or I can submit another patch myself that addresses
> it.

For what it's worth, I never really saw Matthew's comment around what
value does this warning provide addressed. I was the one who originally
moved it into W=1 at the request of Arnd because he felt that instances
of this warning could be bugs and they should be audited.  However, I
have not seen many instances of this warning pop up in new code through
0day build reports and the ones that I have seen seem to be intentional,
as they are using enums like integral values, such as here. If that is
just going to result in a bunch of patches like this adding unnecessary
casts, I think it would just be better to consider disabling this
warning altogether or at the very least, moving it to W=2 (which is for
warnings that are noisy but might contain bugs), since more people are
using W=1 as their normal build configuration nowadays.

Cheers,
Nathan