[PATCH 12/16] Revert the conditional_context patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>

This patch removes the conditional_context attribute again, it turned
out that my attempt to do this was rather misguided and contrary to
what I thought we do not gain anything at all over using macros for
it as the kernel and the tests have been doing for a while.

Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
---
 linearize.c                  |   32 +++++---
 linearize.h                  |    2 
 parse.c                      |   52 -------------
 sparse.1                     |    9 --
 sparse.c                     |   39 ++++------
 symbol.h                     |    2 
 validation/context-dynamic.c |  171 ------------------------------------------
 7 files changed, 37 insertions(+), 270 deletions(-)
 delete mode 100644 validation/context-dynamic.c

diff --git a/linearize.c b/linearize.c
index 8e9775d..34922e9 100644
--- a/linearize.c
+++ b/linearize.c
@@ -439,7 +439,7 @@ const char *show_instruction(struct instruction *insn)
 		break;
 
 	case OP_CONTEXT:
-		buf += sprintf(buf, "%s%d,%d", "", insn->increment, insn->inc_false);
+		buf += sprintf(buf, "%d", insn->increment);
 		break;
 	case OP_RANGE:
 		buf += sprintf(buf, "%s between %s..%s", show_pseudo(insn->src1), show_pseudo(insn->src2), show_pseudo(insn->src3));
@@ -960,7 +960,7 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
 			continue;
 		insn = alloc_instruction(OP_CONTEXT, 0);
 		insn->required = context->in;
-		insn->increment = insn->inc_false = context->out - context->in;
+		insn->increment = context->out - context->in;
 		insn->context_expr = context->context;
 		insn->access_var = ad->source_type;
 		insn->exact = context->exact;
@@ -973,7 +973,7 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
 			continue;
 		insn = alloc_instruction(OP_CONTEXT, 0);
 		insn->required = context->in;
-		insn->increment = insn->inc_false = context->out - context->in;
+		insn->increment = context->out - context->in;
 		insn->context_expr = context->context;
 		insn->access_var = ad->result_type;
 		insn->exact = context->exact;
@@ -988,7 +988,7 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
 				continue;
 			insn = alloc_instruction(OP_CONTEXT, 0);
 			insn->required = context->in;
-			insn->increment = insn->inc_false = context->out - context->in;
+			insn->increment = context->out - context->in;
 			insn->context_expr = context->context;
 			insn->access_var = ad->address->sym;
 			insn->exact = context->exact;
@@ -1049,7 +1049,7 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
 			continue;
 		insn = alloc_instruction(OP_CONTEXT, 0);
 		insn->required = context->in;
-		insn->increment = insn->inc_false = context->out - context->in;
+		insn->increment = context->out - context->in;
 		insn->context_expr = context->context;
 		insn->access_var = ad->source_type;
 		insn->exact = context->exact;
@@ -1062,7 +1062,7 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
 			continue;
 		insn = alloc_instruction(OP_CONTEXT, 0);
 		insn->required = context->in;
-		insn->increment = insn->inc_false = context->out - context->in;
+		insn->increment = context->out - context->in;
 		insn->context_expr = context->context;
 		insn->access_var = ad->result_type;
 		insn->exact = context->exact;
@@ -1077,7 +1077,7 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
 				continue;
 			insn = alloc_instruction(OP_CONTEXT, 0);
 			insn->required = context->in;
-			insn->increment = insn->inc_false = context->out - context->in;
+			insn->increment = context->out - context->in;
 			insn->context_expr = context->context;
 			insn->access_var = ad->address->sym;
 			insn->exact = context->exact;
