[PATCH] saner warnings for restricted types

[PATCH] saner warnings for restricted types

[Al Viro]

* don't crap the type->ident for unsigned int just because somebody did
	typedef unsigned int x;
  only structs, unions, enums and restricted types need this logics.
* generate saner warnings for restricted, include type name(s) into them.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 evaluate.c   |   33 ++++++++++++++++++++-------------
 parse.c      |   11 +++++++++--
 show-parse.c |    4 ++++
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 54fcd3f..777f603 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -500,9 +500,10 @@ static inline void unrestrict(struct expression *expr,
 			      int class, struct symbol **ctype)
 {
 	if (class & TYPE_RESTRICT) {
-		warning(expr->pos, "restricted degrades to integer");
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
+		warning(expr->pos, "%sdegrades to integer",
+			show_typename(*ctype));
 		*ctype = (*ctype)->ctype.base_type; /* get to arithmetic type */
 	}
 }
@@ -1235,7 +1236,9 @@ static int evaluate_assign_op(struct expression *expr)
 		}
 		if (tclass & TYPE_RESTRICT) {
 			if (!restricted_binop(op, t)) {
-				expression_error(expr, "bad restricted assignment");
+				warning(expr->pos, "bad assignment (%s) to %s",
+					show_special(op), show_typename(t));
+				expr->right = cast_to(expr->right, target);
 				return 0;
 			}
 			/* allowed assignments unfoul */
@@ -1248,7 +1251,9 @@ static int evaluate_assign_op(struct expression *expr)
 		/* source and target would better be identical restricted */
 		if (t == s)
 			return 1;
-		warning(expr->pos, "invalid restricted assignment");
+		warning(expr->pos, "invalid assignment: %s", show_special(op));
+		info(expr->pos, "   left side has type %s", show_typename(t));
+		info(expr->pos, "   right side has type %s", show_typename(s));
 		expr->right = cast_to(expr->right, target);
 		return 0;
 	}
@@ -1707,10 +1712,8 @@ static struct symbol *evaluate_postop(struct expression *expr)
 		return NULL;
 	}
 
-	if ((class & TYPE_RESTRICT) && restricted_unop(expr->op, &ctype)) {
-		expression_error(expr, "bad operation on restricted");
-		return NULL;
-	}
+	if ((class & TYPE_RESTRICT) && restricted_unop(expr->op, &ctype))
+		return bad_expr_type(expr);
 
 	if (class & TYPE_NUM) {
 		multiply = 1;
@@ -1799,7 +1802,8 @@ static struct symbol *evaluate_preop(struct expression *expr)
 			expr->right->ctype = ctype;
 			expr->right->fvalue = 0;
 		} else if (is_fouled_type(ctype)) {
-			warning(expr->pos, "restricted degrades to integer");
+			warning(expr->pos, "%sdegrades to integer",
+				show_typename(ctype->ctype.base_type));
 		}
 		ctype = &bool_ctype;
 		break;
@@ -2664,9 +2668,11 @@ static struct symbol *evaluate_cast(struct expression *expr)
 
 	if (t1 != t2) {
 		if (class1 & TYPE_RESTRICT)
-			warning(expr->pos, "cast to restricted type");
+			warning(expr->pos, "cast to %s",
+				show_typename(t1));
 		if (class2 & TYPE_RESTRICT)
-			warning(expr->pos, "cast from restricted type");
+			warning(expr->pos, "cast from %s",
+				show_typename(t2));
 	}
 
 	if (t1 == &ulong_ctype)
@@ -3246,9 +3252,10 @@ static void check_case_type(struct expression *switch_expr,
 		return;
 
 	if (!restricted_binop_type(SPECIAL_EQUAL, case_expr, switch_expr,
-				   cclass, sclass, case_type, switch_type))
-		warning(case_expr->pos, "restricted degrades to integer");
-
+				   cclass, sclass, case_type, switch_type)) {
+		unrestrict(case_expr, cclass, &case_type);
+		unrestrict(switch_expr, sclass, &switch_type);
+	}
 	return;
 
 Bad:
diff --git a/parse.c b/parse.c
index a41939d..6255737 100644
--- a/parse.c
+++ b/parse.c
@@ -2158,8 +2158,15 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 	base_type = decl->ctype.base_type;
 
 	if (is_typedef) {
-		if (base_type && !base_type->ident)
-			base_type->ident = ident;
+		if (base_type && !base_type->ident) {
+			switch (base_type->type) {
+			case SYM_STRUCT:
+			case SYM_UNION:
+			case SYM_ENUM:
+			case SYM_RESTRICT:
+				base_type->ident = ident;
+			}
+		}
 	} else if (base_type && base_type->type == SYM_FN) {
 		/* K&R argument declaration? */
 		if (lookup_type(token))
diff --git a/show-parse.c b/show-parse.c
index 9a1f796..b9a2828 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -309,6 +309,10 @@ static void do_show_type(struct symbol *sym, struct type_name *name,
 		break;
 
 	case SYM_RESTRICT:
+		if (sym->ident) {
+			prepend(name, "restricted %s ", show_ident(sym->ident));
+			return;
+		}
 		break;
 
 	case SYM_FOULED:
-- 
1.5.3.GIT

Re: [PATCH] saner warnings for restricted types

[Pavel Roskin]

On Tue, 2008-01-15 at 16:52 +0000, Al Viro wrote:
>  	if (class & TYPE_RESTRICT) {
> -		warning(expr->pos, "restricted degrades to integer");
>  		if (class & TYPE_FOULED)
>  			*ctype = unfoul(*ctype);
> +		warning(expr->pos, "%sdegrades to integer",
> +			show_typename(*ctype));

Missing space after "%s"?

-- 
Regards,
Pavel Roskin

Re: [PATCH] saner warnings for restricted types

[Al Viro]

On Tue, Jan 15, 2008 at 12:36:12PM -0500, Pavel Roskin wrote:
> On Tue, 2008-01-15 at 16:52 +0000, Al Viro wrote:
> >  	if (class & TYPE_RESTRICT) {
> > -		warning(expr->pos, "restricted degrades to integer");
> >  		if (class & TYPE_FOULED)
> >  			*ctype = unfoul(*ctype);
> > +		warning(expr->pos, "%sdegrades to integer",
> > +			show_typename(*ctype));
> 
> Missing space after "%s"?

No.  Try it and you'll see...  The way show_typename() (actually, the
stuff behind it) works you get a space after the damn thing.  What you'll
get is something like

kernel/power/main.c:242:2: warning: restricted suspend_state_t degrades to integer

Actually, if you look at the things like

fs/afs/fsclient.c:465:21: warning: incorrect type in assignment (different base types)
fs/afs/fsclient.c:465:21:    expected restricted __be32 [usertype] operation_ID
fs/afs/fsclient.c:465:21:    got int

you'll see that the last line is "fs/afs/fsclient.c:465:21:    got int \n" -
with whitespace in the end.  We could get rid of that, but it'll take more
massage and it's definitely a separate patch series.

Re: [PATCH] saner warnings for restricted types

[Pavel Roskin]

On Tue, 2008-01-15 at 18:58 +0000, Al Viro wrote:
> On Tue, Jan 15, 2008 at 12:36:12PM -0500, Pavel Roskin wrote:
> > On Tue, 2008-01-15 at 16:52 +0000, Al Viro wrote:
> > >  	if (class & TYPE_RESTRICT) {
> > > -		warning(expr->pos, "restricted degrades to integer");
> > >  		if (class & TYPE_FOULED)
> > >  			*ctype = unfoul(*ctype);
> > > +		warning(expr->pos, "%sdegrades to integer",
> > > +			show_typename(*ctype));
> > 
> > Missing space after "%s"?
> 
> No.  Try it and you'll see...

OK, just wanted to make sure.  Thanks for the explanation!

-- 
Regards,
Pavel Roskin

Re: [PATCH] saner warnings for restricted types

[Al Viro]

On Tue, Jan 15, 2008 at 06:58:56PM +0000, Al Viro wrote:
> with whitespace in the end.  We could get rid of that, but it'll take more
> massage and it's definitely a separate patch series.

Whee...

; cat >foo.c <<'EOF'
int a[2][3];
EOF
; test-parsing foo.c
foo.c:1:5: warning: symbol 'a' was not declared. Should it be static?
.align 4
int [addressable] [toplevel] a[3][2]
;

See that [3][2]?  IOW, show_typename() really needs fixing.  Oh, well...

The problem is that append() part should go *before* recursive call, not
after it.  We want appended stuff from the outer levels closer to the
center (i.e. to the name if it's present).  Anyway, the fix follows:

[PATCH] fix show_typename()

* postfix stuff had been added in the wrong order (e.g. int a[2][3] generated
int [addressable][toplevel] a[3][2])
* after fixing that, we've no need for recursion anymore, a bunch of arguments
go away and turn into local variables and we get an easy way to get rid of
bogus space in the show_typename() result.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 evaluate.c   |    4 +-
 show-parse.c |  100 +++++++++++++++++++++++++++++----------------------------
 2 files changed, 53 insertions(+), 51 deletions(-)

diff --git a/evaluate.c b/evaluate.c
index 777f603..2901c1b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -502,7 +502,7 @@ static inline void unrestrict(struct expression *expr,
 	if (class & TYPE_RESTRICT) {
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
-		warning(expr->pos, "%sdegrades to integer",
+		warning(expr->pos, "%s degrades to integer",
 			show_typename(*ctype));
 		*ctype = (*ctype)->ctype.base_type; /* get to arithmetic type */
 	}
@@ -1802,7 +1802,7 @@ static struct symbol *evaluate_preop(struct expression *expr)
 			expr->right->ctype = ctype;
 			expr->right->fvalue = 0;
 		} else if (is_fouled_type(ctype)) {
-			warning(expr->pos, "%sdegrades to integer",
+			warning(expr->pos, "%s degrades to integer",
 				show_typename(ctype->ctype.base_type));
 		}
 		ctype = &bool_ctype;
diff --git a/show-parse.c b/show-parse.c
index b9a2828..064af32 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -226,12 +226,16 @@ const char *builtin_ctypename(struct ctype *ctype)
 	return NULL;
 }
 
-static void do_show_type(struct symbol *sym, struct type_name *name,
-			 unsigned long mod, int as, int was_ptr)
+static void do_show_type(struct symbol *sym, struct type_name *name)
 {
 	const char *typename;
-	int is_ptr = was_ptr;
+	unsigned long mod = 0;
+	int as = 0;
+	int was_ptr = 0;
+	int restr = 0;
+	int fouled = 0;
 
+deeper:
 	if (!sym || (sym->type != SYM_NODE && sym->type != SYM_ARRAY &&
 		     sym->type != SYM_BITFIELD)) {
 		const char *s;
@@ -249,14 +253,15 @@ static void do_show_type(struct symbol *sym, struct type_name *name,
 	}
 
 	if (!sym)
-		return;
+		goto out;
 
 	if ((typename = builtin_typename(sym))) {
 		int len = strlen(typename);
-		*--name->start = ' ';
+		if (name->start != name->end)
+			*--name->start = ' ';
 		name->start -= len;
 		memcpy(name->start, typename, len);
-		return;
+		goto out;
 	}
 
 	/* Prepend */
@@ -265,20 +270,29 @@ static void do_show_type(struct symbol *sym, struct type_name *name,
 		prepend(name, "*");
 		mod = sym->ctype.modifiers;
 		as = sym->ctype.as;
-		is_ptr = 1;
+		was_ptr = 1;
 		break;
+
 	case SYM_FN:
-		if (was_ptr)
+		if (was_ptr) {
 			prepend(name, "( ");
-		is_ptr = 0;
+			append(name, " )");
+			was_ptr = 0;
+		}
+		append(name, "( ... )");
 		break;
+
 	case SYM_STRUCT:
-		prepend(name, "struct %s ", show_ident(sym->ident));
-		return;
+		if (name->start != name->end)
+			*--name->start = ' ';
+		prepend(name, "struct %s", show_ident(sym->ident));
+		goto out;
 
 	case SYM_UNION:
-		prepend(name, "union %s ", show_ident(sym->ident));
-		return;
+		if (name->start != name->end)
+			*--name->start = ' ';
+		prepend(name, "union %s", show_ident(sym->ident));
+		goto out;
 
 	case SYM_ENUM:
 		prepend(name, "enum %s ", show_ident(sym->ident));
@@ -298,60 +312,48 @@ static void do_show_type(struct symbol *sym, struct type_name *name,
 
 	case SYM_LABEL:
 		append(name, "label(%s:%p)", show_ident(sym->ident), sym);
-		break;
+		return;
 
 	case SYM_ARRAY:
 		mod |= sym->ctype.modifiers;
 		as |= sym->ctype.as;
-		if (was_ptr)
+		if (was_ptr) {
 			prepend(name, "( ");
-		is_ptr = 0;
+			append(name, " )");
+			was_ptr = 0;
+		}
+		append(name, "[%lld]", get_expression_value(sym->array_size));
 		break;
 
 	case SYM_RESTRICT:
-		if (sym->ident) {
-			prepend(name, "restricted %s ", show_ident(sym->ident));
-			return;
+		if (!sym->ident) {
+			restr = 1;
+			break;
 		}
-		break;
+		if (name->start != name->end)
+			*--name->start = ' ';
+		prepend(name, "restricted %s", show_ident(sym->ident));
+		goto out;
 
 	case SYM_FOULED:
+		fouled = 1;
 		break;
 
 	default:
+		if (name->start != name->end)
+			*--name->start = ' ';
 		prepend(name, "unknown type %d", sym->type);
-		return;
+		goto out;
 	}
 
-	do_show_type(sym->ctype.base_type, name, mod, as, is_ptr);
-
-	switch (sym->type) {
-	case SYM_PTR:
-		return;
-
-	case SYM_FN:
-		if (was_ptr)
-			append(name, " )");
-		append(name, "( ... )");
-		return;
-
-	case SYM_ARRAY:
-		if (was_ptr)
-			append(name, " )");
-		append(name, "[%lld]", get_expression_value(sym->array_size));
-		return;
+	sym = sym->ctype.base_type;
+	goto deeper;
 
-	case SYM_RESTRICT:
+out:
+	if (restr)
 		prepend(name, "restricted ");
-		return;
-
-	case SYM_FOULED:
+	if (fouled)
 		prepend(name, "fouled ");
-		return;
-
-	default:
-		break;
-	}
 }
 
 void show_type(struct symbol *sym)
@@ -360,7 +362,7 @@ void show_type(struct symbol *sym)
 	struct type_name name;
 
 	name.start = name.end = array+100;
-	do_show_type(sym, &name, 0, 0, 0);
+	do_show_type(sym, &name);
 	*name.end = 0;
 	printf("%s", name.start);
 }
@@ -371,7 +373,7 @@ const char *show_typename(struct symbol *sym)
 	struct type_name name;
 
 	name.start = name.end = array+100;
-	do_show_type(sym, &name, 0, 0, 0);
+	do_show_type(sym, &name);
 	*name.end = 0;
 	return name.start;
 }
-- 
1.5.3.GIT