From patchwork Thu Jul 20 12:02:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christopher Li X-Patchwork-Id: 9854701 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 C0D3A602BA for ; Thu, 20 Jul 2017 12:02:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B160F27CEA for ; Thu, 20 Jul 2017 12:02:36 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A61772846D; Thu, 20 Jul 2017 12:02:36 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID 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 0A38227CEA for ; Thu, 20 Jul 2017 12:02:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935448AbdGTMCf (ORCPT ); Thu, 20 Jul 2017 08:02:35 -0400 Received: from mail-pg0-f43.google.com ([74.125.83.43]:35381 "EHLO mail-pg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934876AbdGTMCe (ORCPT ); Thu, 20 Jul 2017 08:02:34 -0400 Received: by mail-pg0-f43.google.com with SMTP id v190so13980398pgv.2 for ; Thu, 20 Jul 2017 05:02:34 -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=921aWlMlMpmGoSpaus03ldEf3jon6lmUE4RM8tRek+0=; b=PGvgFBdi2Be3zXxf2h1DxAEKhqCCVRUK1eZlCX+dxQ6lAnge813eZRX+yZl8zl9pca vTFwUXevP4vCDE0+7SGz2zXzyaPvU+IIiDpij/cCcTNZYxk2OyGESB0kMH3T69MRLL2Y OlwV8i0R0hlQIGaxpnpv+fO0DoefqU4Mh2J55yP5OwyqUWxRloXBuCKA7RNFqJBFBo3x 1udK14Csfsb6hXvoZrOLxdOKN9FkZRS1Xy/cLOw9AcqJ1XpGApN/rZMEOiigweCzjYf6 OenGWERu2gGOvSaq+RSvkJ9j0Rvjhj0DnQRaddhShtzZL2Hy9s0Wdz/VYDfC5ef34V5U zIeQ== 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=921aWlMlMpmGoSpaus03ldEf3jon6lmUE4RM8tRek+0=; b=FB/diIhAsk72HlQQzGfbwFSWSfg7MdpopdNht+vsksxGhohio8/EVejp7uF9tsOqDR mUgK8zejXv3sT9ScFZDOnwvJCZ5rL72gvtSEt1O8ZhGa1G8FqH8OboVfqkioJRL/EUb9 prJMVfyB5xbBPP6oJizKo8rasSRZYSPTke+f+ASRR1u4tVJBny6+CI/1fvHTxW5uLXSe oKuoW8dEpZfXY8NSXhQenj662XDhbGY0P6MyV8FmGtwSQhJm9QN3lbu8EfGt7SmKwn7V +HsLbFA9cbAYN2aNPk4cIAh0RSL+NgNXUWYlQ50XqBsEEBwq7k/4dq6jO2vervYxjahU EcjQ== X-Gm-Message-State: AIVw111/mVd6pefAu643WJPSWYk3LnW3MD0q3t6zrqZy45p70CKkJVI9 SzKS8drNmdTT9UfeCbrri2jl8fYE0w== X-Received: by 10.99.0.151 with SMTP id 145mr3665574pga.115.1500552154082; Thu, 20 Jul 2017 05:02:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.149.130 with HTTP; Thu, 20 Jul 2017 05:02:33 -0700 (PDT) In-Reply-To: <5ffb141e-fdb2-a1b8-a5b7-c922d1a9ef08@ramsayjones.plus.com> References: <5ffb141e-fdb2-a1b8-a5b7-c922d1a9ef08@ramsayjones.plus.com> From: Christopher Li Date: Thu, 20 Jul 2017 08:02:33 -0400 X-Google-Sender-Auth: mtP2rhaAc6xaCFiMK8RUw0O7eBw Message-ID: Subject: Re: [PATCH 2/2] pre-process: replace use of vla's with heap allocation To: Ramsay Jones Cc: Luc Van Oostenryck , Sparse Mailing-list 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 On Wed, Jul 19, 2017 at 4:13 PM, Ramsay Jones wrote: > > The 'selfcheck' make target issues warnings about using vla's in the > pre-processor code, like so: > > CHECK pre-process.c > pre-process.c:712:25: warning: Variable length array is used. > pre-process.c:2019:28: warning: Variable length array is used. > > A Makefile change to pass '-Wno-vla' to sparse when processing this > source file (or all source files) may be a better solution than the > one given here. > > Replace the use of vla's with heap allocation. This has performance > implications (although it may me safer), due to the dynamic memory > allocation and the zero initialisation of the memory (using calloc). > I have not done any timing measurements to see if this is a problem > in practice. I purpose the following patch. Make the expand using stack for small argument numbers. That should not have much performance impact at all because long macro arguments are rare. Incremental patch follows. If you think that is fine, I will apply the combined patch as yours. > + if (nargs > 0) > + args = calloc(nargs, sizeof(*args)); Need to check alloc failed. > + > if (sym->arglist) { > if (!match_op(scan_next(&token->next), '(')) > return 1; Need to free alloc memory. > + if (nargs > 0) > + args = calloc(nargs, sizeof(*args)); Same here need to check alloc failed. Chris Purposed incremental fix up follows: endop, struct token *start) @@ -2024,8 +2035,11 @@ static void dump_macro(struct symbol *sy struct token **args = NULL; struct token *token; - if (nargs > 0) + if (nargs > 0) { args = calloc(nargs, sizeof(*args)); + if (!args) + die("calloc %ld", nargs * sizeof(*args)); + } printf("#define %s", show_ident(sym->ident)); token = sym->arglist; --- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- sparse.chrisl.orig/pre-process.c +++ sparse.chrisl/pre-process.c @@ -709,21 +709,30 @@ static int expand(struct token **list, s struct ident *expanding = token->ident; struct token **tail; int nargs = sym->arglist ? sym->arglist->count.normal : 0; - struct arg *args = NULL; +#define ARG_LIMIT 8 + struct arg arg_array[ARG_LIMIT], *args = arg_array; + int err = 0; if (expanding->tainted) { token->pos.noexpand = 1; return 1; } - if (nargs > 0) + if (nargs >= ARG_LIMIT) { args = calloc(nargs, sizeof(*args)); + if (!args) + die("calloc(%d, %lu) failed", nargs, sizeof(*args)); + } if (sym->arglist) { - if (!match_op(scan_next(&token->next), '(')) - return 1; - if (!collect_arguments(token->next, sym->arglist, args, token)) - return 1; + if (!match_op(scan_next(&token->next), '(')) { + err = 1; + goto exit; + } + if (!collect_arguments(token->next, sym->arglist, args, token)) { + err = 1; + goto exit; + } expand_arguments(nargs, args); } @@ -741,9 +750,11 @@ static int expand(struct token **list, s (*list)->pos.whitespace = token->pos.whitespace; *tail = last; - free(args); +exit: + if (nargs >= ARG_LIMIT) + free(args); - return 0; + return err; } static const char *token_name_sequence(struct token *token, int