From patchwork Sat May 6 20:22:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christopher Li X-Patchwork-Id: 9715099 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 697AF60387 for ; Sat, 6 May 2017 20:22:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4E5C72621D for ; Sat, 6 May 2017 20:22:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 303AB27F8C; Sat, 6 May 2017 20:22:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RCVD_IN_SORBS_SPAM,T_DKIM_INVALID,T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 478422621D for ; Sat, 6 May 2017 20:22:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751157AbdEFUWH (ORCPT ); Sat, 6 May 2017 16:22:07 -0400 Received: from mail-io0-f169.google.com ([209.85.223.169]:35637 "EHLO mail-io0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751115AbdEFUWG (ORCPT ); Sat, 6 May 2017 16:22:06 -0400 Received: by mail-io0-f169.google.com with SMTP id f102so31825622ioi.2 for ; Sat, 06 May 2017 13:22:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=kBgqFNFfqj+C38qqmszfZq3bsbjOTsUJp8V+GRcGI4c=; b=Iuk7C6CVeQmNd9dNLfn07S1gyxfu1bXqYp9FIsM33X/WhDTnov4ee0ju8Fy1cxRy1T SPeEgoJ4ser1Q9bgaMZjs3r5Mt+MzZIsM1MIGtszKVYEOHRzQ+eEkLEf7b4Fg9fQM134 QPjMbq/Jn5txrzxyI7Pc2F82bcNudDiENwOhlRT1z3MD/tDcvx8dfdT6BF+nEiX8UapE tr0rd/7wDd1HBDs/kjdvskHWaZuaskZX8Z7DRVzP2ot3WDbpLsf+vnXzPZTq8aLyhnRo luK/7ABMrQuPgge+FDtBFAZDWKMJTVwXl0weAvHDJfuKhuK/pBt9r+nvDCZg9GW0L//L 0dEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=kBgqFNFfqj+C38qqmszfZq3bsbjOTsUJp8V+GRcGI4c=; b=h26p+ft7u4Nt0Ms1IrPIgDSHt9vdppByxCzXlKZejo6PTO8saqglNRXWQatoDpoQZM kSc5FmfzgR89F3fgFJ72lADPq8g3rhMC1OLSTzCOopaZvs9qawiXQvEyvn1lr7jCX5TP R/GpVJlBsjIJaedxzvROUCCACawPzVhZ2ERfcmbVRnAQ2Zdz/tXGhO+bxXpW4v0QEFZK HuZrffGrbe4WsXDqcqIivbUWQuAGVw7NbFzoLNAtLSilqVIOdb0WvrSEinSDAlB2ha5l wIEv9eNuYQyytKO862KD43sEOHImDmIKuA5e8b09zp6C75CRf88KH79Yxc/a4+yENDjj gFOA== X-Gm-Message-State: AN3rC/6DWPAVy0lBd7r4s+7JSxdqllUM7+VF0dH+uCk4hela4Xog+mVc OJv9eGR/i33W/wdQm3bHcDlgR5JL4Q== X-Received: by 10.107.153.82 with SMTP id b79mr48347766ioe.200.1494102125652; Sat, 06 May 2017 13:22:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.13.18 with HTTP; Sat, 6 May 2017 13:22:04 -0700 (PDT) In-Reply-To: <20170504160038.5328-1-lrichard@redhat.com> References: <20170504160038.5328-1-lrichard@redhat.com> From: Christopher Li Date: Sat, 6 May 2017 16:22:04 -0400 X-Google-Sender-Auth: KiPGo3P_TwvU9Mk5d6ytPDTwAjQ Message-ID: Subject: Re: [PATCH v5] sparse: add support for _Static_assert To: Lance Richardson Cc: Linux-Sparse , Luc Van Oostenryck Sender: linux-sparse-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sparse@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I play with it a bit. On Thu, May 4, 2017 at 12:00 PM, Lance Richardson wrote: > +static int match_static_assert(struct token *token) > +{ > + return token_type(token) == TOKEN_IDENT && > + token->ident == &_Static_assert_ident; > +} I just find out there is already function match_ident() so use that instead will make it clear it is compare against "_Static_assert". Sorry I forget about this function earlier. I only find it when I play with the code today. > > struct statement *alloc_statement(struct position pos, int type) > { > @@ -1864,13 +1877,17 @@ static struct token *declaration_list(struct token *token, struct symbol_list ** > static struct token *struct_declaration_list(struct token *token, struct symbol_list **list) > { > while (!match_op(token, '}')) { > - if (!match_op(token, ';')) > - token = declaration_list(token, list); > - if (!match_op(token, ';')) { > - sparse_error(token->pos, "expected ; at end of declaration"); > - break; > + if (match_static_assert(token)) > + token = parse_static_assert(token, NULL); > + else { > + if (!match_op(token, ';')) > + token = declaration_list(token, list); > + if (!match_op(token, ';')) { > + sparse_error(token->pos, "expected ; at end of declaration"); > + break; > + } > + token = token->next; I reshape the code a bit to make it looks compact. Please see my attachment patch. > +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused) > +{ > + struct expression *expr1 = NULL, *expr2 = NULL; > + int val; > + > + token = expect(token->next, '(', "after _Static_assert"); > + token = constant_expression(token, &expr1); > + token = expect(token, ',', "after first argument of _Static_assert"); > + token = parse_expression(token, &expr2); > + token = expect(token, ')', "after second argument of _Static_assert"); > + > + token = expect(token, ';', "after _Static_assert()"); > + > + if (!expr1 || !expr2) > + return token; OK, this parsing does not complain about the following input: _Static_assert(0, ); _Static_assert(, ""); I fix it in my patch. While I am there, I rename the expr1 to "cond". > /* Make a statement out of an expression */ > static struct statement *make_statement(struct expression *expr) > { > @@ -2389,11 +2436,14 @@ static struct token * statement_list(struct token *token, struct statement_list > } > stmt = alloc_statement(token->pos, STMT_DECLARATION); > token = external_declaration(token, &stmt->declaration); > + add_statement(list, stmt); > + } else if (match_static_assert(token)) { > + token = parse_static_assert(token, NULL); After take a closer look of the grimmer syntax. Static assert is actually a declaration rather than a statement. So I merge this two because external_declaration() can handle _Static_assert parsing any way. Just need to pay attention it does not generate resulting symbol node. I rename the patch as V6. V5 does not apply to master any more. Luc, can you take a look the V6 patch as well? Thanks Chris From patchwork Thu May 4 16:00:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [v5] sparse: add support for _Static_assert From: Lance Richardson X-Patchwork-Id: 9712267 Message-Id: <20170504160038.5328-1-lrichard@redhat.com> To: linux-sparse@vger.kernel.org Date: Thu, 4 May 2017 12:00:38 -0400 This patch introduces support for the C11 _Static_assert() construct. Per the N1539 draft standard, the syntax changes for this construct include: declaration: [opt] ; struct-declaration: [opt] ; static_assert-declaration: _Static_assert ( , ) ; Signed-off-by: Lance Richardson Acked-by: Luc Van Oostenryck --- v5: Incorporated feedback from Christopher Li and Luc van Oostenryck: - Made _Static_assert a reserved identifier - Simplified check for _Static_assert keyword, consolidated into a common function. - Improved the "static assert within a function body" test case by adding a static assertion intermingled with code and adding a static assertion within a compound statement block. - Fixed use of initialized stmt variable. Tested by using sparse on entire kernel tree and a similarly-sized code tree which makes use of _Static_assert(). v4: Addressed feedback, simplified and restructured to better model description in draft standard. v3: - Removed bogus test case introduced in v2 (static assertion on sizeof a structure within the definition of the structure). v2: - Added additional test cases. - Added additional validation for parameters to _Static_assert(). - Reworked implementation to avoid impacting struct/union definition handling ( the v1 implementation, which treated _Static_assert() as an NS_TYPEDEF term, had the unfortunate side-effect of leaving an unnamed field with unknown size attached to structure definitions when a static assert was inside a structure definition). ident-list.h | 1 + parse.c | 66 ++++++++++++++++++++++++++++++++++++++++------ validation/static_assert.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 validation/static_assert.c --- sparse.chrisl.orig/parse.c +++ sparse.chrisl/parse.c @@ -73,6 +73,7 @@ static struct token *parse_context_state static struct token *parse_range_statement(struct token *token, struct statement *stmt); static struct token *parse_asm_statement(struct token *token, struct statement *stmt); static struct token *toplevel_asm_declaration(struct token *token, struct symbol_list **list); +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused); typedef struct token *attr_t(struct token *, struct symbol *, struct decl_state *); @@ -328,6 +329,10 @@ static struct symbol_op asm_op = { .toplevel = toplevel_asm_declaration, }; +static struct symbol_op static_assert_op = { + .toplevel = parse_static_assert, +}; + static struct symbol_op packed_op = { .attribute = attribute_packed, }; @@ -466,6 +471,9 @@ static struct init_keyword { { "__restrict", NS_TYPEDEF, .op = &restrict_op}, { "__restrict__", NS_TYPEDEF, .op = &restrict_op}, + /* Static assertion */ + { "_Static_assert", NS_KEYWORD, .op = &static_assert_op }, + /* Storage class */ { "auto", NS_TYPEDEF, .op = &auto_op }, { "register", NS_TYPEDEF, .op = ®ister_op }, @@ -690,7 +698,6 @@ static int SENTINEL_ATTR match_idents(st return next && token->ident == next; } - struct statement *alloc_statement(struct position pos, int type) { struct statement *stmt = __alloc_statement(0); @@ -1942,11 +1949,18 @@ static struct token *declaration_list(st return token; } +static struct token *static_assert_or_declaration_list(struct token *token, struct symbol_list **list) +{ + if (match_ident(token, &_Static_assert_ident)) + return parse_static_assert(token, NULL); + return declaration_list(token, list); +} + static struct token *struct_declaration_list(struct token *token, struct symbol_list **list) { while (!match_op(token, '}')) { if (!match_op(token, ';')) - token = declaration_list(token, list); + token = static_assert_or_declaration_list(token, list); if (!match_op(token, ';')) { sparse_error(token->pos, "expected ; at end of declaration"); break; @@ -2093,6 +2107,40 @@ static struct token *parse_asm_declarato return token; } + +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused) +{ + struct expression *cond = NULL, *fail_string = NULL; + int val; + + token = expect(token->next, '(', "after _Static_assert"); + token = constant_expression(token, &cond); + if (!cond) + sparse_error(token->pos, "expect expression before '%s' token", show_token(token)); + token = expect(token, ',', "after first argument of _Static_assert"); + token = parse_expression(token, &fail_string); + if (!fail_string) + sparse_error(token->pos, "expect expression before '%s' token", show_token(token)); + token = expect(token, ')', "after second argument of _Static_assert"); + + expect(token, ';', "after _Static_assert()"); + + if (!cond || !fail_string) + return token; + + val = const_expression_value(cond); + + if (fail_string->type != EXPR_STRING) { + sparse_error(fail_string->pos, "bad string literal"); + return token; + } + + if (cond->type == EXPR_VALUE && !val) + sparse_error(cond->pos, "static assertion failed: %s", + show_string(fail_string->string)); + return token; +} + /* Make a statement out of an expression */ static struct statement *make_statement(struct expression *expr) { @@ -2474,13 +2522,18 @@ static struct token * statement_list(str break; if (match_op(token, '}')) break; - if (lookup_type(token)) { + if (lookup_type(token) || match_ident(token, &_Static_assert_ident)) { + struct symbol_list *declaration = NULL; + if (seen_statement) { warning(token->pos, "mixing declarations and code"); seen_statement = 0; } + token = external_declaration(token, &declaration, NULL); + if (!declaration) + continue; stmt = alloc_statement(token->pos, STMT_DECLARATION); - token = external_declaration(token, &stmt->declaration, NULL); + stmt->declaration = declaration; } else { seen_statement = Wdeclarationafterstatement; token = statement(token, &stmt); @@ -2819,7 +2872,7 @@ struct token *external_declaration(struc unsigned long mod; int is_typedef; - /* Top-level inline asm? */ + /* Top-level inline asm or static assertion? */ if (token_type(token) == TOKEN_IDENT) { struct symbol *s = lookup_keyword(token->ident, NS_KEYWORD); if (s && s->op->toplevel) --- sparse.chrisl.orig/ident-list.h +++ sparse.chrisl/ident-list.h @@ -16,6 +16,7 @@ IDENT_RESERVED(for); IDENT_RESERVED(while); IDENT_RESERVED(do); IDENT_RESERVED(goto); +IDENT_RESERVED(_Static_assert); /* C typenames. They get marked as reserved when initialized */ IDENT(struct); --- sparse.chrisl.orig/validation/static_assert.c +++ sparse.chrisl/validation/static_assert.c @@ -0,0 +1,64 @@ +_Static_assert(1, "global ok"); + +struct foo { + _Static_assert(1, "struct ok"); +}; + +void bar(void) +{ + _Static_assert(1, " func1 ok"); + int i; + i = 0; + _Static_assert(1, " func2 ok"); + + if (1) { + _Static_assert(1, " func3 ok"); + } +} + +_Static_assert(0, "expected assertion failure"); + +static int f; +_Static_assert(f, "non-constant expression"); + +static int *p; +_Static_assert(p, "non-integer expression"); + +_Static_assert(0.1, "float expression"); + +_Static_assert(!0 == 1, "non-trivial expression"); + +static char array[4]; +_Static_assert(sizeof(array) == 4, "sizeof expression"); + +static const char non_literal_string[] = "non literal string"; +_Static_assert(0, non_literal_string); + +_Static_assert(1 / 0, "invalid expression: should not show up?"); + +struct s { + char arr[16]; + _Static_assert(1, "inside struct"); +}; + +union u { + char c; + int i; + _Static_assert(1, "inside union"); +}; + +_Static_assert(sizeof(struct s) == 16, "sizeof assertion"); + +/* + * check-name: static assertion + * + * check-error-start +static_assert.c:19:16: error: static assertion failed: "expected assertion failure" +static_assert.c:22:16: error: bad constant expression +static_assert.c:25:16: error: bad constant expression +static_assert.c:27:16: error: bad constant expression +static_assert.c:35:19: error: bad string literal +static_assert.c:37:18: error: bad constant expression + * check-error-end + */ +