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

[Yipeng Zou]

在 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.

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