* Re: [PATCH] arm64/mm: fix variable 'tag' set but not used
2019-08-01 12:07 ` Qian Cai
@ 2019-08-01 12:09 ` Qian Cai
2019-08-01 12:12 ` Will Deacon
1 sibling, 0 replies; 5+ messages in thread
From: Qian Cai @ 2019-08-01 12:09 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, andreyknvl, linux-arm-kernel, linux-kernel
> On Aug 1, 2019, at 8:07 AM, Qian Cai <cai@lca.pw> wrote:
>
>
>
>> On Aug 1, 2019, at 8:01 AM, Will Deacon <will@kernel.org> wrote:
>>
>> On Wed, Jul 31, 2019 at 04:38:18PM -0400, Qian Cai wrote:
>>> When CONFIG_KASAN_SW_TAGS=n, set_tag() is compiled away. GCC throws a
>>> warning,
>>>
>>> mm/kasan/common.c: In function '__kasan_kmalloc':
>>> mm/kasan/common.c:464:5: warning: variable 'tag' set but not used
>>> [-Wunused-but-set-variable]
>>> u8 tag = 0xff;
>>> ^~~
>>>
>>> Fix it by making __tag_set() a static inline function.
>>>
>>> Signed-off-by: Qian Cai <cai@lca.pw>
>>> ---
>>> arch/arm64/include/asm/memory.h | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>>> index b7ba75809751..9645b1340afe 100644
>>> --- a/arch/arm64/include/asm/memory.h
>>> +++ b/arch/arm64/include/asm/memory.h
>>> @@ -210,7 +210,11 @@ static inline unsigned long kaslr_offset(void)
>>> #define __tag_reset(addr) untagged_addr(addr)
>>> #define __tag_get(addr) (__u8)((u64)(addr) >> 56)
>>> #else
>>> -#define __tag_set(addr, tag) (addr)
>>> +static inline const void *__tag_set(const void *addr, u8 tag)
>>> +{
>>> + return addr;
>>> +}
>>
>> Why doesn't this trigger a warning in page_to_virt(), which passes an
>> unsigned long for the address parameter?
>
> #define page_to_virt(page) … __tag_set(__addr, page_kasan_tag(page)); …
>
> static inline u8 page_kasan_tag(const struct page *page)
> {
> return 0xff;
> }
>
> GCC will see that “page” is used.
If you are talking about “addr”,
#define __tag_set(addr, tag) (addr)
It is actually used.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] arm64/mm: fix variable 'tag' set but not used
2019-08-01 12:07 ` Qian Cai
2019-08-01 12:09 ` Qian Cai
@ 2019-08-01 12:12 ` Will Deacon
1 sibling, 0 replies; 5+ messages in thread
From: Will Deacon @ 2019-08-01 12:12 UTC (permalink / raw)
To: Qian Cai; +Cc: Catalin Marinas, andreyknvl, linux-arm-kernel, linux-kernel
On Thu, Aug 01, 2019 at 08:07:12AM -0400, Qian Cai wrote:
>
>
> > On Aug 1, 2019, at 8:01 AM, Will Deacon <will@kernel.org> wrote:
> >
> > On Wed, Jul 31, 2019 at 04:38:18PM -0400, Qian Cai wrote:
> >> When CONFIG_KASAN_SW_TAGS=n, set_tag() is compiled away. GCC throws a
> >> warning,
> >>
> >> mm/kasan/common.c: In function '__kasan_kmalloc':
> >> mm/kasan/common.c:464:5: warning: variable 'tag' set but not used
> >> [-Wunused-but-set-variable]
> >> u8 tag = 0xff;
> >> ^~~
> >>
> >> Fix it by making __tag_set() a static inline function.
> >>
> >> Signed-off-by: Qian Cai <cai@lca.pw>
> >> ---
> >> arch/arm64/include/asm/memory.h | 6 +++++-
> >> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> >> index b7ba75809751..9645b1340afe 100644
> >> --- a/arch/arm64/include/asm/memory.h
> >> +++ b/arch/arm64/include/asm/memory.h
> >> @@ -210,7 +210,11 @@ static inline unsigned long kaslr_offset(void)
> >> #define __tag_reset(addr) untagged_addr(addr)
> >> #define __tag_get(addr) (__u8)((u64)(addr) >> 56)
> >> #else
> >> -#define __tag_set(addr, tag) (addr)
> >> +static inline const void *__tag_set(const void *addr, u8 tag)
> >> +{
> >> + return addr;
> >> +}
> >
> > Why doesn't this trigger a warning in page_to_virt(), which passes an
> > unsigned long for the address parameter?
>
> #define page_to_virt(page) … __tag_set(__addr, page_kasan_tag(page)); …
>
> static inline u8 page_kasan_tag(const struct page *page)
> {
> return 0xff;
> }
>
> GCC will see that “page” is used.
No, I mean because you're making addr 'const void *'.
/me finds a toolchain.
Look:
In file included from ./arch/arm64/include/asm/pgtable-hwdef.h:8,
from ./arch/arm64/include/asm/processor.h:35,
from ./include/linux/mutex.h:19,
from ./include/linux/kernfs.h:12,
from ./include/linux/sysfs.h:16,
from ./include/linux/kobject.h:20,
from ./include/linux/of.h:17,
from ./include/linux/irqdomain.h:35,
from ./include/linux/acpi.h:13,
from ./include/acpi/apei.h:9,
from ./include/acpi/ghes.h:5,
from ./include/linux/arm_sdei.h:14,
from arch/arm64/kernel/asm-offsets.c:10:
./include/linux/mm.h: In function ‘lowmem_page_address’:
./arch/arm64/include/asm/memory.h:309:14: warning: passing argument 1 of ‘__tag_set’ makes pointer from integer without a cast [-Wint-conversion]
__tag_set(__addr, page_kasan_tag(page)); \
^~~~~~
./include/linux/mm.h:1302:9: note: in expansion of macro ‘page_to_virt’
return page_to_virt(page);
^~~~~~~~~~~~
./arch/arm64/include/asm/memory.h:213:49: note: expected ‘const void *’ but argument is of type ‘long unsigned int’
static inline const void *__tag_set(const void *addr, u8 tag)
~~~~~~~~~~~~^~~~
./arch/arm64/include/asm/memory.h:309:4: warning: initialization of ‘long unsigned int’ from ‘const void *’ makes integer from pointer without a cast [-Wint-conversion]
__tag_set(__addr, page_kasan_tag(page)); \
^~~~~~~~~
./include/linux/mm.h:1302:9: note: in expansion of macro ‘page_to_virt’
return page_to_virt(page);
^~~~~~~~~~~~
If you're going to send patches removing compiler warnings, please have the
courtesy to test them first.
Will
^ permalink raw reply [flat|nested] 5+ messages in thread