2023-01-20 11:53:39

by Christian Borntraeger

[permalink] [raw]
Subject: [PATCH 1/1] checkpatch: recognise NOKPROBE_SYMBOL for blank line detection after function/struct/union/enum

NOKPROBE_SYMBOL should be treated like EXPORT_SYMBOL to avoid
false positives.

Signed-off-by: Christian Borntraeger <[email protected]>
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 78cc595b98ce..037acbcee0e1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3995,6 +3995,7 @@ sub process {
$line =~ /^\+/ &&
!($line =~ /^\+\s*$/ ||
$line =~ /^\+\s*(?:EXPORT_SYMBOL|early_param)/ ||
+ $line =~ /^\+\s*NOKPROBE_SYMBOL/ ||
$line =~ /^\+\s*MODULE_/i ||
$line =~ /^\+\s*\#\s*(?:end|elif|else)/ ||
$line =~ /^\+[a-z_]*init/ ||
--
2.38.1


2023-01-20 16:50:19

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/1] checkpatch: recognise NOKPROBE_SYMBOL for blank line detection after function/struct/union/enum

On Fri, 2023-01-20 at 12:46 +0100, Christian Borntraeger wrote:
> NOKPROBE_SYMBOL should be treated like EXPORT_SYMBOL to avoid
> false positives.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3995,6 +3995,7 @@ sub process {
> $line =~ /^\+/ &&
> !($line =~ /^\+\s*$/ ||
> $line =~ /^\+\s*(?:EXPORT_SYMBOL|early_param)/ ||
> + $line =~ /^\+\s*NOKPROBE_SYMBOL/ ||
> $line =~ /^\+\s*MODULE_/i ||
> $line =~ /^\+\s*\#\s*(?:end|elif|else)/ ||
> $line =~ /^\+[a-z_]*init/ ||

$ git grep -P -w -oh '\w*NOKPROBE_SYMBOL\w*' | sort | uniq -c
60 _ASM_NOKPROBE_SYMBOL
2 __NOKPROBE_SYMBOL
304 NOKPROBE_SYMBOL

Any issue with these other variants? (it doesn't seem so)

btw: __NOKPROBE_SYMBOL has a ; at the end of its definition.

include/asm-generic/kprobes.h:# define __NOKPROBE_SYMBOL(fname) \
include/asm-generic/kprobes.h-static unsigned long __used \
include/asm-generic/kprobes.h- __section("_kprobe_blacklist") \
include/asm-generic/kprobes.h- _kbl_addr_##fname = (unsigned long)fname;

And so there are extra ; uses after ever use of NOKPROBE_SYMBOL(foo);

$ git grep -w NOKPROBE_SYMBOL | grep ';' | wc -l
285

There are only 10 uses of NOKPROBE_SYMBOL(foo) without a semicolon

$ git grep -P '\bNOKPROBE_SYMBOL\s*\(\s*\w+\s*\)\s*(?:[^\;]|$)'
arch/arm/kernel/traps.c:NOKPROBE_SYMBOL(do_undefinstr)
arch/arm/probes/kprobes/opt-arm.c:NOKPROBE_SYMBOL(optimized_callback)
arch/arm64/kernel/kgdb.c:NOKPROBE_SYMBOL(kgdb_brk_fn)
arch/powerpc/mm/cacheflush.c:NOKPROBE_SYMBOL(flush_dcache_icache_phys)
arch/powerpc/platforms/82xx/pq2.c:NOKPROBE_SYMBOL(pq2_restart)
include/asm-generic/kprobes.h:# define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname)
include/asm-generic/kprobes.h:# define NOKPROBE_SYMBOL(fname)
kernel/fail_function.c:NOKPROBE_SYMBOL(fei_kprobe_handler)
kernel/kprobes.c:NOKPROBE_SYMBOL(__kretprobe_trampoline_handler)
kernel/trace/trace_eprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
kernel/trace/trace_kprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
kernel/trace/trace_uprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)

The actual uses with NOKPROBE_SYMBOL(foo) should likely have semicolons added
and the __NOKPROBE_SYMBOL definition should have the semicolon removed.

2023-01-24 12:16:36

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH 1/1] checkpatch: recognise NOKPROBE_SYMBOL for blank line detection after function/struct/union/enum



Am 20.01.23 um 17:10 schrieb Joe Perches:
> On Fri, 2023-01-20 at 12:46 +0100, Christian Borntraeger wrote:
>> NOKPROBE_SYMBOL should be treated like EXPORT_SYMBOL to avoid
>> false positives.
> []
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -3995,6 +3995,7 @@ sub process {
>> $line =~ /^\+/ &&
>> !($line =~ /^\+\s*$/ ||
>> $line =~ /^\+\s*(?:EXPORT_SYMBOL|early_param)/ ||
>> + $line =~ /^\+\s*NOKPROBE_SYMBOL/ ||
>> $line =~ /^\+\s*MODULE_/i ||
>> $line =~ /^\+\s*\#\s*(?:end|elif|else)/ ||
>> $line =~ /^\+[a-z_]*init/ ||
>
> $ git grep -P -w -oh '\w*NOKPROBE_SYMBOL\w*' | sort | uniq -c
> 60 _ASM_NOKPROBE_SYMBOL
> 2 __NOKPROBE_SYMBOL
> 304 NOKPROBE_SYMBOL
>
> Any issue with these other variants? (it doesn't seem so)
7
The other variants should not matter.
>
> btw: __NOKPROBE_SYMBOL has a ; at the end of its definition.
>
> include/asm-generic/kprobes.h:# define __NOKPROBE_SYMBOL(fname) \
> include/asm-generic/kprobes.h-static unsigned long __used \
> include/asm-generic/kprobes.h- __section("_kprobe_blacklist") \
> include/asm-generic/kprobes.h- _kbl_addr_##fname = (unsigned long)fname;
>
> And so there are extra ; uses after ever use of NOKPROBE_SYMBOL(foo);
>
> $ git grep -w NOKPROBE_SYMBOL | grep ';' | wc -l
> 285
>
> There are only 10 uses of NOKPROBE_SYMBOL(foo) without a semicolon
>
> $ git grep -P '\bNOKPROBE_SYMBOL\s*\(\s*\w+\s*\)\s*(?:[^\;]|$)'
> arch/arm/kernel/traps.c:NOKPROBE_SYMBOL(do_undefinstr)
> arch/arm/probes/kprobes/opt-arm.c:NOKPROBE_SYMBOL(optimized_callback)
> arch/arm64/kernel/kgdb.c:NOKPROBE_SYMBOL(kgdb_brk_fn)
> arch/powerpc/mm/cacheflush.c:NOKPROBE_SYMBOL(flush_dcache_icache_phys)
> arch/powerpc/platforms/82xx/pq2.c:NOKPROBE_SYMBOL(pq2_restart)
> include/asm-generic/kprobes.h:# define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname)
> include/asm-generic/kprobes.h:# define NOKPROBE_SYMBOL(fname)
> kernel/fail_function.c:NOKPROBE_SYMBOL(fei_kprobe_handler)
> kernel/kprobes.c:NOKPROBE_SYMBOL(__kretprobe_trampoline_handler)
> kernel/trace/trace_eprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
> kernel/trace/trace_kprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
> kernel/trace/trace_uprobe.c:NOKPROBE_SYMBOL(process_fetch_insn)
>
> The actual uses with NOKPROBE_SYMBOL(foo) should likely have semicolons added
> and the __NOKPROBE_SYMBOL definition should have the semicolon removed.

That would be an independent check I guess.