[PATCH-next] arm: fix clang build warning in include/asm/memory.h
Nathan Chancellor
nathan at kernel.org
Tue Mar 19 07:46:53 PDT 2024
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 at 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 at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
> --
> Regards,
> Yipeng Zou
>
More information about the linux-arm-kernel
mailing list