From patchwork Mon Apr 27 09:15:37 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Nagy X-Patchwork-Id: 20081 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n3R9Fk3N019926 for ; Mon, 27 Apr 2009 09:15:47 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752596AbZD0JPn (ORCPT ); Mon, 27 Apr 2009 05:15:43 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752872AbZD0JPn (ORCPT ); Mon, 27 Apr 2009 05:15:43 -0400 Received: from mx2.redhat.com ([66.187.237.31]:60899 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752596AbZD0JPn (ORCPT ); Mon, 27 Apr 2009 05:15:43 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n3R9FfuF029298; Mon, 27 Apr 2009 05:15:42 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n3R9Ff7x004883; Mon, 27 Apr 2009 05:15:41 -0400 Received: from notas (vpn-10-61.str.redhat.com [10.32.10.61]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n3R9FdMp023820; Mon, 27 Apr 2009 05:15:40 -0400 Date: Mon, 27 Apr 2009 11:15:37 +0200 From: Martin Nagy To: Christopher Li Cc: linux-sparse@vger.kernel.org Subject: Re: [PATCH] Print an error if typeof() lacks an argument Message-ID: <20090427111537.17956e38@notas> In-Reply-To: <70318cbf0904262338g698eb2g7265b472fb0efd46@mail.gmail.com> References: <20090425130343.3df87cbb@notas> <70318cbf0904262338g698eb2g7265b472fb0efd46@mail.gmail.com> Organization: Red Hat, Inc. Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Sender: linux-sparse-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sparse@vger.kernel.org Christopher Li wrote: > On Sat, Apr 25, 2009 at 4:03 AM, Martin Nagy wrote: > > +               struct expression *expr; > > I think you want expr = NULL here. Otherwise if(expr) will pick up crap. Right. I somehow assumed that parse_expression() will set it to NULL in case there is not any expression. I attached a new patch which fixes this. > I would just add two lines after "token = > parse_expression(token->next, &typeof_sym->initializer);" > > if (!type->initializer) > sparse_error(token->pos, "expected expression after the '(' token"); > > If there is compile error, the sparse should not continue the later > stage any way. That won't work. sparse_error() will not exit, and sparse will still segfault later, so we have to return from the function. You could instead do something like this: if (!typeof_sym->initializer) { sparse_error(token->pos, "expected ..."); return expect(token, ')', "after typeof"); } Or use a goto to jump to the return statement. In any case, I didn't want to repeat the code and cause a memory leak. And I didn't want to use a goto. I guess that it's ultimately a matter of style. If you think the memory leak would be acceptable I can rework the patch again. The leak would only occur in this specific case, so I guess maybe it would be acceptable, but I wasn't sure so I rather went with this approach. > BTW, can you add a validation test case which will trigger the bug? Yup, it's in the new patch. Martin From bbd2e88cdd9d36d47ce50204d18547e08f2e2bea Mon Sep 17 00:00:00 2001 From: Martin Nagy Date: Mon, 27 Apr 2009 10:48:50 +0200 Subject: [PATCH] Print an error if typeof() lacks an argument We weren't checking if the initializer isn't NULL, which caused sparse to segfault later on when performing lazy evaluation in classify_type(). Signed-off-by: Martin Nagy --- parse.c | 17 +++++++++++------ validation/bad-typeof.c | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 validation/bad-typeof.c diff --git a/parse.c b/parse.c index 9662122..604e528 100644 --- a/parse.c +++ b/parse.c @@ -924,12 +924,17 @@ static struct token *typeof_specifier(struct token *token, struct decl_state *ct ctx->ctype.base_type = sym->ctype.base_type; apply_ctype(token->pos, &sym->ctype, &ctx->ctype); } else { - struct symbol *typeof_sym = alloc_symbol(token->pos, SYM_TYPEOF); - token = parse_expression(token->next, &typeof_sym->initializer); - - typeof_sym->endpos = token->pos; - ctx->ctype.base_type = typeof_sym; - } + struct expression *expr = NULL; + token = parse_expression(token->next, &expr); + if (expr) { + struct symbol *typeof_sym = alloc_symbol(token->pos, SYM_TYPEOF); + typeof_sym->endpos = token->pos; + typeof_sym->initializer = expr; + ctx->ctype.base_type = typeof_sym; + } else { + sparse_error(token->pos, "expected expression after the '(' token"); + } + } return expect(token, ')', "after typeof"); } diff --git a/validation/bad-typeof.c b/validation/bad-typeof.c new file mode 100644 index 0000000..5c27de4 --- /dev/null +++ b/validation/bad-typeof.c @@ -0,0 +1,15 @@ +static int fun(void) +{ + typeof() a; + int b; + + a = b; +} +/* + * check-name: Bad typeof syntax segfault + * + * check-error-start +bad-typeof.c:3:16: error: expected expression after the '(' token +bad-typeof.c:6:9: error: identifier 'a' has no type + * check-error-end + */ -- 1.6.0.6