| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABJPP5AoeDvRRe0WGReHTXqgzSe1=3_iz=jtpRiHp1BiMc6=kw@mail.gmail.com>
Date: Sun, 25 Oct 2020 01:50:11 +0530
From: Dwaipayan Ray <dwaipayanray1@...il.com>
To: Joe Perches <joe@...ches.com>
Cc: linux-kernel-mentees@...ts.linuxfoundation.org,
linux-kernel <linux-kernel@...r.kernel.org>,
Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH v3] checkpatch: extend attributes check to handle more patterns
On Sat, Oct 24, 2020 at 2:36 PM Dwaipayan Ray <dwaipayanray1@...il.com> wrote:
>
> It is generally preferred that the macros from
> include/linux/compiler_attributes.h are used, unless there
> is a reason not to.
>
> checkpatch currently checks __attribute__ for each of
> packed, aligned, printf, scanf, and weak. Other declarations
> in compiler_attributes.h are not handled.
>
> Add a generic test to check the presence of such attributes.
> Some attributes require more specific handling and are kept
> separate.
>
> New attributes which are now handled are:
>
> __alias__(#symbol)
> __always_inline__
> __assume_aligned__(a, ## __VA_ARGS__)
> __cold__
> __const__
> __copy__(symbol)
> __designated_init__
> __externally_visible__
> __gnu_inline__
> __malloc__
> __mode__(x)
> __no_caller_saved_registers__
> __noclone__
> __fallthrough__
> __noinline__
> __nonstring__
> __noreturn__
> __pure__
> __unused__
> __used__
>
> Link: https://lore.kernel.org/linux-kernel-mentees/3ec15b41754b01666d94b76ce51b9832c2dd577a.camel@perches.com/
> Suggested-by: Joe Perches <joe@...ches.com>
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@...il.com>
> ---
> scripts/checkpatch.pl | 105 +++++++++++++++++++++++++++---------------
> 1 file changed, 69 insertions(+), 36 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..01d83d345b4c 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -6155,50 +6155,83 @@ sub process {
> }
> }
>
> -# Check for __attribute__ packed, prefer __packed
> +# Check for compiler attributes
> if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) {
> - WARN("PREFER_PACKED",
> - "__packed is preferred over __attribute__((packed))\n" . $herecurr);
> - }
> + $line =~ /__attribute__\s*\(\s*($balanced_parens)\s*\)/) {
> + my $attr = $1;
> + $attr =~ s/\s*\(\s*(.*)\)\s*/$1/;
> +
> + my %attr_list = (
> + "alias" => "__alias",
> + "aligned" => "__aligned",
> + "always_inline" => "__always_inline",
> + "assume_aligned" => "__assume_aligned",
> + "cold" => "__cold",
> + "const" => "__const",
> + "copy" => "__copy",
> + "designated_init" => "__designated_init",
> + "externally_visible" => "__visible",
> + "fallthrough" => "fallthrough",
> + "gnu_inline" => "__gnu_inline",
> + "malloc" => "__malloc",
> + "mode" => "__mode",
> + "no_caller_saved_registers" => "__no_caller_saved_registers",
> + "noclone" => "__noclone",
> + "noinline" => "noinline",
> + "nonstring" => "__nonstring",
> + "noreturn" => "__noreturn",
> + "packed" => "__packed",
> + "pure" => "__pure",
> + "used" => "__used"
> + );
> +
> + if ($attr =~ /^(\w+)\s*(${balanced_parens})?/) {
> + my $curr_attr = $1;
> + my $params = '';
> + $params = $2 if defined($2);
> + $curr_attr =~ s/^[\s_]+|[\s_]+$//g;
> +
> + if (exists($attr_list{$curr_attr})) {
> + my $new = $attr_list{$curr_attr};
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "$new$params is preffered over __attribute__(($attr))\n" . $herecurr);
> + }
> + }
>
> -# Check for __attribute__ aligned, prefer __aligned
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(.*aligned/) {
> - WARN("PREFER_ALIGNED",
> - "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr);
> - }
> + # Check for __attribute__ format(printf, prefer __printf
> + if ($attr =~ /^_*format_*\s*\(\s*printf/) {
> + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex;
>
> -# Check for __attribute__ section, prefer __section
> - 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);
> - if (WARN("PREFER_SECTION",
> - "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/;
> + }
> }
> - }
>
> -# Check for __attribute__ format(printf, prefer __printf
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf/) {
> - if (WARN("PREFER_PRINTF",
> - "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex;
> + # Check for __attribute__ format(scanf, prefer __scanf
> + if ($attr =~ /^_*format_*\s*\(\s*scanf\b/) {
> + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex;
> + }
> + }
>
> + # Check for __attribute__ section, prefer __section
> + if ($attr =~ /^_*section_*\s*\(\s*("[^"]*")/) {
> + my $old = substr($rawline, $-[1], $+[1] - $-[1]);
> + my $new = substr($old, 1, -1);
> + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/;
> + }
> }
> - }
>
> -# Check for __attribute__ format(scanf, prefer __scanf
> - if ($realfile !~ m@\binclude/uapi/@ &&
> - $line =~ /\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\b/) {
> - if (WARN("PREFER_SCANF",
> - "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) &&
> - $fix) {
> - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex;
> + # Check for __attribute__ unused, prefer __always_unused or __maybe_unused
> + if ($attr =~ /^_*unused/) {
> + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO",
> + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr);
> }
> }
>
> --
> 2.27.0
>
Hi,
In this version there is only one generic test for both parameterized
and non parameterized attributes.
And four attribute checks are kept separate and unaltered from the
original implementation. Those are the format(printf/scanf), section
and unused(which can be resolved to __always_unused or __maybe_unused).
Thanks,
Dwaipayan.
Powered by blists - more mailing lists