Re: [PATCH] Print an error if typeof() lacks an argument

[Martin Nagy <mnagy@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

Christopher Li wrote:
> On Sat, Apr 25, 2009 at 4:03 AM, Martin Nagy <mnagy@redhat.com> 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

[-- Attachment #2: 0001-Print-an-error-if-typeof-lacks-an-argument.patch --]
[-- Type: text/x-patch, Size: 2018 bytes --]

From bbd2e88cdd9d36d47ce50204d18547e08f2e2bea Mon Sep 17 00:00:00 2001
From: Martin Nagy <nagy.martin@gmail.com>
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 <nagy.martin@gmail.com>
---
 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