Re: [PATCH-next] arm: fix clang build warning in include/asm/memory.h

[Nathan Chancellor <nathan@kernel.org>]

On Tue, Mar 19, 2024 at 11:38:02AM +0800, Yipeng Zou wrote:
> 
> 在 2024/3/19 11:16, Yipeng Zou 写道:
> > 
> > 在 2024/3/15 8:43, Nathan Chancellor 写道:
> > > On Thu, Mar 14, 2024 at 03:54:09PM +0800, Yipeng Zou wrote:
> > > > There is a build error has been founded with build in clang-15.0.4:
> > > > 
> > > > ./arch/arm/include/asm/memory.h:358:12: error: result of
> > > > comparison "phys addr_t’ (aka 'unsigned int’) > 4294967295 is
> > > > always false [-Werror, -Wtautological-type-limit-compare]
> > > >                               if (addr > (u32)~0)
> > > >                                   ~~~~ ^ ~~~~~~~
> > > > 
> > > > It will be always goes fail without CONFIG_PHYS_ADDR_T_64BIT.
> > > > 
> > > > Directly silence it by Use CONFIG_PHYS_ADDR_T_64BIT.
> > > > 
> > > > Fixes: 981b6714dbd2 ("ARM: provide improved virt_to_idmap()
> > > > functionality")
> > > > Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
> > > > ---
> > > >   arch/arm/include/asm/memory.h | 4 ++++
> > > >   1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/include/asm/memory.h
> > > > b/arch/arm/include/asm/memory.h
> > > > index ef2aa79ece5a..64ced9eb42ff 100644
> > > > --- a/arch/arm/include/asm/memory.h
> > > > +++ b/arch/arm/include/asm/memory.h
> > > > @@ -347,14 +347,18 @@ static inline bool arm_has_idmap_alias(void)
> > > >       return IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset != 0;
> > > >   }
> > > >   +#ifdef CONFIG_PHYS_ADDR_T_64BIT
> > > >   #define IDMAP_INVALID_ADDR ((u32)~0)
> > > > +#endif
> > > >     static inline unsigned long phys_to_idmap(phys_addr_t addr)
> > > >   {
> > > >       if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) {
> > > >           addr += arch_phys_to_idmap_offset;
> > > > +#ifdef CONFIG_PHYS_ADDR_T_64BIT
> > > >           if (addr > (u32)~0)
> > > >               addr = IDMAP_INVALID_ADDR;
> > > > +#endif
> > > >       }
> > > >       return addr;
> > > >   }
> > > > -- 
> > > > 2.34.1
> > > > 
> > > Would it be better to avoid the ifdef's here and just use IS_ENABLED()?
> > > I can't see why this would not work for avoiding that warning.
> > > 
> > > diff --git a/arch/arm/include/asm/memory.h
> > > b/arch/arm/include/asm/memory.h
> > > index ef2aa79ece5a..fbff07bc3b28 100644
> > > --- a/arch/arm/include/asm/memory.h
> > > +++ b/arch/arm/include/asm/memory.h
> > > @@ -353,7 +353,7 @@ static inline unsigned long
> > > phys_to_idmap(phys_addr_t addr)
> > >   {
> > >       if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) {
> > >           addr += arch_phys_to_idmap_offset;
> > > -        if (addr > (u32)~0)
> > > +        if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && addr > (u32)~0)
> > >               addr = IDMAP_INVALID_ADDR;
> > >       }
> > >       return addr;
> > 
> > It absolutely works for it and yes, It's a better way to avoiding that.
> > 
> > Will fix it in that way.
> > 
> But I notice that, this way can not silence it when compile, it only can
> avoid it in running time.

Yeah, I actually tested my suggestion and saw the same thing. Sometimes
diagnostics will be hidden by '0 &&' but it seems like that is not the
case here. It is not the end of the world, I think that diff looks fine
for the issue at hand.

> So, maybe we still need to silence it in ifdef's.
> 
> Other than that, IDMAP_INVALID_ADDR was used in other place, need to keep it
> defined.
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 158ad13e9f6a..7143147cd7cc 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -351,8 +351,10 @@ static inline unsigned long phys_to_idmap(phys_addr_t
> addr)
>  {
>         if (IS_ENABLED(CONFIG_MMU) && arch_phys_to_idmap_offset) {
>                 addr += arch_phys_to_idmap_offset;
> +#ifdef CONFIG_PHYS_ADDR_T_64BIT
>                 if (addr > (u32)~0)
>                         addr = IDMAP_INVALID_ADDR;
> +#endif
>         }
>         return addr;
>  }
> 
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> -- 
> Regards,
> Yipeng Zou
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel