On Fri, Aug 9, 2019 at 3:58 PM Joe Perches <[email protected]> wrote:
>
> On Fri, 2019-08-09 at 15:21 -0700, Nick Desaulniers wrote:
> > Hi Joe,
> > While debugging:
> > https://github.com/ClangBuiltLinux/linux/issues/619
> > we found a bunch of places where __section is not used but could be,
> > and uses a string literal when it probably should not be.
> >
> > Just a thought that maybe checkpatch.pl could warn if
> > `__attribute__((section` appeared in the added diff, and suggest
> > __section? Then further warn to not use `""` for the section name?
>
> Hmm, that makes me wonder about the existing __section uses
> _with_ a quote are actually in the proper sections.
>
> $ git grep -n -P '\b__section\s*\(\s*"'
> arch/arm64/kernel/smp_spin_table.c:22:volatile unsigned long
> __section(".mmuoff.data.read")
> arch/s390/boot/startup.c:49:static struct diag210 _diag210_tmp_dma
> __section(".dma.data");
> include/linux/compiler.h:27:
> __section("_ftrace_annotated_branch") \
> include/linux/compiler.h:63: __section("_ftrace_branch")
> \
> include/linux/compiler.h:121:#define __annotate_jump_table
> __section(".rodata..c_jump_table")
> include/linux/compiler.h:158: __section("___kentry" "+" #sym )
> \
> include/linux/compiler.h:301: static void *
> __section(".discard.addressable") __used \
> include/linux/export.h:107: static int __ksym_marker_##sym[0]
> __section(".discard.ksym") __used
> include/linux/srcutree.h:127: __section("___srcu_struct_ptrs") =
> &name
I'm going through and fixing all of these now. Thinking about sending
one treewide fix to akpm.
>
> Maybe there should also be a __section("<foo>") test too.
I think so. Some of the trickier ones are ones that use the
stringification C preprocessor operator. I need to think more about
these.
>
> Anyway, how about:
> ---
> scripts/checkpatch.pl | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 1cdacb4fd207..8e6693ca772c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5901,6 +5901,15 @@ sub process {
> "__aligned(size) is preferred over
> __attribute__((aligned(size)))\n" . $herecurr);
> }
>
> +# Check for __attribute__ section, prefer __section (without quotes)
> + if ($realfile !~ m@\binclude/uapi/@ &&
> + $line =~
> /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) {
> + my $old = substr($rawline, $-[1], $+[1] - $-[1]);
> + my $new = substr($old, 1, -1);
> + WARN("PREFER_SECTION",
> + "__section($new) is preferred over
> __attribute__((section($old)))\n" . $herecurr);
> + }
> +
I can't read Perl, but this looks pretty good.
Acked-by: Nick Desaulniers <[email protected]>
--
Thanks,
~Nick Desaulniers
