* [PATCH 0/9] context tracking updates
@ 2008-05-29 8:54 Johannes Berg
2008-05-29 8:54 ` [PATCH 1/9] add test for acquire/release Johannes Berg
` (11 more replies)
0 siblings, 12 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
This updates the context tracking, I just explained most of it in the mail
"Re: sparse context warning problem ..."
(message ID <1212050823.16917.37.camel@johannes.berg>)
johannes
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/9] add test for acquire/release
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 2/9] add __exact_context__ Johannes Berg
` (10 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 002-acq-rel-test.patch --]
[-- Type: text/plain, Size: 837 bytes --]
Test that giving
__attribute__((context(TEST,1,0)))
__attribute__((context(TEST,0,1)))
instead of
__attribute__((context(TEST,1,1)))
works.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
validation/context.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
--- sparse.orig/validation/context.c 2008-05-13 23:40:36.000000000 +0200
+++ sparse/validation/context.c 2008-05-14 00:58:33.000000000 +0200
@@ -380,6 +380,21 @@ static int warn_conditional(void)
return 0;
}
+static void good_require(void)
+__attribute__((context(TEST,1,0)))
+__attribute__((context(TEST,0,1)))
+{
+ __context__(TEST,-1);
+ __context__(TEST,1);
+}
+
+static void good_require_caller(void)
+{
+ __context__(TEST,1,0);
+ good_require();
+ __context__(TEST,-1,1);
+}
+
/*
* check-name: Check -Wcontext
*
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/9] add __exact_context__
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
2008-05-29 8:54 ` [PATCH 1/9] add test for acquire/release Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 3/9] allow context() attribute on variables Johannes Berg
` (9 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 003-exact-context.patch --]
[-- Type: text/plain, Size: 7234 bytes --]
We also need a statement to indicate that an exact context is
required, most notably the next patch will require it so that
it can translate attributes on variables into statements.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
ident-list.h | 1
linearize.c | 1
linearize.h | 2 -
parse.c | 20 ++++++++++++-
parse.h | 1
sparse.1 | 2 -
sparse.c | 14 ++++++---
validation/context-exact.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 101 insertions(+), 7 deletions(-)
--- sparse.orig/ident-list.h 2008-04-26 23:09:00.000000000 +0200
+++ sparse/ident-list.h 2008-04-26 23:09:05.000000000 +0200
@@ -96,6 +96,7 @@ __IDENT(__PRETTY_FUNCTION___ident, "__PR
/* Sparse commands */
IDENT_RESERVED(__context__);
+IDENT_RESERVED(__exact_context__);
IDENT_RESERVED(__range__);
/* Magic function names we recognize */
--- sparse.orig/linearize.c 2008-04-26 23:09:00.000000000 +0200
+++ sparse/linearize.c 2008-04-26 23:09:05.000000000 +0200
@@ -1681,6 +1681,7 @@ static pseudo_t linearize_context(struct
value = expr->value;
insn->required = value;
+ insn->exact = stmt->exact;
insn->context_expr = stmt->context;
add_one_insn(ep, insn);
--- sparse.orig/parse.c 2008-04-26 23:09:04.000000000 +0200
+++ sparse/parse.c 2008-04-26 23:09:05.000000000 +0200
@@ -52,6 +52,7 @@ static struct token *parse_while_stateme
static struct token *parse_do_statement(struct token *token, struct statement *stmt);
static struct token *parse_goto_statement(struct token *token, struct statement *stmt);
static struct token *parse_context_statement(struct token *token, struct statement *stmt);
+static struct token *parse_exact_context_statement(struct token *token, struct statement *stmt);
static struct token *parse_range_statement(struct token *token, struct statement *stmt);
static struct token *parse_asm_statement(struct token *token, struct statement *stmt);
static struct token *toplevel_asm_declaration(struct token *token, struct symbol_list **list);
@@ -149,6 +150,10 @@ static struct symbol_op __context___op =
.statement = parse_context_statement,
};
+static struct symbol_op __exact_context___op = {
+ .statement = parse_exact_context_statement,
+};
+
static struct symbol_op range_op = {
.statement = parse_range_statement,
};
@@ -254,6 +259,7 @@ static struct init_keyword {
{ "do", NS_KEYWORD, .op = &do_op },
{ "goto", NS_KEYWORD, .op = &goto_op },
{ "__context__",NS_KEYWORD, .op = &__context___op },
+ { "__exact_context__",NS_KEYWORD, .op = &__exact_context___op },
{ "__range__", NS_KEYWORD, .op = &range_op },
{ "asm", NS_KEYWORD, .op = &asm_op },
{ "__asm", NS_KEYWORD, .op = &asm_op },
@@ -1810,7 +1816,7 @@ static struct token *parse_goto_statemen
return expect(token, ';', "at end of statement");
}
-static struct token *parse_context_statement(struct token *token, struct statement *stmt)
+static struct token *_parse_context_statement(struct token *token, struct statement *stmt, int exact)
{
struct expression *args[3];
int argc = 0;
@@ -1835,6 +1841,8 @@ static struct token *parse_context_state
stmt->expression = args[0];
stmt->context = NULL;
+ stmt->exact = exact;
+
switch (argc) {
case 0:
sparse_error(token->pos, "__context__ statement needs argument(s)");
@@ -1864,6 +1872,16 @@ static struct token *parse_context_state
return expect(token, ')', "at end of __context__");
}
+static struct token *parse_context_statement(struct token *token, struct statement *stmt)
+{
+ return _parse_context_statement(token, stmt, 0);
+}
+
+static struct token *parse_exact_context_statement(struct token *token, struct statement *stmt)
+{
+ return _parse_context_statement(token, stmt, 1);
+}
+
static struct token *parse_range_statement(struct token *token, struct statement *stmt)
{
stmt->type = STMT_RANGE;
--- sparse.orig/sparse.1 2008-04-26 23:09:00.000000000 +0200
+++ sparse/sparse.1 2008-04-26 23:09:05.000000000 +0200
@@ -90,7 +90,7 @@ To indicate that a function requires
.BI exactly
a certain lock context (not "at least" as above), use the form
.BI __attribute__((exact_context( [expression ,] in_context , out_context ))
-There currently is no corresponding
+There is also the corresponding
.BI __exact_context__( [expression , ]adjust_value[ , required] )
statement.
--- sparse.orig/sparse.c 2008-04-26 23:09:00.000000000 +0200
+++ sparse/sparse.c 2008-04-26 23:09:05.000000000 +0200
@@ -239,7 +239,7 @@ static int handle_context(struct entrypo
struct context_check_list **combined)
{
struct context_check *c;
- const char *name;
+ const char *name, *cmp;
char *buf;
int val, ok;
@@ -256,7 +256,13 @@ static int handle_context(struct entrypo
}
} END_FOR_EACH_PTR(c);
- ok = insn->required <= val;
+ if (insn->exact) {
+ ok = insn->required == val;
+ cmp = "";
+ } else {
+ ok = insn->required <= val;
+ cmp = ">= ";
+ }
if (!ok && Wcontext) {
get_context_string(&buf, &name);
@@ -266,8 +272,8 @@ static int handle_context(struct entrypo
"__context__ statement expected different context",
show_ident(ep->name->ident));
- info(insn->pos, "%swanted >= %d, got %d",
- name, insn->required, val);
+ info(insn->pos, "%swanted %s%d, got %d",
+ name, cmp, insn->required, val);
free(buf);
return -1;
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ sparse/validation/context-exact.c 2008-04-26 23:09:05.000000000 +0200
@@ -0,0 +1,67 @@
+static void a(void) __attribute__((context(TEST,0,1)))
+{
+ __context__(TEST,1);
+}
+
+static void r(void) __attribute__((context(TEST,1,0)))
+{
+ __context__(TEST,-1,1);
+}
+
+static void good_1(void)
+{
+ a();
+ r();
+}
+
+static void good_2(void)
+{
+ a();
+ r();
+ a();
+ r();
+}
+
+static void good_3(void)
+{
+ a();
+ a();
+ r();
+ r();
+}
+
+static void good_4(void)
+{
+ a();
+ a();
+ __context__(TEST,0,1);
+ r();
+ r();
+}
+
+static void warn_1(void)
+{
+ a();
+ a();
+ __exact_context__(TEST,0,1);
+ r();
+ r();
+}
+
+static void good_5(void)
+{
+ a();
+ a();
+ __exact_context__(TEST,0,2);
+ r();
+ r();
+}
+
+/*
+ * check-name: Check __exact_context__ statement with required context
+ *
+ * check-error-start
+context-exact.c:46:2: warning: context imbalance in 'warn_1': __context__ statement expected different context
+context-exact.c:46:2: context 'TEST': wanted 1, got 2
+ * check-error-end
+ */
--- sparse.orig/linearize.h 2008-04-26 23:09:00.000000000 +0200
+++ sparse/linearize.h 2008-04-26 23:09:05.000000000 +0200
@@ -116,7 +116,7 @@ struct instruction {
struct pseudo_list *arguments;
};
struct /* context */ {
- int increment, required, inc_false;
+ int increment, required, inc_false, exact;
struct expression *context_expr;
};
struct /* asm */ {
--- sparse.orig/parse.h 2008-04-26 23:09:00.000000000 +0200
+++ sparse/parse.h 2008-04-26 23:09:05.000000000 +0200
@@ -43,6 +43,7 @@ struct statement {
struct expression *expression;
struct expression *context;
struct expression *required;
+ int exact;
};
struct /* return_statement */ {
struct expression *ret_value;
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/9] allow context() attribute on variables
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
2008-05-29 8:54 ` [PATCH 1/9] add test for acquire/release Johannes Berg
2008-05-29 8:54 ` [PATCH 2/9] add __exact_context__ Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 4/9] evaluate/expand context expressions Johannes Berg
` (8 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 004-context-on-vars.patch --]
[-- Type: text/plain, Size: 17452 bytes --]
This patch makes it possible to add the context attribute on
variables, to warn for example in this case:
struct something {
int x __attribute__((context(L,1,1)));
};
extern struct something *s;
static void warn_access13(void)
{
s->x = 7;
}
This is achieved by translating the context attribute on
variables that are loaded from/stored to into a context
expression internally. A number of tests are included,
including tests for a struct member (as above) and array
accesses.
To distinguish between reads and writes (the default is to
check both) use the fourth parameter to the context attribute:
__attribute__((context(L,1,1,read)))
or
__attribute__((context(L,1,1,write)))
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
The exact_context() attribute is also allowed so we have the
same max/no-max semantics as functions.
linearize.c | 92 ++++++++++++++++-
linearize.h | 1
parse.c | 25 ++++
sparse.1 | 7 +
sparse.c | 18 ++-
symbol.h | 7 +
validation/context-exact.c | 2
validation/context-on-vars.c | 219 +++++++++++++++++++++++++++++++++++++++++
validation/context-statement.c | 6 -
9 files changed, 365 insertions(+), 12 deletions(-)
--- sparse.orig/sparse.c 2008-04-26 23:09:05.000000000 +0200
+++ sparse/sparse.c 2008-04-26 23:09:27.000000000 +0200
@@ -114,6 +114,7 @@ static struct context_check_list *checke
}
#define IMBALANCE_IN "context imbalance in '%s': "
+#define CONTEXT_PROB "context problem in '%s': "
#define DEFAULT_CONTEXT_DESCR " default context: "
static void get_context_string(char **buf, const char **name)
@@ -267,10 +268,19 @@ static int handle_context(struct entrypo
if (!ok && Wcontext) {
get_context_string(&buf, &name);
- warning(insn->pos,
- IMBALANCE_IN
- "__context__ statement expected different context",
- show_ident(ep->name->ident));
+ if (insn->access_var) {
+ char *symname = strdup(show_ident(insn->access_var->ident));
+ warning(insn->pos,
+ CONTEXT_PROB
+ "access to '%s' requires different context",
+ show_ident(ep->name->ident), symname);
+ free(symname);
+ } else {
+ warning(insn->pos,
+ CONTEXT_PROB
+ "__context__ statement expected different context",
+ show_ident(ep->name->ident));
+ }
info(insn->pos, "%swanted %s%d, got %d",
name, cmp, insn->required, val);
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ sparse/validation/context-on-vars.c 2008-04-26 23:37:36.000000000 +0200
@@ -0,0 +1,219 @@
+static void a(void) __attribute__((context(L,0,1)))
+{
+ __context__(L,1);
+}
+
+static void r(void) __attribute__((context(L,1,0)))
+{
+ __context__(L,-1);
+}
+
+static int nl_int __attribute__((context(L,1,1)));
+static int nl_array[100] __attribute__((context(L,1,1)));
+extern int condition;
+
+static void warn_access1(void)
+{
+ nl_int = 7;
+}
+
+static void warn_access2(void)
+{
+ nl_int++;
+}
+
+static void warn_access3(void)
+{
+ if (condition)
+ nl_int++;
+}
+
+static void warn_access4(void)
+{
+ condition -= nl_int;
+}
+
+static void warn_access5(void)
+{
+ int x = condition ? nl_int : 0;
+}
+
+static void warn_access6(void)
+{
+ if (!nl_int) {
+ condition = 1;
+ }
+}
+
+static void warn_access7(void)
+{
+ if (nl_int) {
+ condition = 1;
+ }
+}
+
+static int *warn_access8(void)
+{
+ return &nl_int;
+}
+
+static void warn_access9(void)
+{
+ (void*)nl_int;
+}
+
+static void warn_access10(void)
+{
+ nl_array[7]++;
+}
+
+static void good_access1(void)
+{
+ a();
+ nl_int = 7;
+ r();
+}
+
+static void good_access1(void)
+{
+ if (condition) {
+ a();
+ nl_int = 7;
+ r();
+ }
+}
+
+static void good_access3(void)
+{
+ /* tests more our ability to optimise things out ... */
+ int x = 0 ? nl_int : 0;
+}
+
+static int *good_access4(void)
+{
+ return &nl_int;
+}
+
+struct something {
+ int a;
+ int b;
+};
+
+extern struct something *s __attribute__((context(L,1,1)));
+
+static void warn_access11(void)
+{
+ s->b = 7;
+}
+
+struct something2 {
+ int a;
+ int b __attribute__((context(L,1,1)));
+};
+
+extern struct something2 *s2;
+extern int lx __attribute__((context(L,1,1)));
+
+static void warn_access12(void)
+{
+ s2->b = lx;
+}
+
+static void warn_access13(void)
+{
+ s2->b = 7;
+}
+
+static void good_1(void)
+{
+ a();
+ s2->b = 7;
+ r();
+}
+
+static void good_2(void)
+{
+ a();
+ a();
+ s2->b = 8;
+ r();
+ r();
+}
+
+struct something3 {
+ int a;
+ int b __attribute__((exact_context(L,1,1)));
+};
+
+extern struct something3 *s3;
+
+static void warn_exact1(void)
+{
+ a();
+ a();
+ s3->b = 8;
+ r();
+ r();
+}
+
+extern int x __attribute__((context(L,1,1,read)));
+extern int y __attribute__((context(L,1,1,write)));
+
+static void good_3(void)
+{
+ a();
+ y = x;
+ r();
+}
+
+static void good_4(void)
+{
+ x = y;
+}
+
+static void warn_access14(void)
+{
+ x;
+}
+
+static void warn_access15(void)
+{
+ y = 7;
+}
+
+/*
+ * check-name: Check -Wcontext for variables
+ *
+ * check-error-start
+context-on-vars.c:17:14: warning: context problem in 'warn_access1': access to 'nl_int' requires different context
+context-on-vars.c:17:14: context 'L': wanted >= 1, got 0
+context-on-vars.c:22:11: warning: context problem in 'warn_access2': access to 'nl_int' requires different context
+context-on-vars.c:22:11: context 'L': wanted >= 1, got 0
+context-on-vars.c:28:15: warning: context problem in 'warn_access3': access to 'nl_int' requires different context
+context-on-vars.c:28:15: context 'L': wanted >= 1, got 0
+context-on-vars.c:33:18: warning: context problem in 'warn_access4': access to 'nl_int' requires different context
+context-on-vars.c:33:18: context 'L': wanted >= 1, got 0
+context-on-vars.c:38:25: warning: context problem in 'warn_access5': access to 'nl_int' requires different context
+context-on-vars.c:38:25: context 'L': wanted >= 1, got 0
+context-on-vars.c:43:10: warning: context problem in 'warn_access6': access to 'nl_int' requires different context
+context-on-vars.c:43:10: context 'L': wanted >= 1, got 0
+context-on-vars.c:50:9: warning: context problem in 'warn_access7': access to 'nl_int' requires different context
+context-on-vars.c:50:9: context 'L': wanted >= 1, got 0
+context-on-vars.c:62:12: warning: context problem in 'warn_access9': access to 'nl_int' requires different context
+context-on-vars.c:62:12: context 'L': wanted >= 1, got 0
+context-on-vars.c:67:16: warning: context problem in 'warn_access10': access to 'nl_array' requires different context
+context-on-vars.c:67:16: context 'L': wanted >= 1, got 0
+context-on-vars.c:106:5: warning: context problem in 'warn_access11': access to 's' requires different context
+context-on-vars.c:106:5: context 'L': wanted >= 1, got 0
+context-on-vars.c:119:13: warning: context problem in 'warn_access12': access to 'lx' requires different context
+context-on-vars.c:119:13: context 'L': wanted >= 1, got 0
+context-on-vars.c:124:5: warning: context problem in 'warn_access13': access to 'b' requires different context
+context-on-vars.c:124:5: context 'L': wanted >= 1, got 0
+context-on-vars.c:154:5: warning: context problem in 'warn_exact1': access to 'b' requires different context
+context-on-vars.c:154:5: context 'L': wanted 1, got 2
+context-on-vars.c:176:3: warning: context problem in 'warn_access14': access to 'x' requires different context
+context-on-vars.c:176:3: context 'L': wanted >= 1, got 0
+context-on-vars.c:181:7: warning: context problem in 'warn_access15': access to 'y' requires different context
+context-on-vars.c:181:7: context 'L': wanted >= 1, got 0
+ * check-error-end
+ */
--- sparse.orig/linearize.c 2008-04-26 23:09:05.000000000 +0200
+++ sparse/linearize.c 2008-04-26 23:37:56.000000000 +0200
@@ -30,7 +30,6 @@ static pseudo_t add_setval(struct entryp
static pseudo_t linearize_one_symbol(struct entrypoint *ep, struct symbol *sym);
struct access_data;
-static pseudo_t add_load(struct entrypoint *ep, struct access_data *);
static pseudo_t linearize_initializer(struct entrypoint *ep, struct expression *initializer, struct access_data *);
struct pseudo void_pseudo = {};
@@ -935,6 +934,8 @@ static pseudo_t linearize_store_gen(stru
pseudo_t value,
struct access_data *ad)
{
+ struct context *context;
+ struct instruction *insn;
pseudo_t store = value;
if (type_size(ad->source_type) != type_size(ad->result_type)) {
@@ -950,6 +951,49 @@ static pseudo_t linearize_store_gen(stru
store = add_binary_op(ep, ad->source_type, OP_OR, orig, store);
}
add_store(ep, ad, store);
+
+ FOR_EACH_PTR(ad->source_type->ctype.contexts, context) {
+ if (context->rws != CTX_RWS_BOTH &&
+ context->rws != CTX_RWS_WRITE)
+ continue;
+ insn = alloc_instruction(OP_CONTEXT, 0);
+ insn->required = context->in;
+ insn->increment = insn->inc_false = context->out - context->in;
+ insn->context_expr = context->context;
+ insn->access_var = ad->source_type;
+ insn->exact = context->exact;
+ add_one_insn(ep, insn);
+ } END_FOR_EACH_PTR(context);
+
+ FOR_EACH_PTR(ad->result_type->ctype.contexts, context) {
+ if (context->rws != CTX_RWS_BOTH &&
+ context->rws != CTX_RWS_WRITE)
+ continue;
+ insn = alloc_instruction(OP_CONTEXT, 0);
+ insn->required = context->in;
+ insn->increment = insn->inc_false = context->out - context->in;
+ insn->context_expr = context->context;
+ insn->access_var = ad->result_type;
+ insn->exact = context->exact;
+ add_one_insn(ep, insn);
+ } END_FOR_EACH_PTR(context);
+
+ if (ad->address->type == PSEUDO_SYM &&
+ ad->address->sym->namespace & NS_SYMBOL) {
+ FOR_EACH_PTR(ad->address->sym->ctype.contexts, context) {
+ if (context->rws != CTX_RWS_BOTH &&
+ context->rws != CTX_RWS_WRITE)
+ continue;
+ insn = alloc_instruction(OP_CONTEXT, 0);
+ insn->required = context->in;
+ insn->increment = insn->inc_false = context->out - context->in;
+ insn->context_expr = context->context;
+ insn->access_var = ad->address->sym;
+ insn->exact = context->exact;
+ add_one_insn(ep, insn);
+ } END_FOR_EACH_PTR(context);
+ }
+
return value;
}
@@ -987,6 +1031,8 @@ static pseudo_t add_symbol_address(struc
static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad)
{
+ struct context *context;
+ struct instruction *insn;
pseudo_t new = add_load(ep, ad);
if (ad->bit_offset) {
@@ -994,7 +1040,49 @@ static pseudo_t linearize_load_gen(struc
pseudo_t newval = add_binary_op(ep, ad->source_type, OP_LSR, new, shift);
new = newval;
}
-
+
+ FOR_EACH_PTR(ad->source_type->ctype.contexts, context) {
+ if (context->rws != CTX_RWS_BOTH &&
+ context->rws != CTX_RWS_READ)
+ continue;
+ insn = alloc_instruction(OP_CONTEXT, 0);
+ insn->required = context->in;
+ insn->increment = insn->inc_false = context->out - context->in;
+ insn->context_expr = context->context;
+ insn->access_var = ad->source_type;
+ insn->exact = context->exact;
+ add_one_insn(ep, insn);
+ } END_FOR_EACH_PTR(context);
+
+ FOR_EACH_PTR(ad->result_type->ctype.contexts, context) {
+ if (context->rws != CTX_RWS_BOTH &&
+ context->rws != CTX_RWS_READ)
+ continue;
+ insn = alloc_instruction(OP_CONTEXT, 0);
+ insn->required = context->in;
+ insn->increment = insn->inc_false = context->out - context->in;
+ insn->context_expr = context->context;
+ insn->access_var = ad->result_type;
+ insn->exact = context->exact;
+ add_one_insn(ep, insn);
+ } END_FOR_EACH_PTR(context);
+
+ if (ad->address->type == PSEUDO_SYM &&
+ ad->address->sym->namespace & NS_SYMBOL) {
+ FOR_EACH_PTR(ad->address->sym->ctype.contexts, context) {
+ if (context->rws != CTX_RWS_BOTH &&
+ context->rws != CTX_RWS_READ)
+ continue;
+ insn = alloc_instruction(OP_CONTEXT, 0);
+ insn->required = context->in;
+ insn->increment = insn->inc_false = context->out - context->in;
+ insn->context_expr = context->context;
+ insn->access_var = ad->address->sym;
+ insn->exact = context->exact;
+ add_one_insn(ep, insn);
+ } END_FOR_EACH_PTR(context);
+ }
+
return new;
}
--- sparse.orig/linearize.h 2008-04-26 23:09:05.000000000 +0200
+++ sparse/linearize.h 2008-04-26 23:09:27.000000000 +0200
@@ -118,6 +118,7 @@ struct instruction {
struct /* context */ {
int increment, required, inc_false, exact;
struct expression *context_expr;
+ struct symbol *access_var;
};
struct /* asm */ {
const char *string;
--- sparse.orig/validation/context-statement.c 2008-04-26 23:08:59.000000000 +0200
+++ sparse/validation/context-statement.c 2008-04-26 23:09:06.000000000 +0200
@@ -59,11 +59,11 @@ static void bad_macro3(void)
* check-error-start
context-statement.c:16:8: warning: context imbalance in 'bad_arr': unexpected unlock
context-statement.c:16:8: context 'LOCK': wanted 0, got -1
-context-statement.c:38:5: warning: context imbalance in 'bad_macro1': __context__ statement expected different context
+context-statement.c:38:5: warning: context problem in 'bad_macro1': __context__ statement expected different context
context-statement.c:38:5: context 'LOCK': wanted >= 1, got 0
-context-statement.c:47:5: warning: context imbalance in 'bad_macro2': __context__ statement expected different context
+context-statement.c:47:5: warning: context problem in 'bad_macro2': __context__ statement expected different context
context-statement.c:47:5: context 'LOCK': wanted >= 1, got 0
-context-statement.c:53:5: warning: context imbalance in 'bad_macro3': __context__ statement expected different context
+context-statement.c:53:5: warning: context problem in 'bad_macro3': __context__ statement expected different context
context-statement.c:53:5: context 'LOCK': wanted >= 0, got -1
* check-error-end
*/
--- sparse.orig/sparse.1 2008-04-26 23:09:05.000000000 +0200
+++ sparse/sparse.1 2008-04-26 23:42:50.000000000 +0200
@@ -94,6 +94,13 @@ There is also the corresponding
.BI __exact_context__( [expression , ]adjust_value[ , required] )
statement.
+Both these can also be added to variable accesses but it is not recommended
+to make variable accesses modify the context. For variables, it is possible
+to distinguish between reads and writes (the regular context attribute will
+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 ))
--- sparse.orig/validation/context-exact.c 2008-04-26 23:09:05.000000000 +0200
+++ sparse/validation/context-exact.c 2008-04-26 23:09:06.000000000 +0200
@@ -61,7 +61,7 @@ static void good_5(void)
* check-name: Check __exact_context__ statement with required context
*
* check-error-start
-context-exact.c:46:2: warning: context imbalance in 'warn_1': __context__ statement expected different context
+context-exact.c:46:2: warning: context problem in 'warn_1': __context__ statement expected different context
context-exact.c:46:2: context 'TEST': wanted 1, got 2
* check-error-end
*/
--- sparse.orig/parse.c 2008-04-26 23:37:51.000000000 +0200
+++ sparse/parse.c 2008-04-26 23:39:10.000000000 +0200
@@ -883,16 +883,18 @@ static struct token *attribute_mode(stru
static struct token *_attribute_context(struct token *token, struct symbol *attr, struct ctype *ctype, int exact)
{
struct context *context = alloc_context();
- struct expression *args[3];
+ struct expression *args[4];
int argc = 0;
+ struct token *last = NULL;
token = expect(token, '(', "after context attribute");
while (!match_op(token, ')')) {
struct expression *expr = NULL;
+ last = token;
token = conditional_expression(token, &expr);
if (!expr)
break;
- if (argc < 3)
+ if (argc < 4)
args[argc++] = expr;
else
argc++;
@@ -917,6 +919,25 @@ static struct token *_attribute_context(
context->in = get_expression_value(args[1]);
context->out = get_expression_value(args[2]);
break;
+ case 4: {
+ const char *rw;
+ context->context = args[0];
+ context->in = get_expression_value(args[1]);
+ context->out = get_expression_value(args[2]);
+
+ if (last && token_type(last) == TOKEN_IDENT)
+ rw = show_token(last);
+ else
+ rw = NULL;
+
+ if (rw && strcmp(rw, "read") == 0)
+ context->rws = CTX_RWS_READ;
+ else if (rw && strcmp(rw, "write") == 0)
+ context->rws = CTX_RWS_WRITE;
+ else
+ sparse_error(last->pos, "invalid read/write specifier");
+ break;
+ }
default:
sparse_error(token->pos, "too many arguments to context attribute");
break;
--- sparse.orig/symbol.h 2008-04-26 23:37:51.000000000 +0200
+++ sparse/symbol.h 2008-04-26 23:37:56.000000000 +0200
@@ -69,10 +69,17 @@ enum keyword {
KW_MODE = 1 << 7,
};
+enum context_read_write_specifier {
+ CTX_RWS_BOTH = 0,
+ CTX_RWS_READ,
+ CTX_RWS_WRITE,
+};
+
struct context {
struct expression *context;
unsigned int in, out, out_false;
int exact;
+ enum context_read_write_specifier rws;
};
extern struct context *alloc_context(void);
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/9] evaluate/expand context expressions
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (2 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 3/9] allow context() attribute on variables Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 5/9] revert the conditional_context patch Johannes Berg
` (7 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 005-bugfix-context-stmt.patch --]
[-- Type: text/plain, Size: 6552 bytes --]
But still allow having a standalone symbol as the context
expression, also evaluate the context change/requirement
directly in the parser and pass them up as integers. Also
fixes a number of bugs e.g. in the expression copier and
a segfault when the default context is used as such:
__context__(1,1);
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
evaluate.c | 9 ++++++++-
expand.c | 2 +-
inline.c | 19 ++++++++++++-------
linearize.c | 17 +++--------------
parse.c | 22 +++++++++++++---------
parse.h | 6 ++++--
validation/context.c | 28 ++++++++++++++++++++++++++++
7 files changed, 69 insertions(+), 34 deletions(-)
--- sparse.orig/evaluate.c 2008-05-11 02:41:01.000000000 +0200
+++ sparse/evaluate.c 2008-05-29 10:21:27.000000000 +0200
@@ -3356,7 +3356,14 @@ struct symbol *evaluate_statement(struct
evaluate_asm_statement(stmt);
return NULL;
case STMT_CONTEXT:
- evaluate_expression(stmt->expression);
+ /*
+ * If this is an unknown symbol accept it as-is
+ * as a context name.
+ */
+ if (stmt->context &&
+ (stmt->context->type != EXPR_SYMBOL ||
+ stmt->context->symbol))
+ evaluate_expression(stmt->context);
return NULL;
case STMT_RANGE:
evaluate_expression(stmt->range_expression);
--- sparse.orig/expand.c 2008-05-11 02:41:01.000000000 +0200
+++ sparse/expand.c 2008-05-29 10:21:27.000000000 +0200
@@ -1169,7 +1169,7 @@ static int expand_statement(struct state
/* FIXME! Do the asm parameter evaluation! */
break;
case STMT_CONTEXT:
- expand_expression(stmt->expression);
+ expand_expression(stmt->context);
break;
case STMT_RANGE:
expand_expression(stmt->range_expression);
--- sparse.orig/inline.c 2008-05-11 02:41:00.000000000 +0200
+++ sparse/inline.c 2008-05-29 10:21:27.000000000 +0200
@@ -328,7 +328,18 @@ static struct statement *copy_one_statem
stmt = newstmt;
break;
}
- case STMT_CONTEXT:
+ case STMT_CONTEXT: {
+ struct expression *expr = copy_expression(stmt->context);
+ struct statement *newstmt;
+ if (expr == stmt->context)
+ break;
+ newstmt = dup_statement(stmt);
+ newstmt->context = expr;
+ newstmt->change = stmt->change;
+ newstmt->required = stmt->required;
+ stmt = newstmt;
+ break;
+ }
case STMT_EXPRESSION: {
struct expression *expr = copy_expression(stmt->expression);
struct statement *newstmt;
@@ -336,12 +347,6 @@ static struct statement *copy_one_statem
break;
newstmt = dup_statement(stmt);
newstmt->expression = expr;
- if (stmt->required) {
- expr = copy_expression(stmt->required);
- if (expr == stmt->required)
- break;
- newstmt->required = expr;
- }
stmt = newstmt;
break;
}
--- sparse.orig/linearize.c 2008-05-29 10:15:40.000000000 +0200
+++ sparse/linearize.c 2008-05-29 10:21:28.000000000 +0200
@@ -1753,22 +1753,11 @@ static pseudo_t linearize_inlined_call(s
static pseudo_t linearize_context(struct entrypoint *ep, struct statement *stmt)
{
struct instruction *insn = alloc_instruction(OP_CONTEXT, 0);
- struct expression *expr = stmt->expression;
- int value = 0;
- if (expr->type == EXPR_VALUE)
- value = expr->value;
+ insn->increment = stmt->change;
+ insn->inc_false = stmt->change;
- insn->increment = value;
- insn->inc_false = value;
-
- expr = stmt->required;
- value = 0;
-
- if (expr && expr->type == EXPR_VALUE)
- value = expr->value;
-
- insn->required = value;
+ insn->required = stmt->required;
insn->exact = stmt->exact;
insn->context_expr = stmt->context;
--- sparse.orig/parse.c 2008-05-29 10:15:40.000000000 +0200
+++ sparse/parse.c 2008-05-29 10:21:28.000000000 +0200
@@ -1859,9 +1859,7 @@ static struct token *_parse_context_stat
token = token->next;
}
- stmt->expression = args[0];
stmt->context = NULL;
-
stmt->exact = exact;
switch (argc) {
@@ -1869,21 +1867,27 @@ static struct token *_parse_context_stat
sparse_error(token->pos, "__context__ statement needs argument(s)");
return token;
case 1:
- /* already done */
+ stmt->change = get_expression_value(args[0]);
break;
case 2:
- if (args[0]->type != STMT_EXPRESSION) {
+ /*
+ * We should actually check whether we can evalulate
+ * it as a constant expression and if so use as the
+ * 'change' value. I hope nobody gives a calculation
+ * for the number.
+ */
+ if (args[0]->type != EXPR_VALUE) {
stmt->context = args[0];
- stmt->expression = args[1];
+ stmt->change = get_expression_value(args[1]);
} else {
- stmt->expression = args[0];
- stmt->required = args[1];
+ stmt->change = get_expression_value(args[0]);
+ stmt->required = get_expression_value(args[1]);
}
break;
case 3:
stmt->context = args[0];
- stmt->expression = args[1];
- stmt->required = args[2];
+ stmt->change = get_expression_value(args[1]);
+ stmt->required = get_expression_value(args[2]);
break;
default:
sparse_error(token->pos, "too many arguments for __context__ statement");
--- sparse.orig/parse.h 2008-05-29 10:15:40.000000000 +0200
+++ sparse/parse.h 2008-05-29 10:21:27.000000000 +0200
@@ -39,10 +39,12 @@ struct statement {
struct symbol *label;
struct statement *label_statement;
};
- struct { /* __context__ */
+ struct { /* expression */
struct expression *expression;
+ };
+ struct { /* __context__ */
struct expression *context;
- struct expression *required;
+ int change, required;
int exact;
};
struct /* return_statement */ {
--- sparse.orig/validation/context.c 2008-05-29 10:14:30.000000000 +0200
+++ sparse/validation/context.c 2008-05-29 10:18:23.000000000 +0200
@@ -395,6 +395,32 @@ static void good_require_caller(void)
__context__(TEST,-1,1);
}
+static void areq(void) __attribute__((context(1,2)))
+{
+ __context__(1,1);
+}
+
+static void good_reqlock(void)
+{
+ a();
+ areq();
+ r();
+ r();
+}
+
+static void warn_reqlock(void)
+{
+ areq();
+ r();
+}
+
+
+static void dummy1(void) __attribute__((context(p,0,1)))
+{
+ void *p;
+ __context__(p,1);
+}
+
/*
* check-name: Check -Wcontext
*
@@ -433,5 +459,7 @@ context.c:360:10: warning: context probl
context.c:360:10: default context: wanted >= 1, got 0
context.c:380:12: warning: context imbalance in 'warn_conditional': wrong count at exit
context.c:380:12: default context: wanted 0, got 1
+context.c:413:6: warning: context problem in 'warn_reqlock': 'areq' expected different context
+context.c:413:6: default context: wanted >= 1, got 0
* check-error-end
*/
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/9] revert the conditional_context patch
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (3 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 4/9] evaluate/expand context expressions Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 6/9] check context expressions as expressions Johannes Berg
` (6 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 006-remove-dynamic.patch --]
[-- Type: text/plain, Size: 16398 bytes --]
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@sipsolutions.net>
---
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(-)
--- sparse.orig/linearize.c 2008-04-27 03:10:44.000000000 +0200
+++ sparse/linearize.c 2008-04-27 03:16:56.000000000 +0200
@@ -437,7 +437,7 @@ const char *show_instruction(struct inst
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));
@@ -958,7 +958,7 @@ static pseudo_t linearize_store_gen(stru
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;
@@ -971,7 +971,7 @@ static pseudo_t linearize_store_gen(stru
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;
@@ -986,7 +986,7 @@ static pseudo_t linearize_store_gen(stru
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;
@@ -1047,7 +1047,7 @@ static pseudo_t linearize_load_gen(struc
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;
@@ -1060,7 +1060,7 @@ static pseudo_t linearize_load_gen(struc
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;
@@ -1075,7 +1075,7 @@ static pseudo_t linearize_load_gen(struc
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;
@@ -1320,12 +1320,21 @@ static pseudo_t linearize_call_expressio
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);
@@ -1755,7 +1764,6 @@ static pseudo_t linearize_context(struct
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;
--- sparse.orig/parse.c 2008-04-27 03:10:44.000000000 +0200
+++ sparse/parse.c 2008-04-27 03:10:52.000000000 +0200
@@ -65,7 +65,6 @@ static struct token *attribute_address_s
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 },
@@ -944,7 +938,6 @@ static struct token *_attribute_context(
}
context->exact = exact;
- context->out_false = context->out;
if (argc)
add_ptr_list(&ctype->contexts, context);
@@ -963,51 +956,6 @@ static struct token *attribute_exact_con
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)
--- sparse.orig/sparse.1 2008-04-27 03:10:44.000000000 +0200
+++ sparse/sparse.1 2008-04-27 03:10:52.000000000 +0200
@@ -101,15 +101,6 @@ be required on both reads and writes) by
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
--- sparse.orig/sparse.c 2008-04-27 03:10:44.000000000 +0200
+++ sparse/sparse.c 2008-04-27 03:14:26.000000000 +0200
@@ -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 c
}
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_c
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 conte
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 *checke
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 ent
/* 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 entrypo
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 entry
* 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 entry
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 entrypo
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);
--- sparse.orig/symbol.h 2008-04-27 03:10:44.000000000 +0200
+++ sparse/symbol.h 2008-04-27 03:10:52.000000000 +0200
@@ -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;
};
--- sparse.orig/validation/context-dynamic.c 2008-04-27 03:10:44.000000000 +0200
+++ /dev/null 1970-01-01 00:00:00.000000000 +0000
@@ -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
- */
--- sparse.orig/linearize.h 2008-04-27 03:10:44.000000000 +0200
+++ sparse/linearize.h 2008-04-27 03:11:58.000000000 +0200
@@ -116,7 +116,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;
};
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/9] check context expressions as expressions
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (4 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 5/9] revert the conditional_context patch Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-09-10 7:33 ` [PATCH 6/9 v2] " Johannes Berg
2008-05-29 8:54 ` [PATCH 7/9] test conditional result locking Johannes Berg
` (5 subsequent siblings)
11 siblings, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 007-context-expressions.patch --]
[-- Type: text/plain, Size: 32597 bytes --]
This patch makes sparse evaluate context expressions allowing this:
struct test {
lock_t lock;
int i;
};
extern void r(struct test *t) __attribute__((context(&t->lock,0,1)));
extern struct test *find(int i) __attribute__((context(&RESULT->lock,1,0)));
void test(void)
{
struct test *x = find(42);
r(x);
r(x);
}
to work the way you think it would.
This works by rewriting the given attribute expression into __context__
statements and evaluating those in the caller scope.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Somewhat inspired by Philipp's patch. When it warns, the expression is
given as evaluated, e.g. in above case would result in a warning about
"**x+0". We can fix that later by keeping the original expression and
not expanding/evaluating it, but we need to do both for it to work.
Note that there's a problem: You cannot give RESULT on a static function
that is declared there without getting an error in the function itself...
evaluate.c | 18 ++-
expand.c | 6 +
expression.c | 272 +++++++++++++++++++++++++++++++++++++++++++++-
expression.h | 7 +
ident-list.h | 1
inline.c | 4
linearize.c | 63 ++++++----
linearize.h | 1
parse.c | 17 ++
parse.h | 1
sparse.c | 144 ++++++------------------
symbol.h | 4
validation/context-vars.c | 171 ++++++++++++++++++++++++++++
13 files changed, 575 insertions(+), 134 deletions(-)
--- sparse.orig/expression.c 2008-04-27 10:45:03.000000000 +0200
+++ sparse/expression.c 2008-04-27 11:05:30.000000000 +0200
@@ -27,6 +27,8 @@
#include "expression.h"
#include "target.h"
+struct expression *current_assignment_expression = NULL;
+
static int match_oplist(int op, ...)
{
va_list args;
@@ -509,6 +511,63 @@ static struct token *expression_list(str
return token;
}
+static int ident_equal(struct ident *ident1, struct ident *ident2)
+{
+ if (ident1 == ident2)
+ return 1;
+ if (!ident1 || !ident2)
+ return 0;
+
+ return ident1->len == ident2->len &&
+ !strncmp(ident1->name, ident2->name, ident1->len);
+}
+
+/* TODO: this is probably not complete */
+void replace_ident(struct expression **_in, struct ident *s, struct expression *r)
+{
+ struct expression *in = *_in;
+
+ switch (in->type) {
+ case EXPR_SYMBOL:
+ if (ident_equal(in->symbol_name, s))
+ *_in = r;
+ break;
+ case EXPR_IDENTIFIER:
+ if (ident_equal(in->expr_ident, s))
+ *_in = r;
+ break;
+ case EXPR_BINOP:
+ case EXPR_COMMA:
+ case EXPR_ASSIGNMENT:
+ case EXPR_COMPARE:
+ case EXPR_LOGICAL:
+ replace_ident(&in->left, s, r);
+ replace_ident(&in->right, s, r);
+ break;
+ case EXPR_DEREF:
+ replace_ident(&in->deref, s, r);
+ break;
+ case EXPR_PREOP:
+ case EXPR_POSTOP:
+ replace_ident(&in->unop, s, r);
+ break;
+ case EXPR_CAST:
+ case EXPR_FORCE_CAST:
+ case EXPR_IMPLIED_CAST:
+ case EXPR_SIZEOF:
+ replace_ident(&in->cast_expression, s, r);
+ break;
+ case EXPR_SLICE:
+ replace_ident(&in->base, s, r);
+ break;
+ case EXPR_CONDITIONAL:
+ replace_ident(&in->conditional, s, r);
+ replace_ident(&in->cond_true, s, r);
+ replace_ident(&in->cond_false, s, r);
+ break;
+ }
+}
+
/*
* extend to deal with the ambiguous C grammar for parsing
* a cast expressions followed by an initializer.
@@ -570,10 +629,50 @@ static struct token *postfix_expression(
case '(': { /* Function call */
struct expression *call = alloc_expression(token->pos, EXPR_CALL);
+ struct symbol *fn = NULL;
+ struct context *c;
+
+ if (expr->symbol_name)
+ fn = lookup_symbol(expr->symbol_name, NS_SYMBOL);
call->op = '(';
call->fn = expr;
token = expression_list(token->next, &call->args);
token = expect(token, ')', "in function call");
+
+ if (fn && fn->ctype.base_type) {
+ FOR_EACH_PTR(fn->ctype.contexts, c) {
+ struct context *copy = alloc_context();
+ struct symbol *fn_arg;
+ struct expression *passed_arg;
+
+ copy->in = c->in;
+ copy->out = c->out;
+ copy->exact = c->exact;
+ add_ptr_list(&call->contexts, copy);
+
+ if (!c->token)
+ continue;
+
+ /* re-parse the token at this place */
+ conditional_expression(c->token, ©->context);
+
+ copy->context->pos = expr->pos;
+
+ if (current_assignment_expression)
+ replace_ident(©->context, &RESULT_ident,
+ current_assignment_expression);
+
+ /* and replace the arg */
+ PREPARE_PTR_LIST(fn->ctype.base_type->arguments, fn_arg);
+ FOR_EACH_PTR(call->args, passed_arg);
+ if (fn_arg)
+ replace_ident(©->context, fn_arg->ident, passed_arg);
+ NEXT_PTR_LIST(fn_arg);
+ END_FOR_EACH_PTR(passed_arg);
+ FINISH_PTR_LIST(fn_arg);
+ } END_FOR_EACH_PTR(c);
+ }
+
expr = call;
continue;
}
@@ -894,6 +993,7 @@ struct token *conditional_expression(str
struct token *assignment_expression(struct token *token, struct expression **tree)
{
+ struct expression *old_cae = current_assignment_expression;
token = conditional_expression(token, tree);
if (*tree && token_type(token) == TOKEN_SPECIAL) {
static const int assignments[] = {
@@ -904,15 +1004,19 @@ struct token *assignment_expression(stru
SPECIAL_SHR_ASSIGN, SPECIAL_AND_ASSIGN,
SPECIAL_OR_ASSIGN, SPECIAL_XOR_ASSIGN };
int i, op = token->special;
+ if (op == '=')
+ current_assignment_expression = *tree;
for (i = 0; i < sizeof(assignments)/sizeof(int); i++)
if (assignments[i] == op) {
struct expression * expr = alloc_expression(token->pos, EXPR_ASSIGNMENT);
expr->left = *tree;
expr->op = op;
*tree = expr;
- return assignment_expression(token->next, &expr->right);
+ token = assignment_expression(token->next, &expr->right);
+ break;
}
}
+ current_assignment_expression = old_cae;
return token;
}
@@ -929,4 +1033,170 @@ struct token *parse_expression(struct to
return comma_expression(token,tree);
}
+int expressions_equal(const struct expression *expr1,
+ const struct expression *expr2)
+{
+ if (expr1 == expr2)
+ return 1;
+
+ if (expr1 == NULL || expr2 == NULL)
+ return 0;
+
+ /* Is this the right way to handle casts? */
+ if (expr1->type == EXPR_CAST ||
+ expr1->type == EXPR_FORCE_CAST ||
+ expr1->type == EXPR_IMPLIED_CAST)
+ return expressions_equal(expr1->cast_expression, expr2);
+
+ if (expr2->type == EXPR_CAST ||
+ expr2->type == EXPR_FORCE_CAST ||
+ expr2->type == EXPR_IMPLIED_CAST)
+ return expressions_equal(expr2->cast_expression, expr1);
+
+ if (expr1->type != expr2->type)
+ return 0;
+
+ switch (expr1->type) {
+ case EXPR_SYMBOL:
+ return ident_equal(expr1->symbol_name, expr2->symbol_name);
+
+ case EXPR_VALUE:
+ return expr1->value == expr2->value;
+
+ case EXPR_FVALUE:
+ return expr1->fvalue == expr2->fvalue;
+
+ case EXPR_STRING:
+ return expr1->string->length == expr2->string->length &&
+ !strncmp(expr1->string->data, expr2->string->data, expr1->string->length);
+
+ case EXPR_BINOP:
+ return expr1->op == expr2->op &&
+ expressions_equal(expr1->left, expr2->left) &&
+ expressions_equal(expr1->right, expr2->right);
+
+ case EXPR_COMMA:
+ case EXPR_ASSIGNMENT:
+ return expressions_equal(expr1->left, expr2->left) &&
+ expressions_equal(expr1->right, expr2->right);
+
+ case EXPR_DEREF:
+ return expressions_equal(expr1->deref, expr2->deref) &&
+ ident_equal(expr1->member, expr2->member);
+
+ case EXPR_PREOP:
+ case EXPR_POSTOP:
+ return expr1->op == expr2->op &&
+ expressions_equal(expr1->unop, expr2->unop);
+
+ /* Not needed right now, but for sake of completness ...
+ case EXPR_LABEL:
+ case EXPR_STATEMENT:
+ case EXPR_CALL:
+ case EXPR_LOGICAL:
+ case EXPR_COMPARE:
+ case EXPR_SELECT:
+ case EXPR_CONDITIONAL:
+ case EXPR_CAST:
+ case EXPR_FORCE_CAST:
+ case EXPR_IMPLIED_CAST:
+ case EXPR_SLICE:
+ case EXPR_INITIALIZER:
+ case EXPR_POS:
+ */
+
+ default:
+ /* nothing, we should already have had a warning */
+ ;
+ }
+
+ return 0;
+}
+
+static int ident_str(struct ident *ident, char *buffer, int length)
+{
+ if (!ident)
+ return 0;
+ return snprintf(buffer, length, "%.*s", ident->len, ident->name);
+}
+
+int expression_str(const struct expression *expr,
+ char *buffer, int length)
+{
+ int n;
+
+ memset(buffer, 0, length);
+
+ if (!expr)
+ return 0;
+
+ /* TODO, think about necessary parentheses () */
+ switch (expr->type) {
+ case EXPR_SYMBOL:
+ return ident_str(expr->symbol_name, buffer, length);
+
+ case EXPR_VALUE:
+ return snprintf(buffer, length, "%llu", expr->value);
+
+ case EXPR_FVALUE:
+ return snprintf(buffer, length, "%Lf", expr->fvalue);
+
+ case EXPR_STRING:
+ return snprintf(buffer, length, "\"%.*s\"", expr->string->length, expr->string->data);
+
+ case EXPR_BINOP:
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, "%c", expr->op);
+ n += expression_str(expr->right, buffer+n, length-n);
+ return n;
+
+ case EXPR_COMMA:
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, ",");
+ n += expression_str(expr->right, buffer+n, length-n);
+ return n;
+
+ case EXPR_ASSIGNMENT:
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, "=");
+ n += expression_str(expr->right, buffer+n, length-n);
+ return n;
+
+ case EXPR_DEREF:
+ if (expr->left->type == EXPR_PREOP &&
+ expr->left->op == '*') {
+ n = expression_str(expr->left->unop, buffer, length);
+ n += snprintf(buffer+n, length-n, "->");
+ n += ident_str(expr->member, buffer+n, length-n);
+ } else {
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, ".");
+ n += ident_str(expr->member, buffer+n, length-n);
+ }
+ return n;
+
+ case EXPR_PREOP:
+ n = snprintf(buffer, length, "%c", expr->op);
+ n += expression_str(expr->unop, buffer+n, length-n);
+ return n;
+
+ case EXPR_POSTOP:
+ n = expression_str(expr->unop, buffer, length);
+ n += snprintf(buffer+n, length-n, "%c", expr->op);
+ return n;
+
+ case EXPR_CAST:
+ case EXPR_FORCE_CAST:
+ case EXPR_IMPLIED_CAST:
+ /* todo: print out the cast type's ctype */
+ *buffer++ = '('; length--;
+ *buffer++ = ')'; length--;
+ return expression_str(expr->cast_expression, buffer, length) + 2;
+
+ default:
+ printf("Missing code in expression_str for %d\n", expr->type);
+ }
+
+ return 0;
+}
--- sparse.orig/expression.h 2008-04-27 10:45:03.000000000 +0200
+++ sparse/expression.h 2008-04-27 10:45:27.000000000 +0200
@@ -121,6 +121,7 @@ struct expression {
struct /* call_expr */ {
struct expression *fn;
struct expression_list *args;
+ struct context_list *contexts;
};
// EXPR_LABEL
struct /* label_expr */ {
@@ -216,4 +217,10 @@ struct token *compound_statement(struct
void cast_value(struct expression *expr, struct symbol *newtype,
struct expression *old, struct symbol *oldtype);
+int expressions_equal(const struct expression *expr1,
+ const struct expression *expr2);
+int expression_str(const struct expression *, char *buf, int buflen);
+struct expression *copy_expression(struct expression *expr);
+void replace_ident(struct expression **in, struct ident *s, struct expression *r);
+extern struct expression *current_assignment_expression;
#endif
--- sparse.orig/sparse.c 2008-04-27 10:45:27.000000000 +0200
+++ sparse/sparse.c 2008-04-27 11:04:41.000000000 +0200
@@ -26,7 +26,7 @@
struct context_check {
int val;
- char name[32];
+ const struct expression *expr;
};
DECLARE_ALLOCATOR(context_check);
@@ -34,22 +34,15 @@ DECLARE_PTR_LIST(context_check_list, str
DECLARE_PTR_LIST(context_list_list, struct context_check_list);
ALLOCATOR(context_check, "context check list");
-static const char *unnamed_context = "<unnamed>";
-static const char *context_name(struct context *context)
-{
- if (context->context && context->context->symbol_name)
- return show_ident(context->context->symbol_name);
- return unnamed_context;
-}
-
-static void context_add(struct context_check_list **ccl, const char *name,
+static void context_add(struct context_check_list **ccl,
+ const struct expression *expr,
int offs)
{
struct context_check *check, *found = NULL;
FOR_EACH_PTR(*ccl, check) {
- if (strcmp(name, check->name))
+ if (!expressions_equal(expr, check->expr))
continue;
found = check;
break;
@@ -57,8 +50,7 @@ static void context_add(struct context_c
if (!found) {
found = __alloc_context_check(0);
- strncpy(found->name, name, sizeof(found->name));
- found->name[sizeof(found->name) - 1] = '\0';
+ found->expr = expr;
add_ptr_list(ccl, found);
}
found->val += offs;
@@ -70,7 +62,7 @@ static int context_list_has(struct conte
struct context_check *check;
FOR_EACH_PTR(ccl, check) {
- if (strcmp(c->name, check->name))
+ if (!expressions_equal(c->expr, check->expr))
continue;
return check->val == c->val;
} END_FOR_EACH_PTR(check);
@@ -105,7 +97,7 @@ static struct context_check_list *checke
struct context_check *c;
FOR_EACH_PTR(ccl, c) {
- context_add(&result, c->name, c->val);
+ context_add(&result, c->expr, c->val);
} END_FOR_EACH_PTR(c);
return result;
@@ -115,15 +107,15 @@ static struct context_check_list *checke
#define CONTEXT_PROB "context problem in '%s': "
#define DEFAULT_CONTEXT_DESCR " default context: "
-static void get_context_string(char **buf, const char **name)
+static const char *get_context_string(char **buf, const char *name)
{
- if (strcmp(*name, unnamed_context)) {
- *buf = malloc(strlen(*name) + 16);
- sprintf(*buf, " context '%s': ", *name);
- *name = *buf;
+ if (strlen(name)) {
+ *buf = malloc(strlen(name) + 16);
+ sprintf(*buf, " context '%s': ", name);
+ return *buf;
} else {
- *name = DEFAULT_CONTEXT_DESCR;
*buf = NULL;
+ return DEFAULT_CONTEXT_DESCR;
}
}
@@ -133,12 +125,13 @@ static int context_list_check(struct ent
{
struct context_check *c1, *c2;
int cur, tgt;
- const char *name;
+ char name[1000];
char *buf;
+ const char *pname;
/* make sure the loop below checks all */
FOR_EACH_PTR(ccl_target, c1) {
- context_add(&ccl_cur, c1->name, 0);
+ context_add(&ccl_cur, c1->expr, 0);
} END_FOR_EACH_PTR(c1);
FOR_EACH_PTR(ccl_cur, c1) {
@@ -146,7 +139,7 @@ static int context_list_check(struct ent
tgt = 0;
FOR_EACH_PTR(ccl_target, c2) {
- if (strcmp(c2->name, c1->name))
+ if (!expressions_equal(c1->expr, c2->expr))
continue;
tgt = c2->val;
break;
@@ -162,11 +155,11 @@ static int context_list_check(struct ent
warning(pos, IMBALANCE_IN "unexpected unlock",
show_ident(ep->name->ident));
- name = c1->name;
- get_context_string(&buf, &name);
+ expression_str(c1->expr, name, sizeof(name));
+ pname = get_context_string(&buf, name);
info(pos, "%swanted %d, got %d",
- name, tgt, cur);
+ pname, tgt, cur);
free(buf);
@@ -176,83 +169,21 @@ static int context_list_check(struct ent
return 0;
}
-static int handle_call(struct entrypoint *ep, struct basic_block *bb,
- struct instruction *insn,
- struct context_check_list *combined)
-{
- struct context *ctx;
- struct context_check *c;
- const char *name, *call, *cmp;
- char *buf;
- int val, ok;
-
- if (!insn->func || !insn->func->sym ||
- insn->func->type != PSEUDO_SYM)
- return 0;
-
- /*
- * Check all contexts the function wants.
- */
- FOR_EACH_PTR(insn->func->sym->ctype.contexts, ctx) {
- name = context_name(ctx);
- val = 0;
-
- FOR_EACH_PTR(combined, c) {
- if (strcmp(c->name, name) == 0) {
- val = c->val;
- break;
- }
- } END_FOR_EACH_PTR(c);
-
- if (ctx->exact) {
- ok = ctx->in == val;
- cmp = "";
- } else {
- ok = ctx->in <= val;
- cmp = ">= ";
- }
-
- if (!ok && Wcontext) {
- get_context_string(&buf, &name);
- call = strdup(show_ident(insn->func->ident));
-
- warning(insn->pos, "context problem in '%s': "
- "'%s' expected different context",
- show_ident(ep->name->ident), call);
-
- info(insn->pos, "%swanted %s%d, got %d",
- name, cmp, ctx->in, val);
-
- free((void *)call);
- free(buf);
-
- return -1;
- }
- } END_FOR_EACH_PTR (ctx);
-
- return 0;
-}
-
static int handle_context(struct entrypoint *ep, struct basic_block *bb,
struct instruction *insn,
struct context_check_list **combined)
{
struct context_check *c;
- const char *name, *cmp;
+ const char *cmp, *pname;
char *buf;
+ char name[1000];
int val, ok;
val = 0;
- name = unnamed_context;
- if (insn->context_expr)
- name = show_ident(insn->context_expr->symbol_name);
-
FOR_EACH_PTR(*combined, c) {
- if (strcmp(c->name, name) == 0) {
+ if (expressions_equal(c->expr, insn->context_expr))
val = c->val;
- break;
- }
} END_FOR_EACH_PTR(c);
if (insn->exact) {
@@ -264,7 +195,8 @@ static int handle_context(struct entrypo
}
if (!ok && Wcontext) {
- get_context_string(&buf, &name);
+ expression_str(insn->context_expr, name, sizeof(name));
+ pname = get_context_string(&buf, name);
if (insn->access_var) {
char *symname = strdup(show_ident(insn->access_var->ident));
@@ -273,6 +205,13 @@ static int handle_context(struct entrypo
"access to '%s' requires different context",
show_ident(ep->name->ident), symname);
free(symname);
+ } else if (insn->called_fn) {
+ char *symname = strdup(show_ident(insn->called_fn->ident));
+ warning(insn->pos,
+ CONTEXT_PROB
+ "'%s' expected different context",
+ show_ident(ep->name->ident), symname);
+ free(symname);
} else {
warning(insn->pos,
CONTEXT_PROB
@@ -281,13 +220,13 @@ static int handle_context(struct entrypo
}
info(insn->pos, "%swanted %s%d, got %d",
- name, cmp, insn->required, val);
+ pname, cmp, insn->required, val);
free(buf);
return -1;
}
- context_add(combined, name, insn->increment);
+ context_add(combined, insn->context_expr, insn->increment);
return 0;
}
@@ -324,7 +263,7 @@ static int check_bb_context(struct entry
* for the conditional_context() attribute.
*/
FOR_EACH_PTR(ccl_in, c) {
- context_add(&combined, c->name, c->val);
+ context_add(&combined, c->expr, c->val);
} END_FOR_EACH_PTR(c);
/* Add the new context to the list of already-checked contexts */
@@ -338,11 +277,6 @@ static int check_bb_context(struct entry
*/
FOR_EACH_PTR(bb->insns, insn) {
switch (insn->opcode) {
- case OP_INLINED_CALL:
- case OP_CALL:
- if (handle_call(ep, bb, insn, combined))
- goto out;
- break;
case OP_CONTEXT:
if (handle_context(ep, bb, insn, &combined))
goto out;
@@ -571,10 +505,10 @@ static void check_context(struct entrypo
check_instructions(ep);
FOR_EACH_PTR(sym->ctype.contexts, context) {
- const char *name = context_name(context);
-
- context_add(&ccl_in, name, context->in);
- context_add(&ccl_target, name, context->out);
+ context_add(&ccl_in, context->in_fn,
+ context->in);
+ context_add(&ccl_target, context->in_fn,
+ context->out);
} END_FOR_EACH_PTR(context);
check_bb_context(ep, ep->entry->bb, ccl_in, ccl_target);
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ sparse/validation/context-vars.c 2008-04-27 12:01:17.000000000 +0200
@@ -0,0 +1,171 @@
+static void a(void *p) __attribute__((context(p,0,1)))
+{
+ __context__(p,1);
+}
+
+static void r(void *p) __attribute__((context(p,1,0)))
+{
+ __context__(p,-1,1);
+}
+
+extern void *l1, *l2;
+
+static void good_paired1(void)
+{
+ a(l1);
+ r(l1);
+}
+
+static void good_paired2(void)
+{
+ a(l1);
+ r(l1);
+ a(l1);
+ r(l1);
+ a(l2);
+ r(l2);
+}
+
+static void good_paired3(void)
+{
+ a(l1);
+ a(l2);
+ r(l2);
+ r(l1);
+}
+
+static void good_lock1(void *lp) __attribute__((context(lp,0,1)))
+{
+ a(lp);
+}
+
+static void good_lock2(void *lp) __attribute__((context(lp,0,1)))
+{
+ a(lp);
+ r(lp);
+ a(lp);
+}
+
+extern void **v;
+
+static void warn_lock1(void)
+{
+ a(v[1]);
+}
+
+extern int condition;
+
+static void good_lock3(void)
+{
+ a(v[1]);
+ if (condition) {
+ a(v[2]);
+ r(v[2]);
+ }
+ r(v[1]);
+}
+
+#define cond_lock(x, c)\
+ ((c) ? ({ __context__(x,1); 1; }) : 0)
+
+#define trylock(x) \
+ cond_lock(x, _try_lock(x))
+
+extern void _try_lock(int *x);
+
+static int good_condlock1(void)
+{
+ if (trylock(v[0]))
+ r(v[0]);
+}
+
+static int good_condlock2(void)
+{
+ if (trylock(&condition))
+ r((void*)&condition);
+}
+
+extern void ai(int *p) __attribute__((context(p,0,1)));
+extern void ri(int *p) __attribute__((context(p,1,0)));
+
+struct test {
+ int lock;
+};
+
+static inline void unlock(struct test *y)
+ __attribute__((context(&y->lock,1,0)))
+{
+ ri(&y->lock);
+}
+
+static void good_lock4(struct test *t)
+ __attribute__((context(&t->lock,0,1)))
+{
+ ai(&t->lock);
+}
+
+extern int *find_locked(void) __attribute__((context(RESULT,0,1)));
+
+static void good_find_release1(void)
+{
+ int *p;
+
+ p = find_locked();
+ ri(p);
+}
+
+extern struct test *find_locked_struct(void) __attribute__((context(&RESULT->lock,0,1)));
+
+static void good_find_release2(void)
+{
+ struct test *t;
+
+ t = find_locked_struct();
+ unlock(t);
+}
+
+static void good_find_release3(void)
+{
+ struct test *t = find_locked_struct();
+ unlock(t);
+}
+
+static void warn_unlock(void)
+{
+ struct test *t;
+ /* check that the warning is here, not at the unlock definition */
+ unlock(t);
+}
+
+extern int _locked_struct_test(struct test *ls, int *flags);
+#define locked_struct_test(ls, flags) (( _locked_struct_test((ls), (flags)) ) ? ({ __context__(&ls->lock,1); 1;}) : 0)
+
+static inline void unlock2(struct test *y)
+ __attribute__((context(&y->lock,1,0)))
+{
+ unlock(y);
+}
+
+static void good_locked_val(void)
+{
+ struct test *t;
+ int flags;
+
+ if (!t || !locked_struct_test(t, &flags))
+ goto out;
+
+ unlock2(t);
+ out:
+ ;
+}
+
+/*
+ * check-name: Check -Wcontext with lock variables
+ *
+ * check-error-start
+context-vars.c:53:7: warning: context imbalance in 'warn_lock1': wrong count at exit
+context-vars.c:53:7: context '**v+4': wanted 0, got 1
+context-vars.c:137:11: warning: context problem in 'warn_unlock': 'unlock' expected different context
+context-vars.c:137:11: context '*t+0': wanted >= 1, got 0
+ * check-error-end
+ */
--- sparse.orig/evaluate.c 2008-04-27 10:45:27.000000000 +0200
+++ sparse/evaluate.c 2008-04-27 10:45:27.000000000 +0200
@@ -2954,8 +2954,16 @@ struct symbol *evaluate_expression(struc
return evaluate_alignof(expr);
case EXPR_DEREF:
return evaluate_member_dereference(expr);
- case EXPR_CALL:
+ case EXPR_CALL: {
+ struct context *c;
+ FOR_EACH_PTR(expr->contexts, c) {
+ if (c->context &&
+ (c->context->type != EXPR_SYMBOL ||
+ c->context->symbol))
+ evaluate_expression(c->context);
+ } END_FOR_EACH_PTR(c);
return evaluate_call(expr);
+ }
case EXPR_SELECT:
case EXPR_CONDITIONAL:
return evaluate_conditional_expression(expr);
@@ -3025,6 +3033,7 @@ static void check_duplicates(struct symb
static struct symbol *evaluate_symbol(struct symbol *sym)
{
struct symbol *base_type;
+ struct context *c;
if (!sym)
return sym;
@@ -3054,6 +3063,13 @@ static struct symbol *evaluate_symbol(st
evaluate_statement(base_type->stmt);
current_fn = curr;
+
+ FOR_EACH_PTR(sym->ctype.contexts, c) {
+ if (c->in_fn &&
+ (c->in_fn->type != EXPR_SYMBOL ||
+ c->in_fn->symbol))
+ evaluate_expression(c->in_fn);
+ } END_FOR_EACH_PTR(c);
}
return base_type;
--- sparse.orig/expand.c 2008-04-27 10:45:27.000000000 +0200
+++ sparse/expand.c 2008-04-27 10:45:27.000000000 +0200
@@ -1034,8 +1034,14 @@ int expand_symbol(struct symbol *sym)
retval = expand_expression(sym->initializer);
/* expand the body of the symbol */
if (base_type->type == SYM_FN) {
+ struct context *c;
+
if (base_type->stmt)
expand_statement(base_type->stmt);
+
+ FOR_EACH_PTR(sym->ctype.contexts, c) {
+ expand_expression(c->in_fn);
+ } END_FOR_EACH_PTR(c);
}
return retval;
}
--- sparse.orig/parse.c 2008-04-27 10:45:27.000000000 +0200
+++ sparse/parse.c 2008-04-27 10:45:28.000000000 +0200
@@ -879,12 +879,13 @@ static struct token *_attribute_context(
struct context *context = alloc_context();
struct expression *args[4];
int argc = 0;
- struct token *last = NULL;
+ struct token *first = NULL, *last = NULL;
token = expect(token, '(', "after context attribute");
while (!match_op(token, ')')) {
struct expression *expr = NULL;
last = token;
+ first = first ? : token;
token = conditional_expression(token, &expr);
if (!expr)
break;
@@ -910,12 +911,14 @@ static struct token *_attribute_context(
break;
case 3:
context->context = args[0];
+ context->token = first;
context->in = get_expression_value(args[1]);
context->out = get_expression_value(args[2]);
break;
case 4: {
const char *rw;
context->context = args[0];
+ context->token = first;
context->in = get_expression_value(args[1]);
context->out = get_expression_value(args[2]);
@@ -2096,6 +2099,7 @@ static struct token *parse_function_body
struct symbol *base_type = decl->ctype.base_type;
struct statement *stmt, **p;
struct symbol *arg;
+ struct context *c;
old_symbol_list = function_symbol_list;
if (decl->ctype.modifiers & MOD_INLINE) {
@@ -2124,6 +2128,11 @@ static struct token *parse_function_body
token = compound_statement(token->next, stmt);
+ FOR_EACH_PTR(decl->ctype.contexts, c) {
+ if (c->token)
+ conditional_expression(c->token, &c->in_fn);
+ } END_FOR_EACH_PTR(c);
+
end_function(decl);
if (!(decl->ctype.modifiers & MOD_INLINE))
add_symbol(list, decl);
@@ -2286,11 +2295,17 @@ struct token *external_declaration(struc
for (;;) {
if (!is_typedef && match_op(token, '=')) {
+ struct expression *old_cae = current_assignment_expression;
if (decl->ctype.modifiers & MOD_EXTERN) {
warning(decl->pos, "symbol with external linkage has initializer");
decl->ctype.modifiers &= ~MOD_EXTERN;
}
+ current_assignment_expression = alloc_expression(token->pos, EXPR_SYMBOL);
+ current_assignment_expression->symbol_name = decl->ident;
+ current_assignment_expression->symbol =
+ lookup_symbol(decl->ident, NS_SYMBOL | NS_TYPEDEF);
token = initializer(&decl->initializer, token->next);
+ current_assignment_expression = old_cae;
}
if (!is_typedef) {
if (!(decl->ctype.modifiers & (MOD_EXTERN | MOD_INLINE))) {
--- sparse.orig/symbol.h 2008-04-27 10:45:27.000000000 +0200
+++ sparse/symbol.h 2008-04-27 10:45:28.000000000 +0200
@@ -77,6 +77,10 @@ enum context_read_write_specifier {
struct context {
struct expression *context;
+ /* store the token pointer */
+ struct token *token;
+ /* to re-evaluate this context within the function it is for */
+ struct expression *in_fn;
unsigned int in, out;
int exact;
enum context_read_write_specifier rws;
--- sparse.orig/inline.c 2008-04-27 10:45:27.000000000 +0200
+++ sparse/inline.c 2008-04-27 10:45:28.000000000 +0200
@@ -56,7 +56,7 @@ static struct symbol_list *copy_symbol_l
return dst;
}
-static struct expression * copy_expression(struct expression *expr)
+struct expression *copy_expression(struct expression *expr)
{
if (!expr)
return NULL;
@@ -474,6 +474,7 @@ void copy_statement(struct statement *sr
dst->args = copy_one_statement(src->args);
dst->ret = copy_symbol(src->pos, src->ret);
dst->inline_fn = src->inline_fn;
+ dst->inlined_fn_expr = src->inlined_fn_expr;
}
static struct symbol *create_copy_symbol(struct symbol *orig)
@@ -555,6 +556,7 @@ int inline_function(struct expression *e
stmt->args = decl;
}
stmt->inline_fn = sym;
+ stmt->inlined_fn_expr = expr;
unset_replace_list(fn_symbol_list);
--- sparse.orig/linearize.c 2008-04-27 10:45:27.000000000 +0200
+++ sparse/linearize.c 2008-04-27 11:06:10.000000000 +0200
@@ -35,6 +35,7 @@ static pseudo_t linearize_initializer(st
struct pseudo void_pseudo = {};
static struct position current_pos;
+static int in_inline_skip_context = 0;
ALLOCATOR(pseudo_user, "pseudo_user");
@@ -436,9 +437,15 @@ const char *show_instruction(struct inst
buf += sprintf(buf, "%s <- %s", show_pseudo(insn->target), show_pseudo(insn->src1));
break;
- case OP_CONTEXT:
+ case OP_CONTEXT: {
+ char ctxbuf[100];
+ if (insn->context_expr) {
+ expression_str(insn->context_expr, ctxbuf, sizeof(ctxbuf));
+ buf += sprintf(buf, "%s, ", ctxbuf);
+ }
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));
break;
@@ -1280,7 +1287,7 @@ static pseudo_t linearize_call_expressio
struct instruction *insn = alloc_typed_instruction(OP_CALL, expr->ctype);
pseudo_t retval, call;
struct ctype *ctype = NULL;
- struct context *context;
+ struct context *c;
if (!expr->ctype) {
warning(expr->pos, "call with no type!");
@@ -1316,29 +1323,17 @@ static pseudo_t linearize_call_expressio
insn->target = retval;
add_one_insn(ep, insn);
- if (ctype) {
- FOR_EACH_PTR(ctype->contexts, context) {
- int in = context->in;
- int out = context->out;
- 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 = context_diff;
- insn->context_expr = context->context;
- add_one_insn(ep, insn);
- }
- } END_FOR_EACH_PTR(context);
- }
+ if (!in_inline_skip_context)
+ FOR_EACH_PTR(expr->contexts, c) {
+ struct instruction *tmp = alloc_instruction(OP_CONTEXT, 0);
+ tmp->increment = c->out - c->in;
+ tmp->exact = c->exact;
+ tmp->context_expr = c->context;
+ tmp->required = c->in;
+ tmp->called_fn = call->sym;
+ tmp->pos = insn->pos;
+ add_one_insn(ep, tmp);
+ } END_FOR_EACH_PTR(c);
return retval;
}
@@ -1712,6 +1707,8 @@ static pseudo_t linearize_compound_state
pseudo = VOID;
FOR_EACH_PTR(stmt->stmts, s) {
+ if (in_inline_skip_context && s->type == STMT_CONTEXT)
+ continue;
pseudo = linearize_statement(ep, s);
} END_FOR_EACH_PTR(s);
@@ -1739,6 +1736,7 @@ static pseudo_t linearize_inlined_call(s
struct statement *args = stmt->args;
struct basic_block *bb;
pseudo_t pseudo;
+ struct context *c;
if (args) {
struct symbol *sym;
@@ -1750,12 +1748,27 @@ static pseudo_t linearize_inlined_call(s
} END_FOR_EACH_PTR(sym);
}
+ in_inline_skip_context++;
insn->target = pseudo = linearize_compound_statement(ep, stmt);
+ in_inline_skip_context--;
use_pseudo(insn, symbol_pseudo(ep, stmt->inline_fn), &insn->func);
bb = ep->active;
if (bb && !bb->insns)
bb->pos = stmt->pos;
add_one_insn(ep, insn);
+
+ if (!in_inline_skip_context)
+ FOR_EACH_PTR(stmt->inlined_fn_expr->contexts, c) {
+ struct instruction *tmp = alloc_instruction(OP_CONTEXT, 0);
+ tmp->increment = c->out - c->in;
+ tmp->exact = c->exact;
+ tmp->context_expr = c->context;
+ tmp->required = c->in;
+ tmp->called_fn = stmt->inline_fn;
+ tmp->pos = insn->pos;
+ add_one_insn(ep, tmp);
+ } END_FOR_EACH_PTR(c);
+
return pseudo;
}
--- sparse.orig/parse.h 2008-04-27 10:45:27.000000000 +0200
+++ sparse/parse.h 2008-04-27 10:45:28.000000000 +0200
@@ -61,6 +61,7 @@ struct statement {
struct symbol *ret;
struct symbol *inline_fn;
struct statement *args;
+ struct expression *inlined_fn_expr;
};
struct /* labeled_struct */ {
struct symbol *label_identifier;
--- sparse.orig/linearize.h 2008-04-27 10:45:27.000000000 +0200
+++ sparse/linearize.h 2008-04-27 10:45:28.000000000 +0200
@@ -119,6 +119,7 @@ struct instruction {
int increment, required, exact;
struct expression *context_expr;
struct symbol *access_var;
+ struct symbol *called_fn;
};
struct /* asm */ {
const char *string;
--- sparse.orig/ident-list.h 2008-04-27 10:45:27.000000000 +0200
+++ sparse/ident-list.h 2008-04-27 10:45:28.000000000 +0200
@@ -98,6 +98,7 @@ __IDENT(__PRETTY_FUNCTION___ident, "__PR
IDENT_RESERVED(__context__);
IDENT_RESERVED(__exact_context__);
IDENT_RESERVED(__range__);
+IDENT_RESERVED(RESULT);
/* Magic function names we recognize */
IDENT(memset); IDENT(memcpy);
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 7/9] test conditional result locking
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (5 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 6/9] check context expressions as expressions Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 8/9] show required context in instruction output Johannes Berg
` (4 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 008-test-condititional-lock-result.patch --]
[-- Type: text/plain, Size: 1648 bytes --]
To test a function that can return a locked struct or NULL,
a macro has to be invented. Add a test case for that.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
validation/context-vars.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
--- sparse.orig/validation/context-vars.c 2008-04-27 14:10:37.000000000 +0200
+++ sparse/validation/context-vars.c 2008-04-27 14:41:41.000000000 +0200
@@ -1,3 +1,5 @@
+#include <stddef.h>
+
static void a(void *p) __attribute__((context(p,0,1)))
{
__context__(p,1);
@@ -159,13 +161,32 @@ static void good_locked_val(void)
;
}
+
+extern struct test *_search(int key);
+
+#define search(res, key) do { \
+ (res) = _search((key)); \
+ if (res) \
+ __context__(&(res)->lock,1);\
+ } while (0)
+
+static void test(void)
+{
+ struct test **x;
+
+ search(*x, 32);
+ if (*x)
+ unlock(*x);
+}
+
+
/*
* check-name: Check -Wcontext with lock variables
*
* check-error-start
-context-vars.c:53:7: warning: context imbalance in 'warn_lock1': wrong count at exit
-context-vars.c:53:7: context '**v+4': wanted 0, got 1
-context-vars.c:137:11: warning: context problem in 'warn_unlock': 'unlock' expected different context
-context-vars.c:137:11: context '*t+0': wanted >= 1, got 0
+context-vars.c:55:7: warning: context imbalance in 'warn_lock1': wrong count at exit
+context-vars.c:55:7: context '**v+4': wanted 0, got 1
+context-vars.c:139:11: warning: context problem in 'warn_unlock': 'unlock' expected different context
+context-vars.c:139:11: context '*t+0': wanted >= 1, got 0
* check-error-end
*/
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 8/9] show required context in instruction output
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (6 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 7/9] test conditional result locking Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 8:54 ` [PATCH 9/9] check inlines explicitly Johannes Berg
` (3 subsequent siblings)
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 009-show-context-required.patch --]
[-- Type: text/plain, Size: 1055 bytes --]
Just eases debugging sparse/the context tracking itself.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
expression.c | 2 ++
linearize.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
--- sparse.orig/expression.c 2008-04-28 16:18:40.000000000 +0200
+++ sparse/expression.c 2008-04-28 16:18:40.000000000 +0200
@@ -648,6 +648,8 @@ static struct token *postfix_expression(
copy->in = c->in;
copy->out = c->out;
copy->exact = c->exact;
+ copy->token = c->token;
+ copy->in_fn = c->context;
add_ptr_list(&call->contexts, copy);
if (!c->token)
--- sparse.orig/linearize.c 2008-04-28 16:18:40.000000000 +0200
+++ sparse/linearize.c 2008-04-28 16:18:40.000000000 +0200
@@ -443,7 +443,7 @@ const char *show_instruction(struct inst
expression_str(insn->context_expr, ctxbuf, sizeof(ctxbuf));
buf += sprintf(buf, "%s, ", ctxbuf);
}
- buf += sprintf(buf, "%d", insn->increment);
+ buf += sprintf(buf, "%d %d", insn->increment, insn->required);
break;
}
case OP_RANGE:
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 9/9] check inlines explicitly
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (7 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 8/9] show required context in instruction output Johannes Berg
@ 2008-05-29 8:54 ` Johannes Berg
2008-05-29 23:14 ` [PATCH 9/9 v2] " Johannes Berg
2008-05-29 22:12 ` [PATCH 0/9] context tracking updates Harvey Harrison
` (2 subsequent siblings)
11 siblings, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 8:54 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: 010-check-inlines.patch --]
[-- Type: text/plain, Size: 1674 bytes --]
An earlier patch disabled checking through inline functions because
inlining them clashes with the context tracking code, so this now
makes sparse check the inline functions as though they were really
functions.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
sparse.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
--- sparse.orig/sparse.c 2008-05-29 10:19:13.000000000 +0200
+++ sparse/sparse.c 2008-05-29 10:19:14.000000000 +0200
@@ -29,11 +29,12 @@ struct context_check {
const struct expression *expr;
};
-DECLARE_ALLOCATOR(context_check);
DECLARE_PTR_LIST(context_check_list, struct context_check);
DECLARE_PTR_LIST(context_list_list, struct context_check_list);
ALLOCATOR(context_check, "context check list");
+static struct symbol_list *inline_list = NULL;
+
static void context_add(struct context_check_list **ccl,
const struct expression *expr,
@@ -277,6 +278,15 @@ static int check_bb_context(struct entry
*/
FOR_EACH_PTR(bb->insns, insn) {
switch (insn->opcode) {
+ case OP_INLINED_CALL: {
+ if (!insn->func->sym)
+ break;
+ if (insn->func->sym->visited)
+ break;
+ insn->func->sym->visited = 1;
+ add_ptr_list(&inline_list, insn->func->sym);
+ break;
+ }
case OP_CONTEXT:
if (handle_context(ep, bb, insn, &combined))
goto out;
@@ -545,6 +555,9 @@ int main(int argc, char **argv)
check_symbols(sparse_initialize(argc, argv, &filelist));
FOR_EACH_PTR_NOTAG(filelist, file) {
check_symbols(sparse(file));
+ evaluate_symbol_list(inline_list);
+ check_symbols(inline_list);
+ free_ptr_list(&inline_list);
} END_FOR_EACH_PTR_NOTAG(file);
return 0;
}
--
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (8 preceding siblings ...)
2008-05-29 8:54 ` [PATCH 9/9] check inlines explicitly Johannes Berg
@ 2008-05-29 22:12 ` Harvey Harrison
2008-05-29 22:35 ` Harvey Harrison
2008-07-20 12:30 ` Johannes Berg
11 siblings, 0 replies; 27+ messages in thread
From: Harvey Harrison @ 2008-05-29 22:12 UTC (permalink / raw)
To: Johannes Berg; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
On Thu, 2008-05-29 at 10:54 +0200, Johannes Berg wrote:
> This updates the context tracking, I just explained most of it in the mail
> "Re: sparse context warning problem ..."
> (message ID <1212050823.16917.37.camel@johannes.berg>)
>
> johannes
From initial impressions, this works pretty well, at least in the kernel.
Looking a little deeper at the results now, but seems like a nice improvement.
Harvey
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (9 preceding siblings ...)
2008-05-29 22:12 ` [PATCH 0/9] context tracking updates Harvey Harrison
@ 2008-05-29 22:35 ` Harvey Harrison
2008-05-29 22:45 ` Johannes Berg
2008-07-20 12:30 ` Johannes Berg
11 siblings, 1 reply; 27+ messages in thread
From: Harvey Harrison @ 2008-05-29 22:35 UTC (permalink / raw)
To: Johannes Berg; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
On Thu, 2008-05-29 at 10:54 +0200, Johannes Berg wrote:
> This updates the context tracking, I just explained most of it in the mail
> "Re: sparse context warning problem ..."
> (message ID <1212050823.16917.37.camel@johannes.berg>)
>
With these applied I get a new error when checking
fs/minix/itree_common.c
I'll try and track it down.
Harvey
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 22:35 ` Harvey Harrison
@ 2008-05-29 22:45 ` Johannes Berg
2008-05-29 22:47 ` Harvey Harrison
2008-05-29 22:51 ` Harvey Harrison
0 siblings, 2 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 22:45 UTC (permalink / raw)
To: Harvey Harrison; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
[-- Attachment #1: Type: text/plain, Size: 497 bytes --]
On Thu, 2008-05-29 at 15:35 -0700, Harvey Harrison wrote:
> On Thu, 2008-05-29 at 10:54 +0200, Johannes Berg wrote:
> > This updates the context tracking, I just explained most of it in the mail
> > "Re: sparse context warning problem ..."
> > (message ID <1212050823.16917.37.camel@johannes.berg>)
> >
>
> With these applied I get a new error when checking
> fs/minix/itree_common.c
What sort? Right now, it seems to go into a dead loop on
fs/minix/itree_v1.c for me?!
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 22:45 ` Johannes Berg
@ 2008-05-29 22:47 ` Harvey Harrison
2008-05-29 22:51 ` Harvey Harrison
1 sibling, 0 replies; 27+ messages in thread
From: Harvey Harrison @ 2008-05-29 22:47 UTC (permalink / raw)
To: Johannes Berg; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
On Fri, 2008-05-30 at 00:45 +0200, Johannes Berg wrote:
> On Thu, 2008-05-29 at 15:35 -0700, Harvey Harrison wrote:
> > On Thu, 2008-05-29 at 10:54 +0200, Johannes Berg wrote:
> > > This updates the context tracking, I just explained most of it in the mail
> > > "Re: sparse context warning problem ..."
> > > (message ID <1212050823.16917.37.camel@johannes.berg>)
> > >
> >
> > With these applied I get a new error when checking
> > fs/minix/itree_common.c
>
> What sort? Right now, it seems to go into a dead loop on
> fs/minix/itree_v1.c for me?!
>
That would be the one....after a long spew ending in:
fs/minix/itree_common.c:44:2: warning: label 'continue' already bound
fs/minix/itree_common.c:44:2: warning: label 'break' already bound
fs/minix/itree_common.c:19:2: warning: label 'continue' already bound
fs/minix/itree_common.c:19:2: warning: label 'break' already bound
Something has gotten a bit confused somewhere.
Harvey
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 22:45 ` Johannes Berg
2008-05-29 22:47 ` Harvey Harrison
@ 2008-05-29 22:51 ` Harvey Harrison
2008-05-29 22:54 ` Johannes Berg
2008-05-29 23:04 ` Johannes Berg
1 sibling, 2 replies; 27+ messages in thread
From: Harvey Harrison @ 2008-05-29 22:51 UTC (permalink / raw)
To: Johannes Berg; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
On Fri, 2008-05-30 at 00:45 +0200, Johannes Berg wrote:
> On Thu, 2008-05-29 at 15:35 -0700, Harvey Harrison wrote:
> > On Thu, 2008-05-29 at 10:54 +0200, Johannes Berg wrote:
> > > This updates the context tracking, I just explained most of it in the mail
> > > "Re: sparse context warning problem ..."
> > > (message ID <1212050823.16917.37.camel@johannes.berg>)
> > >
> >
> > With these applied I get a new error when checking
> > fs/minix/itree_common.c
>
> What sort? Right now, it seems to go into a dead loop on
> fs/minix/itree_v1.c for me?!
>
It's a problem introduced with 9/9 explicitly checking inlines.
Harvey
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 22:51 ` Harvey Harrison
@ 2008-05-29 22:54 ` Johannes Berg
2008-05-29 23:03 ` Pavel Roskin
2008-05-29 23:04 ` Johannes Berg
1 sibling, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 22:54 UTC (permalink / raw)
To: Harvey Harrison; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
[-- Attachment #1: Type: text/plain, Size: 226 bytes --]
> > What sort? Right now, it seems to go into a dead loop on
> > fs/minix/itree_v1.c for me?!
> >
>
> It's a problem introduced with 9/9 explicitly checking inlines.
Cool, thanks, I'm just looking at it.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 22:54 ` Johannes Berg
@ 2008-05-29 23:03 ` Pavel Roskin
2008-05-29 23:06 ` Johannes Berg
0 siblings, 1 reply; 27+ messages in thread
From: Pavel Roskin @ 2008-05-29 23:03 UTC (permalink / raw)
To: Johannes Berg
Cc: Harvey Harrison, Josh Triplett, Philipp Reisner, linux-sparse
On Fri, 2008-05-30 at 00:54 +0200, Johannes Berg wrote:
> > > What sort? Right now, it seems to go into a dead loop on
> > > fs/minix/itree_v1.c for me?!
> > >
> >
> > It's a problem introduced with 9/9 explicitly checking inlines.
>
> Cool, thanks, I'm just looking at it.
You may want to look at this:
http://marc.info/?t=120287346100007&r=1&w=2
http://marc.info/?l=linux-sparse&m=120370800715845&w=2
It was happening in ndiswrapper because a typedef was forcing an inline
function to be linearized twice.
--
Regards,
Pavel Roskin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 22:51 ` Harvey Harrison
2008-05-29 22:54 ` Johannes Berg
@ 2008-05-29 23:04 ` Johannes Berg
1 sibling, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 23:04 UTC (permalink / raw)
To: Harvey Harrison; +Cc: Josh Triplett, Philipp Reisner, linux-sparse
[-- Attachment #1: Type: text/plain, Size: 248 bytes --]
> > What sort? Right now, it seems to go into a dead loop on
> > fs/minix/itree_v1.c for me?!
> >
>
> It's a problem introduced with 9/9 explicitly checking inlines.
It is, indeed, very odd. I don't see it right now though.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 23:03 ` Pavel Roskin
@ 2008-05-29 23:06 ` Johannes Berg
2008-05-29 23:15 ` Johannes Berg
0 siblings, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 23:06 UTC (permalink / raw)
To: Pavel Roskin
Cc: Harvey Harrison, Josh Triplett, Philipp Reisner, linux-sparse
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
On Thu, 2008-05-29 at 19:03 -0400, Pavel Roskin wrote:
> On Fri, 2008-05-30 at 00:54 +0200, Johannes Berg wrote:
> > > > What sort? Right now, it seems to go into a dead loop on
> > > > fs/minix/itree_v1.c for me?!
> > > >
> > >
> > > It's a problem introduced with 9/9 explicitly checking inlines.
> >
> > Cool, thanks, I'm just looking at it.
>
> You may want to look at this:
> http://marc.info/?t=120287346100007&r=1&w=2
> http://marc.info/?l=linux-sparse&m=120370800715845&w=2
>
> It was happening in ndiswrapper because a typedef was forcing an inline
> function to be linearized twice.
Ah, thanks for the pointers, that might be the case. Wonder why it's
being linearized twice, and I see it even without the linearization I
force...? will take a look tomorrow.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 9/9 v2] check inlines explicitly
2008-05-29 8:54 ` [PATCH 9/9] check inlines explicitly Johannes Berg
@ 2008-05-29 23:14 ` Johannes Berg
2008-05-29 23:20 ` Harvey Harrison
0 siblings, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 23:14 UTC (permalink / raw)
To: Josh Triplett
Cc: Philipp Reisner, linux-sparse, Harvey Harrison, Pavel Roskin
An earlier patch disabled checking through inline functions because
inlining them clashes with the context tracking code, so this now
makes sparse check the inline functions as though they were really
functions.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Maybe linearize_symbol() should do the check and return sym->ep if not
NULL?
sparse.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
--- sparse.orig/sparse.c 2008-05-30 00:53:33.000000000 +0200
+++ sparse/sparse.c 2008-05-30 01:12:55.000000000 +0200
@@ -29,11 +29,12 @@ struct context_check {
const struct expression *expr;
};
-DECLARE_ALLOCATOR(context_check);
DECLARE_PTR_LIST(context_check_list, struct context_check);
DECLARE_PTR_LIST(context_list_list, struct context_check_list);
ALLOCATOR(context_check, "context check list");
+static struct symbol_list *inline_list = NULL;
+
static void context_add(struct context_check_list **ccl,
const struct expression *expr,
@@ -277,6 +278,15 @@ static int check_bb_context(struct entry
*/
FOR_EACH_PTR(bb->insns, insn) {
switch (insn->opcode) {
+ case OP_INLINED_CALL: {
+ if (!insn->func->sym)
+ break;
+ if (insn->func->sym->visited)
+ break;
+ insn->func->sym->visited = 1;
+ add_ptr_list(&inline_list, insn->func->sym);
+ break;
+ }
case OP_CONTEXT:
if (handle_context(ep, bb, insn, &combined))
goto out;
@@ -526,7 +536,14 @@ static void check_symbols(struct symbol_
struct entrypoint *ep;
expand_symbol(sym);
- ep = linearize_symbol(sym);
+ /*
+ * If we're passing back an inline via the special code
+ * that tests those, it might already be linearized, if
+ * so just check it and don't linearize again.
+ */
+ ep = sym->ep;
+ if (!ep)
+ ep = linearize_symbol(sym);
if (ep) {
if (dbg_entry)
show_entry(ep);
@@ -545,6 +562,9 @@ int main(int argc, char **argv)
check_symbols(sparse_initialize(argc, argv, &filelist));
FOR_EACH_PTR_NOTAG(filelist, file) {
check_symbols(sparse(file));
+ evaluate_symbol_list(inline_list);
+ check_symbols(inline_list);
+ free_ptr_list(&inline_list);
} END_FOR_EACH_PTR_NOTAG(file);
return 0;
}
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 23:06 ` Johannes Berg
@ 2008-05-29 23:15 ` Johannes Berg
0 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-05-29 23:15 UTC (permalink / raw)
To: Pavel Roskin
Cc: Harvey Harrison, Josh Triplett, Philipp Reisner, linux-sparse
[-- Attachment #1: Type: text/plain, Size: 966 bytes --]
On Fri, 2008-05-30 at 01:06 +0200, Johannes Berg wrote:
> On Thu, 2008-05-29 at 19:03 -0400, Pavel Roskin wrote:
> > On Fri, 2008-05-30 at 00:54 +0200, Johannes Berg wrote:
> > > > > What sort? Right now, it seems to go into a dead loop on
> > > > > fs/minix/itree_v1.c for me?!
> > > > >
> > > >
> > > > It's a problem introduced with 9/9 explicitly checking inlines.
> > >
> > > Cool, thanks, I'm just looking at it.
> >
> > You may want to look at this:
> > http://marc.info/?t=120287346100007&r=1&w=2
> > http://marc.info/?l=linux-sparse&m=120370800715845&w=2
> >
> > It was happening in ndiswrapper because a typedef was forcing an inline
> > function to be linearized twice.
>
> Ah, thanks for the pointers, that might be the case. Wonder why it's
> being linearized twice, and I see it even without the linearization I
> force...? will take a look tomorrow.
No, I'm just confused. That updated patch fixes it for me.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9 v2] check inlines explicitly
2008-05-29 23:14 ` [PATCH 9/9 v2] " Johannes Berg
@ 2008-05-29 23:20 ` Harvey Harrison
0 siblings, 0 replies; 27+ messages in thread
From: Harvey Harrison @ 2008-05-29 23:20 UTC (permalink / raw)
To: Johannes Berg; +Cc: Josh Triplett, Philipp Reisner, linux-sparse, Pavel Roskin
On Fri, 2008-05-30 at 01:14 +0200, Johannes Berg wrote:
> An earlier patch disabled checking through inline functions because
> inlining them clashes with the context tracking code, so this now
> makes sparse check the inline functions as though they were really
> functions.
>
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Works for me.
Harvey
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] context tracking updates
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
` (10 preceding siblings ...)
2008-05-29 22:35 ` Harvey Harrison
@ 2008-07-20 12:30 ` Johannes Berg
11 siblings, 0 replies; 27+ messages in thread
From: Johannes Berg @ 2008-07-20 12:30 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse
[-- Attachment #1: Type: text/plain, Size: 452 bytes --]
On Thu, 2008-05-29 at 10:54 +0200, Johannes Berg wrote:
> This updates the context tracking, I just explained most of it in the mail
> "Re: sparse context warning problem ..."
> (message ID <1212050823.16917.37.camel@johannes.berg>)
Ping? Josh, do you intend to merge this? I keep getting asked about the
corresponding kernel patches. If you don't want it, please revert my
previous patches and I'll stop bothering you either way.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/9 v2] check context expressions as expressions
2008-05-29 8:54 ` [PATCH 6/9] check context expressions as expressions Johannes Berg
@ 2008-09-10 7:33 ` Johannes Berg
2008-09-10 19:21 ` Christopher Li
0 siblings, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-09-10 7:33 UTC (permalink / raw)
To: Josh Triplett; +Cc: Philipp Reisner, linux-sparse, Harvey Harrison
This patch makes sparse evaluate context expressions allowing this:
struct test {
lock_t lock;
int i;
};
extern void r(struct test *t) __attribute__((context(&t->lock,0,1)));
extern struct test *find(int i) __attribute__((context(&RESULT->lock,1,0)));
void test(void)
{
struct test *x = find(42);
r(x);
r(x);
}
to work the way you think it would.
This works by rewriting the given attribute expression into __context__
statements and evaluating those in the caller scope.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Somewhat inspired by Philipp's patch. When it warns, the expression is
given as evaluated, e.g. in above case would result in a warning about
"**x+0". We can fix that later by keeping the original expression and
not expanding/evaluating it, but we need to do both for it to work.
Note that there's a problem: You cannot give RESULT on a static function
that is declared there without getting an error in the function itself...
v2: Harvey found a bug in this by running it on the Linux kernel and
I also cleaned up some functions and made them static.
evaluate.c | 18 ++-
expand.c | 6 +
expression.c | 271 +++++++++++++++++++++++++++++++++++++++++++++-
expression.h | 5
ident-list.h | 1
inline.c | 4
linearize.c | 63 ++++++----
linearize.h | 1
parse.c | 17 ++
parse.h | 1
sparse.c | 144 ++++++------------------
symbol.h | 4
validation/context-vars.c | 171 +++++++++++++++++++++++++++++
13 files changed, 572 insertions(+), 134 deletions(-)
--- sparse.orig/expression.c 2008-09-10 08:56:01.000000000 +0200
+++ sparse/expression.c 2008-09-10 09:27:43.000000000 +0200
@@ -27,6 +27,8 @@
#include "expression.h"
#include "target.h"
+struct expression *current_assignment_expression = NULL;
+
static int match_oplist(int op, ...)
{
va_list args;
@@ -509,6 +511,63 @@ static struct token *expression_list(str
return token;
}
+static int ident_equal(struct ident *ident1, struct ident *ident2)
+{
+ if (ident1 == ident2)
+ return 1;
+ if (!ident1 || !ident2)
+ return 0;
+
+ return ident1->len == ident2->len &&
+ !strncmp(ident1->name, ident2->name, ident1->len);
+}
+
+/* TODO: this is probably not complete */
+static void replace_ident(struct expression **_in, struct ident *s, struct expression *r)
+{
+ struct expression *in = *_in;
+
+ switch (in->type) {
+ case EXPR_SYMBOL:
+ if (ident_equal(in->symbol_name, s))
+ *_in = r;
+ break;
+ case EXPR_IDENTIFIER:
+ if (ident_equal(in->expr_ident, s))
+ *_in = r;
+ break;
+ case EXPR_BINOP:
+ case EXPR_COMMA:
+ case EXPR_ASSIGNMENT:
+ case EXPR_COMPARE:
+ case EXPR_LOGICAL:
+ replace_ident(&in->left, s, r);
+ replace_ident(&in->right, s, r);
+ break;
+ case EXPR_DEREF:
+ replace_ident(&in->deref, s, r);
+ break;
+ case EXPR_PREOP:
+ case EXPR_POSTOP:
+ replace_ident(&in->unop, s, r);
+ break;
+ case EXPR_CAST:
+ case EXPR_FORCE_CAST:
+ case EXPR_IMPLIED_CAST:
+ case EXPR_SIZEOF:
+ replace_ident(&in->cast_expression, s, r);
+ break;
+ case EXPR_SLICE:
+ replace_ident(&in->base, s, r);
+ break;
+ case EXPR_CONDITIONAL:
+ replace_ident(&in->conditional, s, r);
+ replace_ident(&in->cond_true, s, r);
+ replace_ident(&in->cond_false, s, r);
+ break;
+ }
+}
+
/*
* extend to deal with the ambiguous C grammar for parsing
* a cast expressions followed by an initializer.
@@ -570,10 +629,49 @@ static struct token *postfix_expression(
case '(': { /* Function call */
struct expression *call = alloc_expression(token->pos, EXPR_CALL);
+ struct symbol *fn = NULL;
+ struct context *c;
+
+ if (expr->symbol_name)
+ fn = lookup_symbol(expr->symbol_name, NS_SYMBOL);
call->op = '(';
call->fn = expr;
token = expression_list(token->next, &call->args);
token = expect(token, ')', "in function call");
+ if (fn && fn->ctype.base_type) {
+ FOR_EACH_PTR(fn->ctype.contexts, c) {
+ struct context *copy = alloc_context();
+ struct symbol *fn_arg;
+ struct expression *passed_arg;
+
+ copy->in = c->in;
+ copy->out = c->out;
+ copy->exact = c->exact;
+ add_ptr_list(&call->contexts, copy);
+
+ if (!c->token)
+ continue;
+
+ /* re-parse the token at this place */
+ conditional_expression(c->token, ©->context);
+
+ copy->context->pos = expr->pos;
+
+ if (current_assignment_expression)
+ replace_ident(©->context, &RESULT_ident,
+ current_assignment_expression);
+
+ /* and replace the arg */
+ PREPARE_PTR_LIST(fn->ctype.base_type->arguments, fn_arg);
+ FOR_EACH_PTR(call->args, passed_arg);
+ if (fn_arg && passed_arg->type != EXPR_CALL)
+ replace_ident(©->context, fn_arg->ident, passed_arg);
+ NEXT_PTR_LIST(fn_arg);
+ END_FOR_EACH_PTR(passed_arg);
+ FINISH_PTR_LIST(fn_arg);
+ } END_FOR_EACH_PTR(c);
+ }
+
expr = call;
continue;
}
@@ -894,6 +992,7 @@ struct token *conditional_expression(str
struct token *assignment_expression(struct token *token, struct expression **tree)
{
+ struct expression *old_cae = current_assignment_expression;
token = conditional_expression(token, tree);
if (*tree && token_type(token) == TOKEN_SPECIAL) {
static const int assignments[] = {
@@ -904,15 +1003,19 @@ struct token *assignment_expression(stru
SPECIAL_SHR_ASSIGN, SPECIAL_AND_ASSIGN,
SPECIAL_OR_ASSIGN, SPECIAL_XOR_ASSIGN };
int i, op = token->special;
+ if (op == '=')
+ current_assignment_expression = *tree;
for (i = 0; i < sizeof(assignments)/sizeof(int); i++)
if (assignments[i] == op) {
struct expression * expr = alloc_expression(token->pos, EXPR_ASSIGNMENT);
expr->left = *tree;
expr->op = op;
*tree = expr;
- return assignment_expression(token->next, &expr->right);
+ token = assignment_expression(token->next, &expr->right);
+ break;
}
}
+ current_assignment_expression = old_cae;
return token;
}
@@ -929,4 +1032,170 @@ struct token *parse_expression(struct to
return comma_expression(token,tree);
}
+int expressions_equal(const struct expression *expr1,
+ const struct expression *expr2)
+{
+ if (expr1 == expr2)
+ return 1;
+
+ if (expr1 == NULL || expr2 == NULL)
+ return 0;
+ /* Is this the right way to handle casts? */
+ if (expr1->type == EXPR_CAST ||
+ expr1->type == EXPR_FORCE_CAST ||
+ expr1->type == EXPR_IMPLIED_CAST)
+ return expressions_equal(expr1->cast_expression, expr2);
+
+ if (expr2->type == EXPR_CAST ||
+ expr2->type == EXPR_FORCE_CAST ||
+ expr2->type == EXPR_IMPLIED_CAST)
+ return expressions_equal(expr2->cast_expression, expr1);
+
+ if (expr1->type != expr2->type)
+ return 0;
+
+ switch (expr1->type) {
+ case EXPR_SYMBOL:
+ return ident_equal(expr1->symbol_name, expr2->symbol_name);
+
+ case EXPR_VALUE:
+ return expr1->value == expr2->value;
+
+ case EXPR_FVALUE:
+ return expr1->fvalue == expr2->fvalue;
+
+ case EXPR_STRING:
+ return expr1->string->length == expr2->string->length &&
+ !strncmp(expr1->string->data, expr2->string->data, expr1->string->length);
+
+ case EXPR_BINOP:
+ return expr1->op == expr2->op &&
+ expressions_equal(expr1->left, expr2->left) &&
+ expressions_equal(expr1->right, expr2->right);
+
+ case EXPR_COMMA:
+ case EXPR_ASSIGNMENT:
+ return expressions_equal(expr1->left, expr2->left) &&
+ expressions_equal(expr1->right, expr2->right);
+
+ case EXPR_DEREF:
+ return expressions_equal(expr1->deref, expr2->deref) &&
+ ident_equal(expr1->member, expr2->member);
+
+ case EXPR_PREOP:
+ case EXPR_POSTOP:
+ return expr1->op == expr2->op &&
+ expressions_equal(expr1->unop, expr2->unop);
+
+ /* Not needed right now, but for sake of completness ...
+ case EXPR_LABEL:
+ case EXPR_STATEMENT:
+ case EXPR_CALL:
+ case EXPR_LOGICAL:
+ case EXPR_COMPARE:
+ case EXPR_SELECT:
+ case EXPR_CONDITIONAL:
+ case EXPR_CAST:
+ case EXPR_FORCE_CAST:
+ case EXPR_IMPLIED_CAST:
+ case EXPR_SLICE:
+ case EXPR_INITIALIZER:
+ case EXPR_POS:
+ */
+
+ default:
+ /* nothing, we should already have had a warning */
+ ;
+ }
+
+ return 0;
+}
+
+static int ident_str(struct ident *ident, char *buffer, int length)
+{
+ if (!ident)
+ return 0;
+ return snprintf(buffer, length, "%.*s", ident->len, ident->name);
+}
+
+int expression_str(const struct expression *expr,
+ char *buffer, int length)
+{
+ int n;
+
+ memset(buffer, 0, length);
+
+ if (!expr)
+ return 0;
+
+ /* TODO, think about necessary parentheses () */
+
+ switch (expr->type) {
+ case EXPR_SYMBOL:
+ return ident_str(expr->symbol_name, buffer, length);
+
+ case EXPR_VALUE:
+ return snprintf(buffer, length, "%llu", expr->value);
+
+ case EXPR_FVALUE:
+ return snprintf(buffer, length, "%Lf", expr->fvalue);
+
+ case EXPR_STRING:
+ return snprintf(buffer, length, "\"%.*s\"", expr->string->length, expr->string->data);
+
+ case EXPR_BINOP:
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, "%c", expr->op);
+ n += expression_str(expr->right, buffer+n, length-n);
+ return n;
+
+ case EXPR_COMMA:
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, ",");
+ n += expression_str(expr->right, buffer+n, length-n);
+ return n;
+
+ case EXPR_ASSIGNMENT:
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, "=");
+ n += expression_str(expr->right, buffer+n, length-n);
+ return n;
+
+ case EXPR_DEREF:
+ if (expr->left->type == EXPR_PREOP &&
+ expr->left->op == '*') {
+ n = expression_str(expr->left->unop, buffer, length);
+ n += snprintf(buffer+n, length-n, "->");
+ n += ident_str(expr->member, buffer+n, length-n);
+ } else {
+ n = expression_str(expr->left, buffer, length);
+ n += snprintf(buffer+n, length-n, ".");
+ n += ident_str(expr->member, buffer+n, length-n);
+ }
+ return n;
+
+ case EXPR_PREOP:
+ n = snprintf(buffer, length, "%c", expr->op);
+ n += expression_str(expr->unop, buffer+n, length-n);
+ return n;
+
+ case EXPR_POSTOP:
+ n = expression_str(expr->unop, buffer, length);
+ n += snprintf(buffer+n, length-n, "%c", expr->op);
+ return n;
+
+ case EXPR_CAST:
+ case EXPR_FORCE_CAST:
+ case EXPR_IMPLIED_CAST:
+ /* todo: print out the cast type's ctype */
+ *buffer++ = '('; length--;
+ *buffer++ = ')'; length--;
+ return expression_str(expr->cast_expression, buffer, length) + 2;
+
+ default:
+ printf("Missing code in expression_str for %d\n", expr->type);
+ }
+
+ return 0;
+}
--- sparse.orig/expression.h 2008-09-10 08:56:01.000000000 +0200
+++ sparse/expression.h 2008-09-10 09:06:18.000000000 +0200
@@ -121,6 +121,7 @@ struct expression {
struct /* call_expr */ {
struct expression *fn;
struct expression_list *args;
+ struct context_list *contexts;
};
// EXPR_LABEL
struct /* label_expr */ {
@@ -216,4 +217,8 @@ struct token *compound_statement(struct
void cast_value(struct expression *expr, struct symbol *newtype,
struct expression *old, struct symbol *oldtype);
+int expressions_equal(const struct expression *expr1,
+ const struct expression *expr2);
+int expression_str(const struct expression *, char *buf, int buflen);
+extern struct expression *current_assignment_expression;
#endif
--- sparse.orig/sparse.c 2008-09-10 08:56:02.000000000 +0200
+++ sparse/sparse.c 2008-09-10 09:13:55.000000000 +0200
@@ -26,7 +26,7 @@
struct context_check {
int val;
- char name[32];
+ const struct expression *expr;
};
DECLARE_ALLOCATOR(context_check);
@@ -34,22 +34,15 @@ DECLARE_PTR_LIST(context_check_list, str
DECLARE_PTR_LIST(context_list_list, struct context_check_list);
ALLOCATOR(context_check, "context check list");
-static const char *unnamed_context = "<unnamed>";
-static const char *context_name(struct context *context)
-{
- if (context->context && context->context->symbol_name)
- return show_ident(context->context->symbol_name);
- return unnamed_context;
-}
-
-static void context_add(struct context_check_list **ccl, const char *name,
+static void context_add(struct context_check_list **ccl,
+ const struct expression *expr,
int offs)
{
struct context_check *check, *found = NULL;
FOR_EACH_PTR(*ccl, check) {
- if (strcmp(name, check->name))
+ if (!expressions_equal(expr, check->expr))
continue;
found = check;
break;
@@ -57,8 +50,7 @@ static void context_add(struct context_c
if (!found) {
found = __alloc_context_check(0);
- strncpy(found->name, name, sizeof(found->name));
- found->name[sizeof(found->name) - 1] = '\0';
+ found->expr = expr;
add_ptr_list(ccl, found);
}
found->val += offs;
@@ -70,7 +62,7 @@ static int context_list_has(struct conte
struct context_check *check;
FOR_EACH_PTR(ccl, check) {
- if (strcmp(c->name, check->name))
+ if (!expressions_equal(c->expr, check->expr))
continue;
return check->val == c->val;
} END_FOR_EACH_PTR(check);
@@ -105,7 +97,7 @@ static struct context_check_list *checke
struct context_check *c;
FOR_EACH_PTR(ccl, c) {
- context_add(&result, c->name, c->val);
+ context_add(&result, c->expr, c->val);
} END_FOR_EACH_PTR(c);
return result;
@@ -115,15 +107,15 @@ static struct context_check_list *checke
#define CONTEXT_PROB "context problem in '%s': "
#define DEFAULT_CONTEXT_DESCR " default context: "
-static void get_context_string(char **buf, const char **name)
+static const char *get_context_string(char **buf, const char *name)
{
- if (strcmp(*name, unnamed_context)) {
- *buf = malloc(strlen(*name) + 16);
- sprintf(*buf, " context '%s': ", *name);
- *name = *buf;
+ if (strlen(name)) {
+ *buf = malloc(strlen(name) + 16);
+ sprintf(*buf, " context '%s': ", name);
+ return *buf;
} else {
- *name = DEFAULT_CONTEXT_DESCR;
*buf = NULL;
+ return DEFAULT_CONTEXT_DESCR;
}
}
@@ -133,12 +125,13 @@ static int context_list_check(struct ent
{
struct context_check *c1, *c2;
int cur, tgt;
- const char *name;
+ char name[1000];
char *buf;
+ const char *pname;
/* make sure the loop below checks all */
FOR_EACH_PTR(ccl_target, c1) {
- context_add(&ccl_cur, c1->name, 0);
+ context_add(&ccl_cur, c1->expr, 0);
} END_FOR_EACH_PTR(c1);
FOR_EACH_PTR(ccl_cur, c1) {
@@ -146,7 +139,7 @@ static int context_list_check(struct ent
tgt = 0;
FOR_EACH_PTR(ccl_target, c2) {
- if (strcmp(c2->name, c1->name))
+ if (!expressions_equal(c1->expr, c2->expr))
continue;
tgt = c2->val;
break;
@@ -162,11 +155,11 @@ static int context_list_check(struct ent
warning(pos, IMBALANCE_IN "unexpected unlock",
show_ident(ep->name->ident));
- name = c1->name;
- get_context_string(&buf, &name);
+ expression_str(c1->expr, name, sizeof(name));
+ pname = get_context_string(&buf, name);
info(pos, "%swanted %d, got %d",
- name, tgt, cur);
+ pname, tgt, cur);
free(buf);
@@ -176,83 +169,21 @@ static int context_list_check(struct ent
return 0;
}
-static int handle_call(struct entrypoint *ep, struct basic_block *bb,
- struct instruction *insn,
- struct context_check_list *combined)
-{
- struct context *ctx;
- struct context_check *c;
- const char *name, *call, *cmp;
- char *buf;
- int val, ok;
-
- if (!insn->func || !insn->func->sym ||
- insn->func->type != PSEUDO_SYM)
- return 0;
-
- /*
- * Check all contexts the function wants.
- */
- FOR_EACH_PTR(insn->func->sym->ctype.contexts, ctx) {
- name = context_name(ctx);
- val = 0;
-
- FOR_EACH_PTR(combined, c) {
- if (strcmp(c->name, name) == 0) {
- val = c->val;
- break;
- }
- } END_FOR_EACH_PTR(c);
-
- if (ctx->exact) {
- ok = ctx->in == val;
- cmp = "";
- } else {
- ok = ctx->in <= val;
- cmp = ">= ";
- }
-
- if (!ok && Wcontext) {
- get_context_string(&buf, &name);
- call = strdup(show_ident(insn->func->ident));
-
- warning(insn->pos, "context problem in '%s': "
- "'%s' expected different context",
- show_ident(ep->name->ident), call);
-
- info(insn->pos, "%swanted %s%d, got %d",
- name, cmp, ctx->in, val);
-
- free((void *)call);
- free(buf);
-
- return -1;
- }
- } END_FOR_EACH_PTR (ctx);
-
- return 0;
-}
-
static int handle_context(struct entrypoint *ep, struct basic_block *bb,
struct instruction *insn,
struct context_check_list **combined)
{
struct context_check *c;
- const char *name, *cmp;
+ const char *cmp, *pname;
char *buf;
+ char name[1000];
int val, ok;
val = 0;
- name = unnamed_context;
- if (insn->context_expr)
- name = show_ident(insn->context_expr->symbol_name);
-
FOR_EACH_PTR(*combined, c) {
- if (strcmp(c->name, name) == 0) {
+ if (expressions_equal(c->expr, insn->context_expr))
val = c->val;
- break;
- }
} END_FOR_EACH_PTR(c);
if (insn->exact) {
@@ -264,7 +195,8 @@ static int handle_context(struct entrypo
}
if (!ok && Wcontext) {
- get_context_string(&buf, &name);
+ expression_str(insn->context_expr, name, sizeof(name));
+ pname = get_context_string(&buf, name);
if (insn->access_var) {
char *symname = strdup(show_ident(insn->access_var->ident));
@@ -273,6 +205,13 @@ static int handle_context(struct entrypo
"access to '%s' requires different context",
show_ident(ep->name->ident), symname);
free(symname);
+ } else if (insn->called_fn) {
+ char *symname = strdup(show_ident(insn->called_fn->ident));
+ warning(insn->pos,
+ CONTEXT_PROB
+ "'%s' expected different context",
+ show_ident(ep->name->ident), symname);
+ free(symname);
} else {
warning(insn->pos,
CONTEXT_PROB
@@ -281,13 +220,13 @@ static int handle_context(struct entrypo
}
info(insn->pos, "%swanted %s%d, got %d",
- name, cmp, insn->required, val);
+ pname, cmp, insn->required, val);
free(buf);
return -1;
}
- context_add(combined, name, insn->increment);
+ context_add(combined, insn->context_expr, insn->increment);
return 0;
}
@@ -324,7 +263,7 @@ static int check_bb_context(struct entry
* for the conditional_context() attribute.
*/
FOR_EACH_PTR(ccl_in, c) {
- context_add(&combined, c->name, c->val);
+ context_add(&combined, c->expr, c->val);
} END_FOR_EACH_PTR(c);
/* Add the new context to the list of already-checked contexts */
@@ -338,11 +277,6 @@ static int check_bb_context(struct entry
*/
FOR_EACH_PTR(bb->insns, insn) {
switch (insn->opcode) {
- case OP_INLINED_CALL:
- case OP_CALL:
- if (handle_call(ep, bb, insn, combined))
- goto out;
- break;
case OP_CONTEXT:
if (handle_context(ep, bb, insn, &combined))
goto out;
@@ -571,10 +505,10 @@ static void check_context(struct entrypo
check_instructions(ep);
FOR_EACH_PTR(sym->ctype.contexts, context) {
- const char *name = context_name(context);
-
- context_add(&ccl_in, name, context->in);
- context_add(&ccl_target, name, context->out);
+ context_add(&ccl_in, context->in_fn,
+ context->in);
+ context_add(&ccl_target, context->in_fn,
+ context->out);
} END_FOR_EACH_PTR(context);
check_bb_context(ep, ep->entry->bb, ccl_in, ccl_target);
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ sparse/validation/context-vars.c 2008-09-10 09:13:56.000000000 +0200
@@ -0,0 +1,171 @@
+static void a(void *p) __attribute__((context(p,0,1)))
+{
+ __context__(p,1);
+}
+
+static void r(void *p) __attribute__((context(p,1,0)))
+{
+ __context__(p,-1,1);
+}
+
+extern void *l1, *l2;
+
+static void good_paired1(void)
+{
+ a(l1);
+ r(l1);
+}
+
+static void good_paired2(void)
+{
+ a(l1);
+ r(l1);
+ a(l1);
+ r(l1);
+ a(l2);
+ r(l2);
+}
+
+static void good_paired3(void)
+{
+ a(l1);
+ a(l2);
+ r(l2);
+ r(l1);
+}
+
+static void good_lock1(void *lp) __attribute__((context(lp,0,1)))
+{
+ a(lp);
+}
+
+static void good_lock2(void *lp) __attribute__((context(lp,0,1)))
+{
+ a(lp);
+ r(lp);
+ a(lp);
+}
+
+extern void **v;
+
+static void warn_lock1(void)
+{
+ a(v[1]);
+}
+
+extern int condition;
+
+static void good_lock3(void)
+{
+ a(v[1]);
+ if (condition) {
+ a(v[2]);
+ r(v[2]);
+ }
+ r(v[1]);
+}
+
+#define cond_lock(x, c)\
+ ((c) ? ({ __context__(x,1); 1; }) : 0)
+
+#define trylock(x) \
+ cond_lock(x, _try_lock(x))
+
+extern void _try_lock(int *x);
+
+static int good_condlock1(void)
+{
+ if (trylock(v[0]))
+ r(v[0]);
+}
+
+static int good_condlock2(void)
+{
+ if (trylock(&condition))
+ r((void*)&condition);
+}
+
+extern void ai(int *p) __attribute__((context(p,0,1)));
+extern void ri(int *p) __attribute__((context(p,1,0)));
+
+struct test {
+ int lock;
+};
+
+static inline void unlock(struct test *y)
+ __attribute__((context(&y->lock,1,0)))
+{
+ ri(&y->lock);
+}
+
+static void good_lock4(struct test *t)
+ __attribute__((context(&t->lock,0,1)))
+{
+ ai(&t->lock);
+}
+
+extern int *find_locked(void) __attribute__((context(RESULT,0,1)));
+
+static void good_find_release1(void)
+{
+ int *p;
+
+ p = find_locked();
+ ri(p);
+}
+
+extern struct test *find_locked_struct(void) __attribute__((context(&RESULT->lock,0,1)));
+
+static void good_find_release2(void)
+{
+ struct test *t;
+
+ t = find_locked_struct();
+ unlock(t);
+}
+
+static void good_find_release3(void)
+{
+ struct test *t = find_locked_struct();
+ unlock(t);
+}
+
+static void warn_unlock(void)
+{
+ struct test *t;
+ /* check that the warning is here, not at the unlock definition */
+ unlock(t);
+}
+
+extern int _locked_struct_test(struct test *ls, int *flags);
+#define locked_struct_test(ls, flags) (( _locked_struct_test((ls), (flags)) ) ? ({ __context__(&ls->lock,1); 1;}) : 0)
+
+static inline void unlock2(struct test *y)
+ __attribute__((context(&y->lock,1,0)))
+{
+ unlock(y);
+}
+
+static void good_locked_val(void)
+{
+ struct test *t;
+ int flags;
+
+ if (!t || !locked_struct_test(t, &flags))
+ goto out;
+
+ unlock2(t);
+ out:
+ ;
+}
+
+/*
+ * check-name: Check -Wcontext with lock variables
+ *
+ * check-error-start
+context-vars.c:53:7: warning: context imbalance in 'warn_lock1': wrong count at exit
+context-vars.c:53:7: context '**v+4': wanted 0, got 1
+context-vars.c:137:11: warning: context problem in 'warn_unlock': 'unlock' expected different context
+context-vars.c:137:11: context '*t+0': wanted >= 1, got 0
+ * check-error-end
+ */
--- sparse.orig/evaluate.c 2008-09-10 08:56:02.000000000 +0200
+++ sparse/evaluate.c 2008-09-10 08:56:34.000000000 +0200
@@ -2954,8 +2954,16 @@ struct symbol *evaluate_expression(struc
return evaluate_alignof(expr);
case EXPR_DEREF:
return evaluate_member_dereference(expr);
- case EXPR_CALL:
+ case EXPR_CALL: {
+ struct context *c;
+ FOR_EACH_PTR(expr->contexts, c) {
+ if (c->context &&
+ (c->context->type != EXPR_SYMBOL ||
+ c->context->symbol))
+ evaluate_expression(c->context);
+ } END_FOR_EACH_PTR(c);
return evaluate_call(expr);
+ }
case EXPR_SELECT:
case EXPR_CONDITIONAL:
return evaluate_conditional_expression(expr);
@@ -3025,6 +3033,7 @@ static void check_duplicates(struct symb
static struct symbol *evaluate_symbol(struct symbol *sym)
{
struct symbol *base_type;
+ struct context *c;
if (!sym)
return sym;
@@ -3054,6 +3063,13 @@ static struct symbol *evaluate_symbol(st
evaluate_statement(base_type->stmt);
current_fn = curr;
+
+ FOR_EACH_PTR(sym->ctype.contexts, c) {
+ if (c->in_fn &&
+ (c->in_fn->type != EXPR_SYMBOL ||
+ c->in_fn->symbol))
+ evaluate_expression(c->in_fn);
+ } END_FOR_EACH_PTR(c);
}
return base_type;
--- sparse.orig/expand.c 2008-09-10 08:56:02.000000000 +0200
+++ sparse/expand.c 2008-09-10 08:56:34.000000000 +0200
@@ -1034,8 +1034,14 @@ int expand_symbol(struct symbol *sym)
retval = expand_expression(sym->initializer);
/* expand the body of the symbol */
if (base_type->type == SYM_FN) {
+ struct context *c;
+
if (base_type->stmt)
expand_statement(base_type->stmt);
+
+ FOR_EACH_PTR(sym->ctype.contexts, c) {
+ expand_expression(c->in_fn);
+ } END_FOR_EACH_PTR(c);
}
return retval;
}
--- sparse.orig/parse.c 2008-09-10 08:56:01.000000000 +0200
+++ sparse/parse.c 2008-09-10 08:56:34.000000000 +0200
@@ -879,12 +879,13 @@ static struct token *_attribute_context(
struct context *context = alloc_context();
struct expression *args[4];
int argc = 0;
- struct token *last = NULL;
+ struct token *first = NULL, *last = NULL;
token = expect(token, '(', "after context attribute");
while (!match_op(token, ')')) {
struct expression *expr = NULL;
last = token;
+ first = first ? : token;
token = conditional_expression(token, &expr);
if (!expr)
break;
@@ -910,12 +911,14 @@ static struct token *_attribute_context(
break;
case 3:
context->context = args[0];
+ context->token = first;
context->in = get_expression_value(args[1]);
context->out = get_expression_value(args[2]);
break;
case 4: {
const char *rw;
context->context = args[0];
+ context->token = first;
context->in = get_expression_value(args[1]);
context->out = get_expression_value(args[2]);
@@ -2096,6 +2099,7 @@ static struct token *parse_function_body
struct symbol *base_type = decl->ctype.base_type;
struct statement *stmt, **p;
struct symbol *arg;
+ struct context *c;
old_symbol_list = function_symbol_list;
if (decl->ctype.modifiers & MOD_INLINE) {
@@ -2124,6 +2128,11 @@ static struct token *parse_function_body
token = compound_statement(token->next, stmt);
+ FOR_EACH_PTR(decl->ctype.contexts, c) {
+ if (c->token)
+ conditional_expression(c->token, &c->in_fn);
+ } END_FOR_EACH_PTR(c);
+
end_function(decl);
if (!(decl->ctype.modifiers & MOD_INLINE))
add_symbol(list, decl);
@@ -2286,11 +2295,17 @@ struct token *external_declaration(struc
for (;;) {
if (!is_typedef && match_op(token, '=')) {
+ struct expression *old_cae = current_assignment_expression;
if (decl->ctype.modifiers & MOD_EXTERN) {
warning(decl->pos, "symbol with external linkage has initializer");
decl->ctype.modifiers &= ~MOD_EXTERN;
}
+ current_assignment_expression = alloc_expression(token->pos, EXPR_SYMBOL);
+ current_assignment_expression->symbol_name = decl->ident;
+ current_assignment_expression->symbol =
+ lookup_symbol(decl->ident, NS_SYMBOL | NS_TYPEDEF);
token = initializer(&decl->initializer, token->next);
+ current_assignment_expression = old_cae;
}
if (!is_typedef) {
if (!(decl->ctype.modifiers & (MOD_EXTERN | MOD_INLINE))) {
--- sparse.orig/symbol.h 2008-09-10 08:56:01.000000000 +0200
+++ sparse/symbol.h 2008-09-10 08:56:34.000000000 +0200
@@ -77,6 +77,10 @@ enum context_read_write_specifier {
struct context {
struct expression *context;
+ /* store the token pointer */
+ struct token *token;
+ /* to re-evaluate this context within the function it is for */
+ struct expression *in_fn;
unsigned int in, out;
int exact;
enum context_read_write_specifier rws;
--- sparse.orig/inline.c 2008-09-10 08:56:01.000000000 +0200
+++ sparse/inline.c 2008-09-10 09:00:00.000000000 +0200
@@ -56,7 +56,7 @@ static struct symbol_list *copy_symbol_l
return dst;
}
-static struct expression * copy_expression(struct expression *expr)
+static struct expression *copy_expression(struct expression *expr)
{
if (!expr)
return NULL;
@@ -474,6 +474,7 @@ void copy_statement(struct statement *sr
dst->args = copy_one_statement(src->args);
dst->ret = copy_symbol(src->pos, src->ret);
dst->inline_fn = src->inline_fn;
+ dst->inlined_fn_expr = src->inlined_fn_expr;
}
static struct symbol *create_copy_symbol(struct symbol *orig)
@@ -555,6 +556,7 @@ int inline_function(struct expression *e
stmt->args = decl;
}
stmt->inline_fn = sym;
+ stmt->inlined_fn_expr = expr;
unset_replace_list(fn_symbol_list);
--- sparse.orig/linearize.c 2008-09-10 08:56:02.000000000 +0200
+++ sparse/linearize.c 2008-09-10 09:13:56.000000000 +0200
@@ -35,6 +35,7 @@ static pseudo_t linearize_initializer(st
struct pseudo void_pseudo = {};
static struct position current_pos;
+static int in_inline_skip_context = 0;
ALLOCATOR(pseudo_user, "pseudo_user");
@@ -436,9 +437,15 @@ const char *show_instruction(struct inst
buf += sprintf(buf, "%s <- %s", show_pseudo(insn->target), show_pseudo(insn->src1));
break;
- case OP_CONTEXT:
+ case OP_CONTEXT: {
+ char ctxbuf[100];
+ if (insn->context_expr) {
+ expression_str(insn->context_expr, ctxbuf, sizeof(ctxbuf));
+ buf += sprintf(buf, "%s, ", ctxbuf);
+ }
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));
break;
@@ -1280,7 +1287,7 @@ static pseudo_t linearize_call_expressio
struct instruction *insn = alloc_typed_instruction(OP_CALL, expr->ctype);
pseudo_t retval, call;
struct ctype *ctype = NULL;
- struct context *context;
+ struct context *c;
if (!expr->ctype) {
warning(expr->pos, "call with no type!");
@@ -1316,29 +1323,17 @@ static pseudo_t linearize_call_expressio
insn->target = retval;
add_one_insn(ep, insn);
- if (ctype) {
- FOR_EACH_PTR(ctype->contexts, context) {
- int in = context->in;
- int out = context->out;
- 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 = context_diff;
- insn->context_expr = context->context;
- add_one_insn(ep, insn);
- }
- } END_FOR_EACH_PTR(context);
- }
+ if (!in_inline_skip_context)
+ FOR_EACH_PTR(expr->contexts, c) {
+ struct instruction *tmp = alloc_instruction(OP_CONTEXT, 0);
+ tmp->increment = c->out - c->in;
+ tmp->exact = c->exact;
+ tmp->context_expr = c->context;
+ tmp->required = c->in;
+ tmp->called_fn = call->sym;
+ tmp->pos = insn->pos;
+ add_one_insn(ep, tmp);
+ } END_FOR_EACH_PTR(c);
return retval;
}
@@ -1712,6 +1707,8 @@ static pseudo_t linearize_compound_state
pseudo = VOID;
FOR_EACH_PTR(stmt->stmts, s) {
+ if (in_inline_skip_context && s->type == STMT_CONTEXT)
+ continue;
pseudo = linearize_statement(ep, s);
} END_FOR_EACH_PTR(s);
@@ -1739,6 +1736,7 @@ static pseudo_t linearize_inlined_call(s
struct statement *args = stmt->args;
struct basic_block *bb;
pseudo_t pseudo;
+ struct context *c;
if (args) {
struct symbol *sym;
@@ -1750,12 +1748,27 @@ static pseudo_t linearize_inlined_call(s
} END_FOR_EACH_PTR(sym);
}
+ in_inline_skip_context++;
insn->target = pseudo = linearize_compound_statement(ep, stmt);
+ in_inline_skip_context--;
use_pseudo(insn, symbol_pseudo(ep, stmt->inline_fn), &insn->func);
bb = ep->active;
if (bb && !bb->insns)
bb->pos = stmt->pos;
add_one_insn(ep, insn);
+
+ if (!in_inline_skip_context)
+ FOR_EACH_PTR(stmt->inlined_fn_expr->contexts, c) {
+ struct instruction *tmp = alloc_instruction(OP_CONTEXT, 0);
+ tmp->increment = c->out - c->in;
+ tmp->exact = c->exact;
+ tmp->context_expr = c->context;
+ tmp->required = c->in;
+ tmp->called_fn = stmt->inline_fn;
+ tmp->pos = insn->pos;
+ add_one_insn(ep, tmp);
+ } END_FOR_EACH_PTR(c);
+
return pseudo;
}
--- sparse.orig/parse.h 2008-09-10 08:56:02.000000000 +0200
+++ sparse/parse.h 2008-09-10 08:56:34.000000000 +0200
@@ -61,6 +61,7 @@ struct statement {
struct symbol *ret;
struct symbol *inline_fn;
struct statement *args;
+ struct expression *inlined_fn_expr;
};
struct /* labeled_struct */ {
struct symbol *label_identifier;
--- sparse.orig/linearize.h 2008-09-10 08:56:02.000000000 +0200
+++ sparse/linearize.h 2008-09-10 08:56:34.000000000 +0200
@@ -119,6 +119,7 @@ struct instruction {
int increment, required, exact;
struct expression *context_expr;
struct symbol *access_var;
+ struct symbol *called_fn;
};
struct /* asm */ {
const char *string;
--- sparse.orig/ident-list.h 2008-09-10 08:56:02.000000000 +0200
+++ sparse/ident-list.h 2008-09-10 08:56:34.000000000 +0200
@@ -98,6 +98,7 @@ __IDENT(__PRETTY_FUNCTION___ident, "__PR
IDENT_RESERVED(__context__);
IDENT_RESERVED(__exact_context__);
IDENT_RESERVED(__range__);
+IDENT_RESERVED(RESULT);
/* Magic function names we recognize */
IDENT(memset); IDENT(memcpy);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9 v2] check context expressions as expressions
2008-09-10 7:33 ` [PATCH 6/9 v2] " Johannes Berg
@ 2008-09-10 19:21 ` Christopher Li
2008-09-10 21:34 ` Johannes Berg
0 siblings, 1 reply; 27+ messages in thread
From: Christopher Li @ 2008-09-10 19:21 UTC (permalink / raw)
To: Johannes Berg
Cc: Josh Triplett, Philipp Reisner, linux-sparse, Harvey Harrison
On Wed, Sep 10, 2008 at 12:33 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> +static int ident_equal(struct ident *ident1, struct ident *ident2)
> +{
> + if (ident1 == ident2)
> + return 1;
> + if (!ident1 || !ident2)
> + return 0;
> +
> + return ident1->len == ident2->len &&
> + !strncmp(ident1->name, ident2->name, ident1->len);
> +}
Nah, you pretty should never need to do that.
Ident equal should just need to compare ident1 == ident2.
All ident has been hashed. Same ident, should show up
as same pointer. Doing strncmp again is unnecessary.
I feel that this patch is adding too much hack to the
sparse front end. At the very least, can you
just change __context__ parsing to accept symbol expression
and the let the checking back end to do the code motions
stuff?
I haven't study the new context checking very carefully.
I actually prefer the Linus's old simple context checking.
Yes, that does not distinguish which symbols taking the lock.
But other than that it is working and counting the number
correctly. And, it is very simple.
The new context checking seems able to do more features.
But it has too many sore spots. I vote for back it out.
Instead of keep adding more hacks to fix up the problem. I
think we should step back and ask ourselves what do we really
want to achieve.
The fundamental problem I saw here is that, sparse does not
support cross function checking. There is no good way to save
some analyzed result for some function and used it later by other
function. That is why we actually have to put __context__
around so many functions. The __context__ describe what
these functions in forms of source code annotation. There is
only so much we can do with source code annotations.
I am not saying that annotation is not useful. I agree source
code annotation helps on the source code reading. But it
shouldn't limit checker only use the annotations. The checker
should be able to draw intelligent conclusions, by looking the
the function source code itself.
e.g. Why do we have to annotate foo_lock(&bar->lock) will take a lock
on &bar->lock? The checker should be able to find out foo_lock() just
callers some lower level locking functions. For example, let say if we
can force foo to be inlined into the caller even foo is not declared as
inline. Then there is no need to annotate foo itself. The caller see
exactly what foo does.
So, I think we should implement cross functions checking capability
systematically rather than putting more hacks on source code
annotation. The writer patch I send out earlier is one step towards it.
I will write up more detail proposal.
Chris
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9 v2] check context expressions as expressions
2008-09-10 19:21 ` Christopher Li
@ 2008-09-10 21:34 ` Johannes Berg
2008-09-11 0:15 ` Christopher Li
0 siblings, 1 reply; 27+ messages in thread
From: Johannes Berg @ 2008-09-10 21:34 UTC (permalink / raw)
To: Christopher Li
Cc: Josh Triplett, Philipp Reisner, linux-sparse, Harvey Harrison
[-- Attachment #1: Type: text/plain, Size: 3953 bytes --]
On Wed, 2008-09-10 at 12:21 -0700, Christopher Li wrote:
> Nah, you pretty should never need to do that.
> Ident equal should just need to compare ident1 == ident2.
>
> All ident has been hashed. Same ident, should show up
> as same pointer. Doing strncmp again is unnecessary.
>
> I feel that this patch is adding too much hack to the
> sparse front end. At the very least, can you
> just change __context__ parsing to accept symbol expression
> and the let the checking back end to do the code motions
> stuff?
Can you explain?
> I haven't study the new context checking very carefully.
> I actually prefer the Linus's old simple context checking.
> Yes, that does not distinguish which symbols taking the lock.
> But other than that it is working and counting the number
> correctly. And, it is very simple.
Well, yeah, it's very simple, and that doesn't help. And it's not about
cross-function checking either:
extern spinlock_t a;
spin_lock(&a);
rcu_read_unlock()
will not result in an error which is a pain.
> The new context checking seems able to do more features.
> But it has too many sore spots. I vote for back it out.
I don't care, I don't have any more cycles to burn on this. For all I'm
concerned it works pretty well and is a HUGE improvement over the
current sparse which
* doesn't tell you what line the error really is in, it only warns
about the last line of the function
* isn't able to check different contexts despite documenting it
(see above)
> Instead of keep adding more hacks to fix up the problem. I
> think we should step back and ask ourselves what do we really
> want to achieve.
See above.
> The fundamental problem I saw here is that, sparse does not
> support cross function checking. There is no good way to save
> some analyzed result for some function and used it later by other
> function. That is why we actually have to put __context__
> around so many functions. The __context__ describe what
> these functions in forms of source code annotation. There is
> only so much we can do with source code annotations.
> I am not saying that annotation is not useful. I agree source
> code annotation helps on the source code reading. But it
> shouldn't limit checker only use the annotations. The checker
> should be able to draw intelligent conclusions, by looking the
> the function source code itself.
Actually, I kinda disagree. Annotating *each and every function* with
the locks it requires is _very_ useful, not just for sparse but also for
human readers of the code. Hence, I don't just want sparse to check
across functions, I want to be able to tell people what to do for
calling a given function as well. The fact that sparse can check my
annotations is great, but having them would be useful without that as
well.
> e.g. Why do we have to annotate foo_lock(&bar->lock) will take a lock
> on &bar->lock? The checker should be able to find out foo_lock() just
> callers some lower level locking functions. For example, let say if we
> can force foo to be inlined into the caller even foo is not declared as
> inline. Then there is no need to annotate foo itself. The caller see
> exactly what foo does.
It used to be like that. But I'm arguing (and others backed me up on
this) that inlines are really just functions and as such should be
annotated, not magically inlined into the code and then checked as
sparse behaves right now.
> So, I think we should implement cross functions checking capability
> systematically rather than putting more hacks on source code
> annotation. The writer patch I send out earlier is one step towards it.
> I will write up more detail proposal.
Please don't focus on cross-function checking so much. Having different
contexts is way more desirable, and cross-function checking probably is
_not_ desirable in that you _want_ the annotations.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9 v2] check context expressions as expressions
2008-09-10 21:34 ` Johannes Berg
@ 2008-09-11 0:15 ` Christopher Li
0 siblings, 0 replies; 27+ messages in thread
From: Christopher Li @ 2008-09-11 0:15 UTC (permalink / raw)
To: Johannes Berg
Cc: Josh Triplett, Philipp Reisner, linux-sparse, Harvey Harrison
On Wed, Sep 10, 2008 at 2:34 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>> I feel that this patch is adding too much hack to the
>> sparse front end. At the very least, can you
>> just change __context__ parsing to accept symbol expression
>> and the let the checking back end to do the code motions
>> stuff?
>
> Can you explain?
Maybe you can help to explain why do you need to make so
many front end(parser) changes. I especially don't like the part
that does the replace ident.
From what I can tell, you just need to know that context are
using the same expression on accessing the locks. Let's put
aside that it is right assumption or not. From the back end side,
you should able to get to all the expression and verify they are
the same without doing much front end changes.
All the code for "expression is equal test" should be move to
back end using linearized byte code. As matter of fact, sparse
have a simple common expression eliminate already.
> Well, yeah, it's very simple, and that doesn't help. And it's not about
> cross-function checking either:
>
> extern spinlock_t a;
>
> spin_lock(&a);
> rcu_read_unlock()
As I said, it is simple and does not take into account of which symbol
take the lock. But using same expression is not solid either.
How about:
extern spinlock_t a;
spinlock_t *p = &a;
spin_lock(&a);
spin_unlock(p);
> will not result in an error which is a pain.
>
>> The new context checking seems able to do more features.
>> But it has too many sore spots. I vote for back it out.
>
> I don't care, I don't have any more cycles to burn on this. For all I'm
> concerned it works pretty well and is a HUGE improvement over the
> current sparse which
> * doesn't tell you what line the error really is in, it only warns
> about the last line of the function
> * isn't able to check different contexts despite documenting it
> (see above)
You should able to do the same thing in the linearized back byte code.
You just linearized the context expression using pseudo. Then on
the checker side you can use the byte code to evaluate the expression
is same or not, by comparing expression instructions.
You can even get around the p=&a problem I mention above because
in the linearized byte code level, you can do some real data flow
analyze.
I think it is unnecessary to change the front end code to track
expression. In the back end you can do that much better.
That is the main reason I object to this patch.
>> I am not saying that annotation is not useful. I agree source
>> code annotation helps on the source code reading. But it
>
> Actually, I kinda disagree. Annotating *each and every function* with
> the locks it requires is _very_ useful, not just for sparse but also for
> human readers of the code. Hence, I don't just want sparse to check
> across functions, I want to be able to tell people what to do for
> calling a given function as well. The fact that sparse can check my
> annotations is great, but having them would be useful without that as
> well.
I don't think we have disagreement here. I do agree having source
code annotation is good for human reading. Please read my email.
But that shouldn't stop you having the checker to understand your code.
Even better, how do you know your annotation is in sync with your
code? You can have the checker to warn you that your annotation
is not in sync with code.
>
>
> It used to be like that. But I'm arguing (and others backed me up on
> this) that inlines are really just functions and as such should be
> annotated, not magically inlined into the code and then checked as
> sparse behaves right now.
First of all, I am just give it as example of idea how this problem can
be solved.
About annotate inline function or not, I don't have strong preference
against it. But I can see it is imposing a burden to maintain the additional
annotation. Now you update the code and you have to update the annotation
as well. You require developers who write new function to understand
the spase context checking and maintain the annotation as well.
You are creating addition work for kernel developers.
> Please don't focus on cross-function checking so much. Having different
> contexts is way more desirable, and cross-function checking probably is
> _not_ desirable in that you _want_ the annotations.
That is not what I said, you can keep your precious annotations for
human reading. But that does not mean sparse checker should limited
itself to only using annotations if it can be smarter about it.
Having cross-function checking is not exclusive to the annotations.
But having cross-function checking can enable some very useful
feature which we can't do right now.
Chris
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2008-09-11 0:15 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-29 8:54 [PATCH 0/9] context tracking updates Johannes Berg
2008-05-29 8:54 ` [PATCH 1/9] add test for acquire/release Johannes Berg
2008-05-29 8:54 ` [PATCH 2/9] add __exact_context__ Johannes Berg
2008-05-29 8:54 ` [PATCH 3/9] allow context() attribute on variables Johannes Berg
2008-05-29 8:54 ` [PATCH 4/9] evaluate/expand context expressions Johannes Berg
2008-05-29 8:54 ` [PATCH 5/9] revert the conditional_context patch Johannes Berg
2008-05-29 8:54 ` [PATCH 6/9] check context expressions as expressions Johannes Berg
2008-09-10 7:33 ` [PATCH 6/9 v2] " Johannes Berg
2008-09-10 19:21 ` Christopher Li
2008-09-10 21:34 ` Johannes Berg
2008-09-11 0:15 ` Christopher Li
2008-05-29 8:54 ` [PATCH 7/9] test conditional result locking Johannes Berg
2008-05-29 8:54 ` [PATCH 8/9] show required context in instruction output Johannes Berg
2008-05-29 8:54 ` [PATCH 9/9] check inlines explicitly Johannes Berg
2008-05-29 23:14 ` [PATCH 9/9 v2] " Johannes Berg
2008-05-29 23:20 ` Harvey Harrison
2008-05-29 22:12 ` [PATCH 0/9] context tracking updates Harvey Harrison
2008-05-29 22:35 ` Harvey Harrison
2008-05-29 22:45 ` Johannes Berg
2008-05-29 22:47 ` Harvey Harrison
2008-05-29 22:51 ` Harvey Harrison
2008-05-29 22:54 ` Johannes Berg
2008-05-29 23:03 ` Pavel Roskin
2008-05-29 23:06 ` Johannes Berg
2008-05-29 23:15 ` Johannes Berg
2008-05-29 23:04 ` Johannes Berg
2008-07-20 12:30 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).