Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201201152017.3576951-1-elver@google.com>
In-Reply-To: <20201201152017.3576951-1-elver@google.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Tue, 1 Dec 2020 11:59:50 -0800
Message-ID: <CAKwvOdkcv=FES2CXfoY+AFcvg_rbPd2Nk8sEwXNBJqXL4wQGBg@mail.gmail.com>
Subject: Re: [PATCH] genksyms: Ignore module scoped _Static_assert()
To:     Marco Elver <elver@google.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Joe Perches <joe@perches.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Dec 1, 2020 at 7:21 AM Marco Elver <elver@google.com> wrote:
>
> The C11 _Static_assert() keyword may be used at module scope, and we
> need to teach genksyms about it to not abort with an error. We currently
> have a growing number of static_assert() (but also direct usage of
> _Static_assert()) users at module scope:
>
>         git grep -E '^_Static_assert\(|^static_assert\(' | grep -v '^tools' | wc -l
>         135
>
> More recently, when enabling CONFIG_MODVERSIONS with CONFIG_KCSAN, we
> observe a number of warnings:
>
>         WARNING: modpost: EXPORT symbol "<..all kcsan symbols..>" [vmlinux] [...]
>
> When running a preprocessed source through 'genksyms -w' a number of
> syntax errors point at usage of static_assert()s. In the case of
> kernel/kcsan/encoding.h, new static_assert()s had been introduced which
> used expressions that appear to cause genksyms to not even be able to
> recover from the syntax error gracefully (as it appears was the case
> previously).
>
> Therefore, make genksyms ignore all _Static_assert() and the contained
> expression. With the fix, usage of _Static_assert() no longer cause
> "syntax error" all over the kernel, and the above modpost warnings for
> KCSAN are gone, too.
>
> Signed-off-by: Marco Elver <elver@google.com>

Ah, genksyms...if only there were a library that we could use to parse
C code...:P
Acked-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  scripts/genksyms/keywords.c |  3 +++
>  scripts/genksyms/lex.l      | 27 ++++++++++++++++++++++++++-
>  scripts/genksyms/parse.y    |  7 +++++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/genksyms/keywords.c b/scripts/genksyms/keywords.c
> index 057c6cabad1d..b85e0979a00c 100644
> --- a/scripts/genksyms/keywords.c
> +++ b/scripts/genksyms/keywords.c
> @@ -32,6 +32,9 @@ static struct resword {
>         { "restrict", RESTRICT_KEYW },
>         { "asm", ASM_KEYW },
>
> +       // c11 keywords that can be used at module scope
> +       { "_Static_assert", STATIC_ASSERT_KEYW },
> +
>         // attribute commented out in modutils 2.4.2.  People are using 'attribute' as a
>         // field name which breaks the genksyms parser.  It is not a gcc keyword anyway.
>         // KAO. },
> diff --git a/scripts/genksyms/lex.l b/scripts/genksyms/lex.l
> index e265c5d96861..ae76472efc43 100644
> --- a/scripts/genksyms/lex.l
> +++ b/scripts/genksyms/lex.l
> @@ -118,7 +118,7 @@ yylex(void)
>  {
>    static enum {
>      ST_NOTSTARTED, ST_NORMAL, ST_ATTRIBUTE, ST_ASM, ST_TYPEOF, ST_TYPEOF_1,
> -    ST_BRACKET, ST_BRACE, ST_EXPRESSION,
> +    ST_BRACKET, ST_BRACE, ST_EXPRESSION, ST_STATIC_ASSERT,
>      ST_TABLE_1, ST_TABLE_2, ST_TABLE_3, ST_TABLE_4,
>      ST_TABLE_5, ST_TABLE_6
>    } lexstate = ST_NOTSTARTED;
> @@ -201,6 +201,11 @@ repeat:
>
>                   case EXPORT_SYMBOL_KEYW:
>                       goto fini;
> +
> +                 case STATIC_ASSERT_KEYW:
> +                   lexstate = ST_STATIC_ASSERT;
> +                   count = 0;
> +                   goto repeat;
>                   }
>               }
>             if (!suppress_type_lookup)
> @@ -401,6 +406,26 @@ repeat:
>         }
>        break;
>
> +    case ST_STATIC_ASSERT:
> +      APP;
> +      switch (token)
> +       {
> +       case '(':
> +         ++count;
> +         goto repeat;
> +       case ')':
> +         if (--count == 0)
> +           {
> +             lexstate = ST_NORMAL;
> +             token = STATIC_ASSERT_PHRASE;
> +             break;
> +           }
> +         goto repeat;
> +       default:
> +         goto repeat;
> +       }
> +      break;
> +
>      case ST_TABLE_1:
>        goto repeat;
>
> diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y
> index e22b42245bcc..8e9b5e69e8f0 100644
> --- a/scripts/genksyms/parse.y
> +++ b/scripts/genksyms/parse.y
> @@ -80,6 +80,7 @@ static void record_compound(struct string_list **keyw,
>  %token SHORT_KEYW
>  %token SIGNED_KEYW
>  %token STATIC_KEYW
> +%token STATIC_ASSERT_KEYW
>  %token STRUCT_KEYW
>  %token TYPEDEF_KEYW
>  %token UNION_KEYW
> @@ -97,6 +98,7 @@ static void record_compound(struct string_list **keyw,
>  %token BRACE_PHRASE
>  %token BRACKET_PHRASE
>  %token EXPRESSION_PHRASE
> +%token STATIC_ASSERT_PHRASE
>
>  %token CHAR
>  %token DOTS
> @@ -130,6 +132,7 @@ declaration1:
>         | function_definition
>         | asm_definition
>         | export_definition
> +       | static_assert
>         | error ';'                             { $$ = $2; }
>         | error '}'                             { $$ = $2; }
>         ;
> @@ -493,6 +496,10 @@ export_definition:
>                 { export_symbol((*$3)->string); $$ = $5; }
>         ;
>
> +/* Ignore any module scoped _Static_assert(...) */
> +static_assert:
> +       STATIC_ASSERT_PHRASE ';'                        { $$ = $2; }
> +       ;
>
>  %%
>
> --
> 2.29.2.454.gaff20da3a2-goog
>


-- 
Thanks,
~Nick Desaulniers