@@ -1322,12 +1322,21 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
 		FOR_EACH_PTR(ctype->contexts, context) {
 			int in = context->in;
 			int out = context->out;
-
-			if (out - in || context->out_false - in) {
+			int check = 0;
+			int context_diff;
+			if (in < 0) {
+				check = 1;
+				in = 0;
+			}
+			if (out < 0) {
+				check = 0;
+				out = 0;
+			}
+			context_diff = out - in;
+			if (check || context_diff) {
 				insn = alloc_instruction(OP_CONTEXT, 0);
-				insn->increment = out - in;
+				insn->increment = context_diff;
 				insn->context_expr = context->context;
-				insn->inc_false = context->out_false - in;
 				add_one_insn(ep, insn);
 			}
 		} END_FOR_EACH_PTR(context);
@@ -1757,7 +1766,6 @@ static pseudo_t linearize_context(struct entrypoint *ep, struct statement *stmt)
 	struct instruction *insn = alloc_instruction(OP_CONTEXT, 0);
 
 	insn->increment = stmt->change;
-	insn->inc_false = stmt->change;
 
 	insn->required = stmt->required;
 	insn->exact = stmt->exact;
diff --git a/linearize.h b/linearize.h
index 4e9100d..3bd5098 100644
--- a/linearize.h
+++ b/linearize.h
@@ -117,7 +117,7 @@ struct instruction {
 			struct pseudo_list *arguments;
 		};
 		struct /* context */ {
-			int increment, required, inc_false, exact;
+			int increment, required, exact;
 			struct expression *context_expr;
 			struct symbol *access_var;
 		};
diff --git a/parse.c b/parse.c
index fdb7efb..5e3354c 100644
--- a/parse.c
+++ b/parse.c
@@ -65,7 +65,6 @@ static struct token *attribute_address_space(struct token *token, struct symbol
 static struct token *attribute_aligned(struct token *token, struct symbol *attr, struct ctype *ctype);
 static struct token *attribute_mode(struct token *token, struct symbol *attr, struct ctype *ctype);
 static struct token *attribute_context(struct token *token, struct symbol *attr, struct ctype *ctype);
-static struct token *attribute_conditional_context(struct token *token, struct symbol *attr, struct ctype *ctype);
 static struct token *attribute_exact_context(struct token *token, struct symbol *attr, struct ctype *ctype);
 static struct token *attribute_transparent_union(struct token *token, struct symbol *attr, struct ctype *ctype);
 static struct token *ignore_attribute(struct token *token, struct symbol *attr, struct ctype *ctype);
@@ -189,10 +188,6 @@ static struct symbol_op context_op = {
 	.attribute = attribute_context,
 };
 
-static struct symbol_op conditional_context_op = {
-	.attribute = attribute_conditional_context,
-};
-
 static struct symbol_op exact_context_op = {
 	.attribute = attribute_exact_context,
 };
@@ -279,7 +274,6 @@ static struct init_keyword {
 	{ "address_space",NS_KEYWORD,	.op = &address_space_op },
 	{ "mode",	NS_KEYWORD,	.op = &mode_op },
 	{ "context",	NS_KEYWORD,	.op = &context_op },
-	{ "conditional_context",	NS_KEYWORD,	.op = &conditional_context_op },
 	{ "exact_context",	NS_KEYWORD,	.op = &exact_context_op },
 	{ "__transparent_union__",	NS_KEYWORD,	.op = &transparent_union_op },
 
@@ -945,7 +939,6 @@ static struct token *_attribute_context(struct token *token, struct symbol *attr
 	}
 
 	context->exact = exact;
-	context->out_false = context->out;
 
 	if (argc)
 		add_ptr_list(&ctype->contexts, context);
@@ -964,51 +957,6 @@ static struct token *attribute_exact_context(struct token *token, struct symbol
 	return _attribute_context(token, attr, ctype, 1);
 }
 
-static struct token *attribute_conditional_context(struct token *token, struct symbol *attr, struct ctype *ctype)
-{
-	struct context *context = alloc_context();
-	struct expression *args[4];
-	int argc = 0;
-
-	token = expect(token, '(', "after conditional_context attribute");
-	while (!match_op(token, ')')) {
-		struct expression *expr = NULL;
-		token = conditional_expression(token, &expr);
-		if (!expr)
-			break;
-		if (argc < 4)
-			args[argc++] = expr;
-		else
-			argc++;
-		if (!match_op(token, ','))
-			break;
-		token = token->next;
-	}
-
-	switch(argc) {
-	case 3:
-		context->in = get_expression_value(args[0]);
-		context->out = get_expression_value(args[1]);
-		context->out_false = get_expression_value(args[2]);
-		break;
-	case 4:
-		context->context = args[0];
-		context->in = get_expression_value(args[1]);
-		context->out = get_expression_value(args[2]);
-		context->out_false = get_expression_value(args[3]);
-		break;
-	default:
-		sparse_error(token->pos, "invalid number of arguments to conditional_context attribute");
-		break;
-	}
-
-	if (argc)
-		add_ptr_list(&ctype->contexts, context);
-
-	token = expect(token, ')', "after conditional_context attribute");
-	return token;
-}
-
 static struct token *attribute_transparent_union(struct token *token, struct symbol *attr, struct ctype *ctype)
 {
 	if (Wtransparent_union)
diff --git a/sparse.1 b/sparse.1
index 61c40b8..ebe62f0 100644
--- a/sparse.1
+++ b/sparse.1
@@ -101,15 +101,6 @@ be required on both reads and writes) by using either the token "read" or
 the token "write" for an optional fourth argument:
 .BI __attribute__((context( expression , in_context , out_context , read|write )).
 
-To indicate that a certain function acquires a context depending on its
-return value, use
-.BI __attribute__((conditional_context( [expression ,] in_context , out_success , out_failure ))
-where \fIout_success\fR and \fIout_failure\fR indicate the context change
-done depending on success (non-zero) or failure (zero) return of the
-function. Note that currently, using this attribute on a function means that
-the function itself won't be checked for context handling at all. See the
-testsuite for examples.
-
 Sparse will warn when it sees a function change a
 context without indicating this with a \fBcontext\fR or \fBexact_context\fR attribute, either by
 decreasing a context below zero (such as by releasing a lock without acquiring
diff --git a/sparse.c b/sparse.c
index 1149244..76b1db2 100644
--- a/sparse.c
+++ b/sparse.c
@@ -25,7 +25,7 @@
 #include "linearize.h"
 
 struct context_check {
-	int val, val_false;
+	int val;
 	char name[32];
 };
 
@@ -44,7 +44,7 @@ static const char *context_name(struct context *context)
 }
 
 static void context_add(struct context_check_list **ccl, const char *name,
-			int offs, int offs_false)
+			int offs)
 {
 	struct context_check *check, *found = NULL;
 
@@ -62,7 +62,6 @@ static void context_add(struct context_check_list **ccl, const char *name,
 		add_ptr_list(ccl, found);
 	}
 	found->val += offs;
-	found->val_false += offs_false;
 }
 
 static int context_list_has(struct context_check_list *ccl,
@@ -73,12 +72,11 @@ static int context_list_has(struct context_check_list *ccl,
 	FOR_EACH_PTR(ccl, check) {
 		if (strcmp(c->name, check->name))
 			continue;
-		return check->val == c->val &&
-		       check->val_false == c->val_false;
+		return check->val == c->val;
 	} END_FOR_EACH_PTR(check);
 
 	/* not found is equal to 0 */
-	return c->val == 0 && c->val_false == 0;
+	return c->val == 0;
 }
 
 static int context_lists_equal(struct context_check_list *ccl1,
@@ -107,7 +105,7 @@ static struct context_check_list *checked_copy(struct context_check_list *ccl)
 	struct context_check *c;
 
 	FOR_EACH_PTR(ccl, c) {
-		context_add(&result, c->name, c->val_false, c->val_false);
+		context_add(&result, c->name, c->val);
 	} END_FOR_EACH_PTR(c);
 
 	return result;
@@ -140,7 +138,7 @@ static int context_list_check(struct entrypoint *ep, struct position pos,
 
 	/* make sure the loop below checks all */
 	FOR_EACH_PTR(ccl_target, c1) {
-		context_add(&ccl_cur, c1->name, 0, 0);
+		context_add(&ccl_cur, c1->name, 0);
 	} END_FOR_EACH_PTR(c1);
 
 	FOR_EACH_PTR(ccl_cur, c1) {
@@ -289,15 +287,14 @@ static int handle_context(struct entrypoint *ep, struct basic_block *bb,
 		return -1;
 	}
 
-	context_add(combined, name, insn->increment, insn->inc_false);
+	context_add(combined, name, insn->increment);
 
 	return 0;
 }
 
 static int check_bb_context(struct entrypoint *ep, struct basic_block *bb,
 			    struct context_check_list *ccl_in,
-			    struct context_check_list *ccl_target,
-			    int in_false)
+			    struct context_check_list *ccl_target)
 {
 	struct context_check_list *combined = NULL, *done;
 	struct context_check *c;
@@ -327,10 +324,7 @@ static int check_bb_context(struct entrypoint *ep, struct basic_block *bb,
 	 * for the conditional_context() attribute.
 	 */
 	FOR_EACH_PTR(ccl_in, c) {
-		if (in_false)
-			context_add(&combined, c->name, c->val_false, c->val_false);
-		else
-			context_add(&combined, c->name, c->val, c->val);
+		context_add(&combined, c->name, c->val);
 	} END_FOR_EACH_PTR(c);
 
 	/* Add the new context to the list of already-checked contexts */
@@ -356,18 +350,18 @@ static int check_bb_context(struct entrypoint *ep, struct basic_block *bb,
 		case OP_BR:
 			if (insn->bb_true)
 				if (check_bb_context(ep, insn->bb_true,
-						     combined, ccl_target, 0))
+						     combined, ccl_target))
 					goto out;
 			if (insn->bb_false)
 				if (check_bb_context(ep, insn->bb_false,
-						     combined, ccl_target, 1))
+						     combined, ccl_target))
 					goto out;
 			break;
 		case OP_SWITCH:
 		case OP_COMPUTEDGOTO:
 			FOR_EACH_PTR(insn->multijmp_list, mj) {
 				if (check_bb_context(ep, mj->target,
-					             combined, ccl_target, 0))
+					             combined, ccl_target))
 					goto out;
 			} END_FOR_EACH_PTR(mj);
 			break;
@@ -579,14 +573,11 @@ static void check_context(struct entrypoint *ep)
 	FOR_EACH_PTR(sym->ctype.contexts, context) {
 		const char *name = context_name(context);
 
-		context_add(&ccl_in, name, context->in, context->in);
-		context_add(&ccl_target, name, context->out, context->out_false);
-		/* we don't currently check the body of trylock functions */
-		if (context->out != context->out_false)
-			return;
+		context_add(&ccl_in, name, context->in);
+		context_add(&ccl_target, name, context->out);
 	} END_FOR_EACH_PTR(context);
 
-	check_bb_context(ep, ep->entry->bb, ccl_in, ccl_target, 0);
+	check_bb_context(ep, ep->entry->bb, ccl_in, ccl_target);
 	free_ptr_list(&ccl_in);
 	free_ptr_list(&ccl_target);
 	free_bb_context_lists(ep->entry->bb);
diff --git a/symbol.h b/symbol.h
index f79674f..4e605c2 100644
--- a/symbol.h
+++ b/symbol.h
@@ -77,7 +77,7 @@ enum context_read_write_specifier {
 
 struct context {
 	struct expression *context;
-	unsigned int in, out, out_false;
+	unsigned int in, out;
 	int exact;
 	enum context_read_write_specifier rws;
 };
diff --git a/validation/context-dynamic.c b/validation/context-dynamic.c
deleted file mode 100644
index 5e172f0..0000000
--- a/validation/context-dynamic.c
+++ /dev/null
@@ -1,171 +0,0 @@
-static void a(void) __attribute__ ((context(A, 0, 1)))
-{
-    __context__(A, 1);
-}
-
-static void r(void) __attribute__ ((context(A, 1, 0)))
-{
-    __context__(A, -1);
-}
-
-extern int condition, condition2;
-
-static int tl(void) __attribute__ ((conditional_context(A, 0, 1, 0)))
-{
-    if (condition) {
-        a();
-        return 1;
-    }
-    return 0;
-}
-
-static int tl2(void) __attribute__ ((conditional_context(A, 0, 0, 1)))
-{
-    if (condition) {
-        a();
-        return 1;
-    }
-    return 0;
-}
-
-static int dummy(void)
-{
-    return condition + condition2;
-}
-
-static int good_trylock1(void)
-{
-    if (tl()) {
-        r();
-    }
-}
-
-static int good_trylock2(void)
-{
-    if (tl()) {
-        r();
-    }
-
-    if (tl()) {
-        r();
-    }
-}
-static int good_trylock3(void)
-{
-    a();
-    if (tl()) {
-        r();
-    }
-    r();
-    if (tl()) {
-        r();
-    }
-}
-
-static int good_trylock4(void)
-{
-    a();
-    if (tl()) {
-        r();
-    }
-    if (tl()) {
-        r();
-    }
-    r();
-}
-
-static void bad_trylock1(void)
-{
-    a();
-    if (dummy()) {
-        r();
-    }
-    r();
-}
-
-static int good_trylock5(void)
-{
-    if (!tl2()) {
-        r();
-    }
-}
-
-static int good_trylock6(void)
-{
-    if (!tl2()) {
-        r();
-    }
-
-    if (!tl2()) {
-        r();
-    }
-}
-static int good_trylock7(void)
-{
-    a();
-    if (!tl2()) {
-        r();
-    }
-    r();
-    if (!tl2()) {
-        r();
-    }
-}
-
-static int good_trylock8(void)
-{
-    a();
-    if (!tl2()) {
-        r();
-    }
-    if (!tl2()) {
-        r();
-    }
-    r();
-}
-
-static void bad_trylock2(void)
-{
-    a();
-    if (!dummy()) {
-        r();
-    }
-    r();
-}
-
-static int good_switch(void)
-{
-    switch (condition) {
-    case 1:
-        a();
-        break;
-    case 2:
-        a();
-        break;
-    case 3:
-        a();
-        break;
-    default:
-        a();
-    }
-    r();
-}
-
-static void bad_lock1(void)
-{
-    r();
-    a();
-}
-
-/*
- * check-name: Check -Wcontext with lock trylocks
- *
- * check-error-start
-context-dynamic.c:83:6: warning: context problem in 'bad_trylock1': 'r' expected different context
-context-dynamic.c:83:6:    context 'A': wanted >= 1, got 0
-context-dynamic.c:133:6: warning: context problem in 'bad_trylock2': 'r' expected different context
-context-dynamic.c:133:6:    context 'A': wanted >= 1, got 0
-context-dynamic.c:156:6: warning: context problem in 'bad_lock1': 'r' expected different context
-context-dynamic.c:156:6:    context 'A': wanted >= 1, got 0
- * check-error-end
- */

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux