* [PATCH 01/16] Add enum member list to the parent
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
@ 2008-12-18 21:51 ` Alexey Zaytsev
2008-12-18 21:51 ` [PATCH 02/16] Expand "dubious !x & y" handling to other combinations of !, &, and | Alexey Zaytsev
` (14 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 21:51 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Christopher Li <sparse@chrisli.org>
Signed-Off-By: Christopher Li <sparse@chrisli.org>
Acked-by: Thomas Schmid <Thomas.Schmid@br-automation.com>
---
parse.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/parse.c b/parse.c
index 915ae20..eb31871 100644
--- a/parse.c
+++ b/parse.c
@@ -665,7 +665,6 @@ static struct token *parse_enum_declaration(struct token *token, struct symbol *
unsigned long long lastval = 0;
struct symbol *ctype = NULL, *base_type = NULL;
Num upper = {-1, 0}, lower = {1, 0};
- struct symbol_list *entries = NULL;
parent->examined = 1;
parent->ctype.base_type = &int_ctype;
@@ -701,7 +700,7 @@ static struct token *parse_enum_declaration(struct token *token, struct symbol *
sym->initializer = expr;
sym->enum_member = 1;
sym->ctype.base_type = parent;
- add_ptr_list(&entries, sym);
+ add_ptr_list(&parent->symbol_list, sym);
if (base_type != &bad_ctype) {
if (ctype->type == SYM_NODE)
@@ -777,8 +776,7 @@ static struct token *parse_enum_declaration(struct token *token, struct symbol *
parent->ctype.modifiers |= (base_type->ctype.modifiers & MOD_UNSIGNED);
parent->examined = 0;
- cast_enum_list(entries, base_type);
- free_ptr_list(&entries);
+ cast_enum_list(parent->symbol_list, base_type);
return token;
}
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 02/16] Expand "dubious !x & y" handling to other combinations of !, &, and |.
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
2008-12-18 21:51 ` [PATCH 01/16] Add enum member list to the parent Alexey Zaytsev
@ 2008-12-18 21:51 ` Alexey Zaytsev
2008-12-18 21:52 ` [PATCH 03/16] Set gcc include path at runtime Alexey Zaytsev
` (13 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 21:51 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Josh Triplett <josh@freedesktop.org>
Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
evaluate.c | 13 ++++++++++---
validation/dubious-bitwise-with-not.c | 19 +++++++++++++++++--
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index c501323..f976645 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -921,9 +921,16 @@ static struct symbol *evaluate_binop(struct expression *expr)
rtype = integer_promotion(rtype);
} else {
// The rest do usual conversions
- if (op == '&' && expr->left->type == EXPR_PREOP &&
- expr->left->op == '!')
- warning(expr->pos, "dubious: !x & y");
+ const unsigned left_not = expr->left->type == EXPR_PREOP
+ && expr->left->op == '!';
+ const unsigned right_not = expr->right->type == EXPR_PREOP
+ && expr->right->op == '!';
+ if ((op == '&' || op == '|') && (left_not || right_not))
+ warning(expr->pos, "dubious: %sx %c %sy",
+ left_not ? "!" : "",
+ op,
+ right_not ? "!" : "");
+
ltype = usual_conversions(op, expr->left, expr->right,
lclass, rclass, ltype, rtype);
ctype = rtype = ltype;
diff --git a/validation/dubious-bitwise-with-not.c b/validation/dubious-bitwise-with-not.c
index e076899..c48bcae 100644
--- a/validation/dubious-bitwise-with-not.c
+++ b/validation/dubious-bitwise-with-not.c
@@ -1,9 +1,24 @@
-static unsigned int ok1 = !1 && 2;
-static unsigned int bad1 = !1 & 2;
+static unsigned int ok1 = !1 && 2;
+static unsigned int bad1 = !1 & 2;
+static unsigned int ok2 = !1 || 2;
+static unsigned int bad2 = !1 | 2;
+static unsigned int ok3 = 1 && !2;
+static unsigned int bad3 = 1 & !2;
+static unsigned int ok4 = 1 || !2;
+static unsigned int bad4 = 1 | !2;
+static unsigned int ok5 = !1 && !2;
+static unsigned int bad5 = !1 & !2;
+static unsigned int ok6 = !1 || !2;
+static unsigned int bad6 = !1 | !2;
/*
* check-name: Dubious bitwise operation on !x
*
* check-error-start
dubious-bitwise-with-not.c:2:31: warning: dubious: !x & y
+dubious-bitwise-with-not.c:4:31: warning: dubious: !x | y
+dubious-bitwise-with-not.c:6:31: warning: dubious: x & !y
+dubious-bitwise-with-not.c:8:31: warning: dubious: x | !y
+dubious-bitwise-with-not.c:10:31: warning: dubious: !x & !y
+dubious-bitwise-with-not.c:12:31: warning: dubious: !x | !y
* check-error-end
*/
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 03/16] Set gcc include path at runtime.
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
2008-12-18 21:51 ` [PATCH 01/16] Add enum member list to the parent Alexey Zaytsev
2008-12-18 21:51 ` [PATCH 02/16] Expand "dubious !x & y" handling to other combinations of !, &, and | Alexey Zaytsev
@ 2008-12-18 21:52 ` Alexey Zaytsev
2008-12-18 21:52 ` [PATCH 04/16] Let cgcc pass -gcc-base-dir to sparse Alexey Zaytsev
` (12 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 21:52 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
You can now tell sparse where to look for the compiler
headers with -gcc-base-dir <dir>. Otherwise sparse will
look for headers used to build it.
Also adds $GCC_BASE/include-fixed used by newer gcc
versions.
Signed-off-by: Alexey zaytsev <alexey.zaytsev@gmail.com>
---
Makefile | 7 ++--
lib.c | 23 ++++++++++++--
pre-process.c | 96 +++++++++++++++++++++++++++++++++++++++++++++------------
3 files changed, 99 insertions(+), 27 deletions(-)
diff --git a/Makefile b/Makefile
index ca33218..18cec68 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,8 @@ AR = ar
HAVE_LIBXML=$(shell pkg-config --exists libxml-2.0 && echo 'yes')
+CFLAGS += -DGCC_BASE=\"$(shell $(CC) --print-file-name=)\"
+
DESTDIR=
PREFIX=$(HOME)
BINDIR=$(PREFIX)/bin
@@ -147,7 +149,7 @@ lib.o: $(LIB_H)
allocate.o: $(LIB_H)
ptrlist.o: $(LIB_H)
parse.o: $(LIB_H)
-pre-process.o: $(LIB_H) pre-process.h
+pre-process.o: $(LIB_H)
scope.o: $(LIB_H)
show-parse.o: $(LIB_H)
symbol.o: $(LIB_H)
@@ -186,9 +188,6 @@ compat-solaris.o: compat/mmap-blob.c $(LIB_H)
compat-mingw.o: $(LIB_H)
compat-cygwin.o: $(LIB_H)
-pre-process.h:
- $(QUIET_GEN)echo "#define GCC_INTERNAL_INCLUDE \"`$(CC) -print-file-name=include`\"" > pre-process.h
-
.c.o:
$(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) $<
diff --git a/lib.c b/lib.c
index 8dadfa9..45a4f39 100644
--- a/lib.c
+++ b/lib.c
@@ -41,6 +41,8 @@ int gcc_major = __GNUC__;
int gcc_minor = __GNUC_MINOR__;
int gcc_patchlevel = __GNUC_PATCHLEVEL__;
+static const char *gcc_base_dir = GCC_BASE;
+
struct token *skip_to(struct token *token, int op)
{
while (!match_op(token, op) && !eof_token(token))
@@ -591,6 +593,14 @@ static char **handle_dirafter(char *arg, char **next)
return next;
}
+static char **handle_base_dir(char *arg, char **next)
+{
+ gcc_base_dir = *++next;
+ if (!gcc_base_dir)
+ die("missing argument for -gcc-base-dir option");
+ return next;
+}
+
struct switches {
const char *name;
char **(*fn)(char *, char **);
@@ -601,6 +611,7 @@ static char **handle_switch(char *arg, char **next)
static struct switches cmd[] = {
{ "nostdinc", handle_nostdinc },
{ "dirafter", handle_dirafter },
+ { "gcc-base-dir", handle_base_dir},
{ NULL, NULL }
};
struct switches *s;
@@ -644,8 +655,8 @@ void declare_builtin_functions(void)
/* Gaah. gcc knows tons of builtin <string.h> functions */
add_pre_buffer("extern void *__builtin_memcpy(void *, const void *, __SIZE_TYPE__);\n");
add_pre_buffer("extern void *__builtin_mempcpy(void *, const void *, __SIZE_TYPE__);\n");
- add_pre_buffer("extern void *__builtin_memset(void *, int, __SIZE_TYPE__);\n");
- add_pre_buffer("extern int __builtin_memcmp(const void *, const void *, __SIZE_TYPE__);\n");
+ add_pre_buffer("extern void *__builtin_memset(void *, int, __SIZE_TYPE__);\n");
+ add_pre_buffer("extern int __builtin_memcmp(const void *, const void *, __SIZE_TYPE__);\n");
add_pre_buffer("extern char *__builtin_strcat(char *, const char *);\n");
add_pre_buffer("extern char *__builtin_strncat(char *, const char *, __SIZE_TYPE__);\n");
add_pre_buffer("extern int __builtin_strcmp(const char *, const char *);\n");
@@ -698,6 +709,12 @@ void create_builtin_stream(void)
add_pre_buffer("#weak_define __GNUC__ %d\n", gcc_major);
add_pre_buffer("#weak_define __GNUC_MINOR__ %d\n", gcc_minor);
add_pre_buffer("#weak_define __GNUC_PATCHLEVEL__ %d\n", gcc_patchlevel);
+
+ /* We add compiler headers path here because we have to parse
+ * the arguments to get it, falling back to default. */
+ add_pre_buffer("#add_system \"%s/include\"\n", gcc_base_dir);
+ add_pre_buffer("#add_system \"%s/include-fixed\"\n", gcc_base_dir);
+
add_pre_buffer("#define __extension__\n");
add_pre_buffer("#define __pragma__\n");
@@ -780,7 +797,7 @@ static struct symbol_list *sparse_tokenstream(struct token *token)
putchar('\n');
return NULL;
- }
+ }
// Parse the resulting C code
while (!eof_token(token))
diff --git a/pre-process.c b/pre-process.c
index ca1d8ef..9c6ef5b 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -20,7 +20,6 @@
#include <limits.h>
#include <time.h>
-#include "pre-process.h"
#include "lib.h"
#include "allocate.h"
#include "parse.h"
@@ -36,13 +35,14 @@ const char *includepath[INCLUDEPATHS+1] = {
"",
"/usr/include",
"/usr/local/include",
- GCC_INTERNAL_INCLUDE,
NULL
};
static const char **quote_includepath = includepath;
static const char **angle_includepath = includepath + 1;
+static const char **isys_includepath = includepath + 1;
static const char **sys_includepath = includepath + 1;
+static const char **dirafter_includepath = includepath + 3;
#define dirty_stream(stream) \
do { \
@@ -653,8 +653,14 @@ static int already_tokenized(const char *path)
*
* Three set of include paths are known:
* quote_includepath: Path to search when using #include "file.h"
- * angle_includepath: Path to search when using #include <file.h>
- * sys_includepath: Built-in include paths
+ * angle_includepath: Paths to search when using #include <file.h>
+ * isys_includepath: Paths specified with -isystem, come before the
+ * built-in system include paths. Gcc would suppress
+ * warnings from system headers. Here we separate
+ * them from the angle_ ones to keep search ordering.
+ *
+ * sys_includepath: Built-in include paths.
+ * dirafter_includepath Paths added with -dirafter.
*
* The above is implemented as one array with pointers
* +--------------+
@@ -664,21 +670,23 @@ static int already_tokenized(const char *path)
* +--------------+
* angle_includepath ---> | |
* +--------------+
+ * isys_includepath ---> | |
+ * +--------------+
* sys_includepath ---> | |
* +--------------+
- * | |
+ * dirafter_includepath -> | |
* +--------------+
*
- * -I dir insert dir just before sys_includepath and move the rest
+ * -I dir insert dir just before isys_includepath and move the rest
* -I- makes all dirs specified with -I before to quote dirs only and
- * angle_includepath is set equal to sys_includepath.
- * -nostdinc removes all sys dirs be storing NULL in entry pointed
- * to by * sys_includepath. Note this will reset all dirs built-in and added
- * before -nostdinc by -isystem and -dirafter
- * -isystem dir adds dir where sys_includepath points adding this dir as
+ * angle_includepath is set equal to isys_includepath.
+ * -nostdinc removes all sys dirs by storing NULL in entry pointed
+ * to by * sys_includepath. Note that this will reset all dirs built-in
+ * and added before -nostdinc by -isystem and -dirafter.
+ * -isystem dir adds dir where isys_includepath points adding this dir as
* first systemdir
* -dirafter dir adds dir to the end of the list
- **/
+ */
static void set_stream_include_path(struct stream *stream)
{
@@ -1449,7 +1457,41 @@ static int handle_nostdinc(struct stream *stream, struct token **line, struct to
return 1;
}
-static void add_path_entry(struct token *token, const char *path, const char ***where, const char **new_path)
+static inline void update_inc_ptrs(const char ***where)
+{
+
+ if (*where <= dirafter_includepath) {
+ dirafter_includepath++;
+ /* If this was the entry that we prepend, don't
+ * rise the lower entries, even if they are at
+ * the same level. */
+ if (where == &dirafter_includepath)
+ return;
+ }
+ if (*where <= sys_includepath) {
+ sys_includepath++;
+ if (where == &sys_includepath)
+ return;
+ }
+ if (*where <= isys_includepath) {
+ isys_includepath++;
+ if (where == &isys_includepath)
+ return;
+ }
+
+ /* angle_includepath is actually never updated, since we
+ * don't suppport -iquote rught now. May change some day. */
+ if (*where <= angle_includepath) {
+ angle_includepath++;
+ if (where == &angle_includepath)
+ return;
+ }
+}
+
+/* Add a path before 'where' and update the pointers associated with the
+ * includepath array */
+static void add_path_entry(struct token *token, const char *path,
+ const char ***where)
{
const char **dst;
const char *next;
@@ -1467,20 +1509,19 @@ static void add_path_entry(struct token *token, const char *path, const char ***
}
next = path;
dst = *where;
- *where = new_path;
+
+ update_inc_ptrs(where);
/*
* Move them all up starting at dst,
* insert the new entry..
*/
- for (;;) {
+ do {
const char *tmp = *dst;
*dst = next;
- if (!next)
- break;
next = tmp;
dst++;
- }
+ } while (next);
}
static int handle_add_include(struct stream *stream, struct token **line, struct token *token)
@@ -1493,7 +1534,7 @@ static int handle_add_include(struct stream *stream, struct token **line, struct
warning(token->pos, "expected path string");
return 1;
}
- add_path_entry(token, token->string->data, &sys_includepath, sys_includepath + 1);
+ add_path_entry(token, token->string->data, &isys_includepath);
}
}
@@ -1507,7 +1548,21 @@ static int handle_add_isystem(struct stream *stream, struct token **line, struct
sparse_error(token->pos, "expected path string");
return 1;
}
- add_path_entry(token, token->string->data, &sys_includepath, sys_includepath);
+ add_path_entry(token, token->string->data, &sys_includepath);
+ }
+}
+
+static int handle_add_system(struct stream *stream, struct token **line, struct token *token)
+{
+ for (;;) {
+ token = token->next;
+ if (eof_token(token))
+ return 1;
+ if (token_type(token) != TOKEN_STRING) {
+ sparse_error(token->pos, "expected path string");
+ return 1;
+ }
+ add_path_entry(token, token->string->data, &dirafter_includepath);
}
}
@@ -1625,6 +1680,7 @@ static void init_preprocessor(void)
{ "nostdinc", handle_nostdinc },
{ "add_include", handle_add_include },
{ "add_isystem", handle_add_isystem },
+ { "add_system", handle_add_system },
{ "add_dirafter", handle_add_dirafter },
{ "split_include", handle_split_include },
}, special[] = {
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 04/16] Let cgcc pass -gcc-base-dir to sparse.
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (2 preceding siblings ...)
2008-12-18 21:52 ` [PATCH 03/16] Set gcc include path at runtime Alexey Zaytsev
@ 2008-12-18 21:52 ` Alexey Zaytsev
2008-12-18 21:52 ` [PATCH 05/16] Document -gcc-base-dir in sparse.1 Alexey Zaytsev
` (11 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 21:52 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
cgcc | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/cgcc b/cgcc
index cda8dab..fdda6d1 100755
--- a/cgcc
+++ b/cgcc
@@ -10,9 +10,11 @@ my $has_specs = 0;
my $gendeps = 0;
my $do_check = 0;
my $do_compile = 1;
+my $gcc_base_dir;
my $verbose = 0;
-foreach (@ARGV) {
+while (@ARGV) {
+ $_ = shift(@ARGV);
# Look for a .c file. We don't want to run the checker on .o or .so files
# in the link run. (This simplistic check knows nothing about options
# with arguments, but it seems to do the job.)
@@ -36,6 +38,12 @@ foreach (@ARGV) {
next;
}
+ if (/^-gcc-base-dir$/) {
+ $gcc_base_dir = shift @ARGV;
+ die ("$0: missing argument for -gcc-base-dir option") if !$gcc_base_dir;
+ next;
+ }
+
# If someone adds "-E", don't pre-process twice.
$do_compile = 0 if $_ eq '-E';
@@ -56,6 +64,10 @@ if ($do_check) {
$check .= &add_specs ('host_arch_specs');
$check .= &add_specs ('host_os_specs');
}
+
+ $gcc_base_dir = qx($cc -print-file-name=) if !$gcc_base_dir;
+ $check .= " -gcc-base-dir " . $gcc_base_dir if $gcc_base_dir;
+
print "$check\n" if $verbose;
if ($do_compile) {
system ($check);
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 05/16] Document -gcc-base-dir in sparse.1
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (3 preceding siblings ...)
2008-12-18 21:52 ` [PATCH 04/16] Let cgcc pass -gcc-base-dir to sparse Alexey Zaytsev
@ 2008-12-18 21:52 ` Alexey Zaytsev
2008-12-18 21:52 ` [PATCH 06/16] Rename dirafter to idirafter Alexey Zaytsev
` (10 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 21:52 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
Signed-off-by: Alexey zaytsev <alexey.zaytsev@gmail.com>
---
sparse.1 | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/sparse.1 b/sparse.1
index d242dc7..92a1cae 100644
--- a/sparse.1
+++ b/sparse.1
@@ -287,6 +287,11 @@ However, this behavior can lead to subtle errors.
Sparse does not issue these warnings by default.
.
+.SH MISC OPTIONS
+.TP
+.B \-gcc-base-dir \fIdir\fR
+Look for compiler-provided system headers in \fIdir\fR/include/ and \fIdir\fR/include-fixed/.
+.
.SH SEE ALSO
.BR cgcc (1)
.
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 06/16] Rename dirafter to idirafter.
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (4 preceding siblings ...)
2008-12-18 21:52 ` [PATCH 05/16] Document -gcc-base-dir in sparse.1 Alexey Zaytsev
@ 2008-12-18 21:52 ` Alexey Zaytsev
2008-12-18 22:32 ` [PATCH 7/16] Let void have sizeof 1 Alexey Zaytsev
` (9 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 21:52 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
Dirafter was probably just a mistake.
Gcc uses -idirafter.
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Alexey zaytsev <alexey.zaytsev@gmail.com>
---
lib.c | 15 +++++----------
pre-process.c | 4 ++--
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/lib.c b/lib.c
index 45a4f39..dc2da14 100644
--- a/lib.c
+++ b/lib.c
@@ -321,6 +321,11 @@ static char **handle_switch_i(char *arg, char **next)
if (!path)
die("missing argument for -isystem option");
add_pre_buffer("#add_isystem \"%s/\"\n", path);
+ } else if (*next && !strcmp(arg, "idirafter")) {
+ char *path = *++next;
+ if (!path)
+ die("missing argument for -idirafter option");
+ add_pre_buffer("#add_dirafter \"%s/\"\n", path);
}
return next;
}
@@ -584,15 +589,6 @@ static char **handle_nostdinc(char *arg, char **next)
return next;
}
-static char **handle_dirafter(char *arg, char **next)
-{
- char *path = *++next;
- if (!path)
- die("missing argument for -dirafter option");
- add_pre_buffer("#add_dirafter \"%s/\"\n", path);
- return next;
-}
-
static char **handle_base_dir(char *arg, char **next)
{
gcc_base_dir = *++next;
@@ -610,7 +606,6 @@ static char **handle_switch(char *arg, char **next)
{
static struct switches cmd[] = {
{ "nostdinc", handle_nostdinc },
- { "dirafter", handle_dirafter },
{ "gcc-base-dir", handle_base_dir},
{ NULL, NULL }
};
diff --git a/pre-process.c b/pre-process.c
index 9c6ef5b..cf53893 100644
--- a/pre-process.c
+++ b/pre-process.c
@@ -682,10 +682,10 @@ static int already_tokenized(const char *path)
* angle_includepath is set equal to isys_includepath.
* -nostdinc removes all sys dirs by storing NULL in entry pointed
* to by * sys_includepath. Note that this will reset all dirs built-in
- * and added before -nostdinc by -isystem and -dirafter.
+ * and added before -nostdinc by -isystem and -idirafter.
* -isystem dir adds dir where isys_includepath points adding this dir as
* first systemdir
- * -dirafter dir adds dir to the end of the list
+ * -idirafter dir adds dir to the end of the list
*/
static void set_stream_include_path(struct stream *stream)
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 7/16] Let void have sizeof 1
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (5 preceding siblings ...)
2008-12-18 21:52 ` [PATCH 06/16] Rename dirafter to idirafter Alexey Zaytsev
@ 2008-12-18 22:32 ` Alexey Zaytsev
2008-12-23 3:51 ` Christopher Li
2008-12-18 22:33 ` [PATCH 08/16] Add test for acquire/release Alexey Zaytsev
` (8 subsequent siblings)
15 siblings, 1 reply; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:32 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
Gcc assumes sizeof(void) being 1.
Currently sparse would generate wrong code for:
void *test(void *p) {
p++;
return p;
}
unsigned long test1(void *p)
{
return sizeof(*p);
}
.L0x2b48867c1010:
<entry-point>
add.32 %r2 <- %arg1, $-1
ret.32 %r2
test1:
.L0x2b48867c10b0:
<entry-point>
ret.32 $-1
And with bit_size set to &bits_in_char, the code looks
as expected.
Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
---
symbol.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/symbol.c b/symbol.c
index 4da253b..02844cf 100644
--- a/symbol.c
+++ b/symbol.c
@@ -834,7 +834,7 @@ static const struct ctype_declare {
struct symbol *base_type;
} ctype_declaration[] = {
{ &bool_ctype, SYM_BASETYPE, MOD_UNSIGNED, &bits_in_bool, &max_int_alignment, &int_type },
- { &void_ctype, SYM_BASETYPE, 0, NULL, NULL, NULL },
+ { &void_ctype, SYM_BASETYPE, 0, &bits_in_char, NULL, NULL },
{ &type_ctype, SYM_BASETYPE, MOD_TYPE, NULL, NULL, NULL },
{ &incomplete_ctype,SYM_BASETYPE, 0, NULL, NULL, NULL },
{ &bad_ctype, SYM_BASETYPE, 0, NULL, NULL, NULL },
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-18 22:32 ` [PATCH 7/16] Let void have sizeof 1 Alexey Zaytsev
@ 2008-12-23 3:51 ` Christopher Li
2008-12-23 4:37 ` Alexey Zaytsev
0 siblings, 1 reply; 31+ messages in thread
From: Christopher Li @ 2008-12-23 3:51 UTC (permalink / raw)
To: Alexey Zaytsev; +Cc: Josh Triplett, Johannes Berg, linux-sparse
Void type is an incomplete type. It should not have storage size.
Program should not depend on sizeof(void) is 1.
Chris
On Thu, Dec 18, 2008 at 2:32 PM, Alexey Zaytsev
<alexey.zaytsev@gmail.com> wrote:
> Gcc assumes sizeof(void) being 1.
> Currently sparse would generate wrong code for:
>
> void *test(void *p) {
> p++;
> return p;
> }
>
> unsigned long test1(void *p)
> {
> return sizeof(*p);
> }
>
> .L0x2b48867c1010:
> <entry-point>
> add.32 %r2 <- %arg1, $-1
> ret.32 %r2
>
> test1:
> .L0x2b48867c10b0:
> <entry-point>
> ret.32 $-1
>
> And with bit_size set to &bits_in_char, the code looks
> as expected.
>
> Signed-off-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> ---
> symbol.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/symbol.c b/symbol.c
> index 4da253b..02844cf 100644
> --- a/symbol.c
> +++ b/symbol.c
> @@ -834,7 +834,7 @@ static const struct ctype_declare {
> struct symbol *base_type;
> } ctype_declaration[] = {
> { &bool_ctype, SYM_BASETYPE, MOD_UNSIGNED, &bits_in_bool, &max_int_alignment, &int_type },
> - { &void_ctype, SYM_BASETYPE, 0, NULL, NULL, NULL },
> + { &void_ctype, SYM_BASETYPE, 0, &bits_in_char, NULL, NULL },
> { &type_ctype, SYM_BASETYPE, MOD_TYPE, NULL, NULL, NULL },
> { &incomplete_ctype,SYM_BASETYPE, 0, NULL, NULL, NULL },
> { &bad_ctype, SYM_BASETYPE, 0, NULL, NULL, NULL },
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-23 3:51 ` Christopher Li
@ 2008-12-23 4:37 ` Alexey Zaytsev
2008-12-23 5:29 ` Alexey Zaytsev
2008-12-23 5:51 ` Christopher Li
0 siblings, 2 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-23 4:37 UTC (permalink / raw)
To: Christopher Li; +Cc: Josh Triplett, Johannes Berg, linux-sparse
On Tue, Dec 23, 2008 at 06:51, Christopher Li <sparse@chrisli.org> wrote:
> Void type is an incomplete type. It should not have storage size.
But how about void *? If void *p = x, what p+1 should be? Gcc defines
sizeof(void) being 1, and people seem to know and use this.
http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Warning-Options.html#index-Wno_002dpointer_002darith-347
In the Linux kernel, we've got lots of void * address arithmetics:
linux/linux-2.6$ make -j 8 CC='gcc -Wpointer-arith' 2>&1 | grep
'warning: pointer of type \'void \*\' used in arithmetic' | uniq | wc
-l
45095
So even if we can't agree if such use is legid or not, we definetely
have to support this in sparse. And there is no need to warn about
it, as gcc already can do this.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-23 4:37 ` Alexey Zaytsev
@ 2008-12-23 5:29 ` Alexey Zaytsev
2008-12-23 9:00 ` Derek M Jones
2008-12-23 5:51 ` Christopher Li
1 sibling, 1 reply; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-23 5:29 UTC (permalink / raw)
To: Christopher Li; +Cc: Josh Triplett, Johannes Berg, linux-sparse
On Tue, Dec 23, 2008 at 07:37, Alexey Zaytsev <alexey.zaytsev@gmail.com> wrote:
> On Tue, Dec 23, 2008 at 06:51, Christopher Li <sparse@chrisli.org> wrote:
> In the Linux kernel, we've got lots of void * address arithmetics:
> linux/linux-2.6$ make -j 8 CC='gcc -Wpointer-arith' 2>&1 | grep
> 'warning: pointer of type \'void \*\' used in arithmetic' | uniq | wc
> -l
> 45095
I screwed up, it should be | sort -u | uniq | wc -l
But it's still 14488.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-23 5:29 ` Alexey Zaytsev
@ 2008-12-23 9:00 ` Derek M Jones
2008-12-23 15:05 ` Alexey Zaytsev
0 siblings, 1 reply; 31+ messages in thread
From: Derek M Jones @ 2008-12-23 9:00 UTC (permalink / raw)
To: Alexey Zaytsev; +Cc: Christopher Li, Josh Triplett, Johannes Berg, linux-sparse
Alexey,
>> In the Linux kernel, we've got lots of void * address arithmetics:
>> linux/linux-2.6$ make -j 8 CC='gcc -Wpointer-arith' 2>&1 | grep
>> 'warning: pointer of type \'void \*\' used in arithmetic' | uniq | wc
>> -l
>> 45095
> I screwed up, it should be | sort -u | uniq | wc -l
> But it's still 14488.
This is a sufficiently large number that we ought to consider adding
checks for the usage, for instance:
1) Are the arguments really chars of one sort or another and
therefore the parameter ought to be declared as such?
2) The arguments have various structure and/or array types and
the storage is being walked over in the called function. Can the
minimum size ever passed be worked out and this information used in
the size_void (my proposed name for a pointer that has either sizeof,
++ or -- performed on it) function to check for overruns.
3) Are there any restrictions on passing a void pointer to a
size_void function or vice versa? I cannot think of any obvious
ones, but then I don't know the various kinds of circumstances
in which this happens.
4) Other possible fault issues, people?
--
Derek M. Jones tel: +44 (0) 1252 520 667
Knowledge Software Ltd mailto:derek@knosof.co.uk
Source code analysis http://www.knosof.co.uk
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-23 9:00 ` Derek M Jones
@ 2008-12-23 15:05 ` Alexey Zaytsev
2008-12-24 0:26 ` Derek M Jones
0 siblings, 1 reply; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-23 15:05 UTC (permalink / raw)
To: Derek M Jones; +Cc: Christopher Li, Josh Triplett, Johannes Berg, linux-sparse
On Tue, Dec 23, 2008 at 12:00, Derek M Jones <derek@knosof.co.uk> wrote:
> Alexey,
>
>>> In the Linux kernel, we've got lots of void * address arithmetics:
>>> linux/linux-2.6$ make -j 8 CC='gcc -Wpointer-arith' 2>&1 | grep
>>> 'warning: pointer of type \'void \*\' used in arithmetic' | uniq | wc
>>> -l
>>> 45095
>>
>> I screwed up, it should be | sort -u | uniq | wc -l
>> But it's still 14488.
>
> This is a sufficiently large number that we ought to consider adding
> checks for the usage, for instance:
>
> 1) Are the arguments really chars of one sort or another and
> therefore the parameter ought to be declared as such?
You mean, if address arithmetics is performed on a void * cast result,
check that the casted type too has sizeof 1?
> 2) The arguments have various structure and/or array types and
> the storage is being walked over in the called function. Can the
> minimum size ever passed be worked out and this information used in
> the size_void (my proposed name for a pointer that has either sizeof,
> ++ or -- performed on it) function to check for overruns.
This won't be possible without dataflow analysis, and sparse currently
can't do any. This may change in the future, but if so, we will be able to
check for many types of overruns, not only void *.
>
> 3) Are there any restrictions on passing a void pointer to a
> size_void function or vice versa? I cannot think of any obvious
> ones, but then I don't know the various kinds of circumstances
> in which this happens.
>
> 4) Other possible fault issues, people?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-23 15:05 ` Alexey Zaytsev
@ 2008-12-24 0:26 ` Derek M Jones
2008-12-24 2:39 ` Alexey Zaytsev
0 siblings, 1 reply; 31+ messages in thread
From: Derek M Jones @ 2008-12-24 0:26 UTC (permalink / raw)
To: Alexey Zaytsev; +Cc: Christopher Li, Josh Triplett, Johannes Berg, linux-sparse
Alexey,
I have been looking through the source to look at the contexts
in which arithmetic is performed on void pointers.
>> 1) Are the arguments really chars of one sort or another and
>> therefore the parameter ought to be declared as such?
> You mean, if address arithmetics is performed on a void * cast result,
> check that the casted type too has sizeof 1?
I was thinking more along the lines of pointer to a character type being
converted to void * for no obvious reason, or a value being converted to
void * having an arithmetic operation performed and then converted to
a pointer to character type.
For an example see line 156 of arch/x86/kernel/module_64.c
I would expect the void * to come from/go to a type that had a
size greater than 1.
>> 4) Other possible fault issues, people?
I have found an instance (arch/x86/kernel/kprobes.c:834) that
effectively does:
(void *)long_val + an_int_calculation
when it should have done:
(void *)(long_val + an_int_calculation)
hardly an earth shattering misuse.
--
Derek M. Jones tel: +44 (0) 1252 520 667
Knowledge Software Ltd mailto:derek@knosof.co.uk
Source code analysis http://www.knosof.co.uk
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-24 0:26 ` Derek M Jones
@ 2008-12-24 2:39 ` Alexey Zaytsev
2008-12-24 21:59 ` David Given
0 siblings, 1 reply; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-24 2:39 UTC (permalink / raw)
To: Derek M Jones; +Cc: Christopher Li, Josh Triplett, Johannes Berg, linux-sparse
On Wed, Dec 24, 2008 at 03:26, Derek M Jones <derek@knosof.co.uk> wrote:
> Alexey,
>
> I have been looking through the source to look at the contexts
> in which arithmetic is performed on void pointers.
>
>>> 1) Are the arguments really chars of one sort or another and
>>> therefore the parameter ought to be declared as such?
>>
>> You mean, if address arithmetics is performed on a void * cast result,
>> check that the casted type too has sizeof 1?
>
> I was thinking more along the lines of pointer to a character type being
> converted to void * for no obvious reason, or a value being converted to
> void * having an arithmetic operation performed and then converted to
> a pointer to character type.
> For an example see line 156 of arch/x86/kernel/module_64.
>
> I would expect the void * to come from/go to a type that had a
> size greater than 1.
>
>>> 4) Other possible fault issues, people?
>
> I have found an instance (arch/x86/kernel/kprobes.c:834) that
> effectively does:
>
> (void *)long_val + an_int_calculation
>
> when it should have done:
>
> (void *)(long_val + an_int_calculation)
>
> hardly an earth shattering misuse.
There is nothing wrong here. Just depends on the way you think
about it. In the first case you convert a long to a pointer, and add
an int, the the second, you convert the sum. The result is of course
the same. And this has nothing to do with void*, it would be the
same with char*.
So far the only concern with void* is that not everyone knows
the it is sizeof 1. So if you propose repulsing void* with char*
when we don't really need a void*, that's probably fine. But don't
you think there are more useful ways to spend your time?
With open issues like the Global Warming, World Hunger and
standing 2.6.28 regressions, I don't see anyone bothered with
replacing unnecessary void* casts.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-24 2:39 ` Alexey Zaytsev
@ 2008-12-24 21:59 ` David Given
2008-12-24 23:10 ` Christopher Li
0 siblings, 1 reply; 31+ messages in thread
From: David Given @ 2008-12-24 21:59 UTC (permalink / raw)
To: linux-sparse
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Alexey Zaytsev wrote:
[...]
> So far the only concern with void* is that not everyone knows
> the it is sizeof 1. So if you propose repulsing void* with char*
> when we don't really need a void*, that's probably fine. But don't
> you think there are more useful ways to spend your time?
> With open issues like the Global Warming, World Hunger and
> standing 2.6.28 regressions, I don't see anyone bothered with
> repacing unnecessary void* casts.
The thing is, sizeof(void) is *not* 1. sizeof(void) is *illegal*. It
just happens that gcc, as a platform-specific extension, treats
sizeof(void) as 1 by default.
As a linter, sparse really ought not to be encouraging non-portable
behaviour. Admittedly, there's so much stuff in the kernel source that's
gcc-specific that it's probably not going to be possible to make it
build on anything else, but it should still warn people about it unless
specifically told otherwise --- it's bad practice, and may be indicative
of further problems elsewhere, and as such is worth a diagnostic.
- --
David Given
dg@cowlark.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFJUrCmf9E0noFvlzgRAkRCAJwPffkuAuAMxfJJSkh+H7AlIPHQkQCgtGRQ
0t71dcB7ZaIKud6Qi5XXrx8=
=83ss
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-24 21:59 ` David Given
@ 2008-12-24 23:10 ` Christopher Li
2008-12-25 0:14 ` Derek M Jones
` (2 more replies)
0 siblings, 3 replies; 31+ messages in thread
From: Christopher Li @ 2008-12-24 23:10 UTC (permalink / raw)
To: David Given; +Cc: linux-sparse
On Wed, Dec 24, 2008 at 1:59 PM, David Given <dg@cowlark.com> wrote:
> The thing is, sizeof(void) is *not* 1. sizeof(void) is *illegal*. It
> just happens that gcc, as a platform-specific extension, treats
> sizeof(void) as 1 by default.
Exactly. I am unhappy about that patch as well.
Sparse internally use symbol->bit_size == 0 to determine uncompleted type.
Thanks this change. Now is_byte_type() will return true for void type as well.
> As a linter, sparse really ought not to be encouraging non-portable
> behaviour. Admittedly, there's so much stuff in the kernel source that's
> gcc-specific that it's probably not going to be possible to make it
> build on anything else, but it should still warn people about it unless
> specifically told otherwise --- it's bad practice, and may be indicative
> of further problems elsewhere, and as such is worth a diagnostic.
I don't see the kernel directly use sizeof(void). Most of the place is
using (void*) pointer + offset. It is not portable. But it is probably not
worthy while to fix. Convert the void* to char*, add offset, convert it back
to void* is pretty annoying as well. If we really want to make it clean, maybe
we can use a macro or inline functions. Again, probably not worth the effort.
But legitimize sizeof(void) == 1 is a different story. That I feel is just
plain wrong.
Even gcc is self contradicting regarding the size of void.
Compiling "void i;" will give you:
/tmp/void.c:1: error: storage size of 'i' isn't known
I saw Josh just merge the sizeof(void) patch. Oh well.
Chris
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-24 23:10 ` Christopher Li
@ 2008-12-25 0:14 ` Derek M Jones
[not found] ` <4952C758.8070605@numba-tu.com>
2008-12-25 17:12 ` Alexey Zaytsev
2 siblings, 0 replies; 31+ messages in thread
From: Derek M Jones @ 2008-12-25 0:14 UTC (permalink / raw)
To: Christopher Li; +Cc: David Given, linux-sparse
Christopher,
> I don't see the kernel directly use sizeof(void). Most of the place is
In some places it uses sizeof(*void_ptr) and I don't think we want to
prohibit a usage that is consistent with how void_ptr can be
manipulated.
> using (void*) pointer + offset. It is not portable. But it is probably not
> worthy while to fix. Convert the void* to char*, add offset, convert it back
> to void* is pretty annoying as well. If we really want to make it clean, maybe
> we can use a macro or inline functions. Again, probably not worth the effort.
I agree. However, we need to recognise that this hole that has been
put into the type system needs to be carefully monitored to make
sure that unintended consequences do not occur. Hence my suggestion
that sparse check for suspicious usage, but the only one I have come
up with so far is a sequence of conversions that is equivalent to:
(char*)((void *)ch_ptr + offset)
--
Derek M. Jones tel: +44 (0) 1252 520 667
Knowledge Software Ltd mailto:derek@knosof.co.uk
Source code analysis http://www.knosof.co.uk
^ permalink raw reply [flat|nested] 31+ messages in thread[parent not found: <4952C758.8070605@numba-tu.com>]
* Re: [PATCH 7/16] Let void have sizeof 1
[not found] ` <4952C758.8070605@numba-tu.com>
@ 2008-12-25 0:15 ` Christopher Li
0 siblings, 0 replies; 31+ messages in thread
From: Christopher Li @ 2008-12-25 0:15 UTC (permalink / raw)
To: Tommy Thorn; +Cc: David Given, linux-sparse
On Wed, Dec 24, 2008 at 3:35 PM, Tommy Thorn <tommy@numba-tu.com> wrote:
>
> You can't have one without the other as you will break identities like
>
> (uintptr_t) (x + k) === (uintptr_t) x + sizeof (typeof (x)) * k
>
> which could appear in a macro.
Right. That is exactly the place I actuall want to know. We should
consider fix that in the source to have proper type. I expect there is
not much place in the kernel use that.
I can write some code to find out how many place in the kernel
actually need to evaluate sizeof(void).
What do you think?
>
> I really think you should let it go. The C standard for Linux is for all
> practical purposes GCC C, not C99 C.
>
> Trying to impose a stricter standard will just make sparse a less useful
> tool.
I understand that kernel is C stander gcc. But the whole point of sparse is
stricter than gcc. Otherwise we will just use gcc instead.
I agree that we should probably let the place use void* + offset go.
It all boils down to do we care about place directly use sizeof(void) or not.
Another aspect is that, inside sparse, it become very hard to distinguish
void vs char, other than compare it is &void_ctype or not. If we decide go
with sizeof(void) == 1. Those place that assume only uncompleted ctype
will have bit_size == 0 need to be fixed as well.
Chris
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-24 23:10 ` Christopher Li
2008-12-25 0:14 ` Derek M Jones
[not found] ` <4952C758.8070605@numba-tu.com>
@ 2008-12-25 17:12 ` Alexey Zaytsev
2 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-25 17:12 UTC (permalink / raw)
To: Christopher Li; +Cc: David Given, linux-sparse
On Thu, Dec 25, 2008 at 02:10, Christopher Li <sparse@chrisli.org> wrote:
> On Wed, Dec 24, 2008 at 1:59 PM, David Given <dg@cowlark.com> wrote:
>> The thing is, sizeof(void) is *not* 1. sizeof(void) is *illegal*. It
>> just happens that gcc, as a platform-specific extension, treats
>> sizeof(void) as 1 by default.
>
> Exactly. I am unhappy about that patch as well.
> Sparse internally use symbol->bit_size == 0 to determine uncompleted type.
> Thanks this change. Now is_byte_type() will return true for void type as well.
I can only agree here. I made my patch that way only because of my poor
understanding of the sparse internals (which I stated in my first email).
>> As a linter, sparse really ought not to be encouraging non-portable
>> behaviour. Admittedly, there's so much stuff in the kernel source that's
>> gcc-specific that it's probably not going to be possible to make it
>> build on anything else, but it should still warn people about it unless
>> specifically told otherwise --- it's bad practice, and may be indicative
>> of further problems elsewhere, and as such is worth a diagnostic.
>
> I don't see the kernel directly use sizeof(void). Most of the place is
> using (void*) pointer + offset. It is not portable. But it is probably not
> worthy while to fix. Convert the void* to char*, add offset, convert it back
> to void* is pretty annoying as well. If we really want to make it clean, maybe
> we can use a macro or inline functions. Again, probably not worth the effort.
>
> But legitimize sizeof(void) == 1 is a different story. That I feel is just
> plain wrong.
Of course explicit sizeof(void) itself is just plain wrong, but also
it's something
that nobody would ever use. So, if you warn here, I of course have no
objections.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/16] Let void have sizeof 1
2008-12-23 4:37 ` Alexey Zaytsev
2008-12-23 5:29 ` Alexey Zaytsev
@ 2008-12-23 5:51 ` Christopher Li
2008-12-23 6:09 ` Alexey Zaytsev
1 sibling, 1 reply; 31+ messages in thread
From: Christopher Li @ 2008-12-23 5:51 UTC (permalink / raw)
To: Alexey Zaytsev; +Cc: Josh Triplett, Johannes Berg, linux-sparse
> But how about void *? If void *p = x, what p+1 should be? Gcc defines
> sizeof(void) being 1, and people seem to know and use this.
> http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Warning-Options.html#index-Wno_002dpointer_002darith-347
Using pointer arithmetic on void* is not as crazy as using
sizeof(void) directly. Most people using void* arithmetic is
thinking it as char *. You should able to add a one line change
in evaluate_ptr_add() to allow void* pointer arithmetic.
Still better than hard code sizeof(void) as 1.
Chris
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/16] Add test for acquire/release
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (6 preceding siblings ...)
2008-12-18 22:32 ` [PATCH 7/16] Let void have sizeof 1 Alexey Zaytsev
@ 2008-12-18 22:33 ` Alexey Zaytsev
2008-12-18 22:33 ` [PATCH 09/16] Add __exact_context__ Alexey Zaytsev
` (7 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:33 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/validation/context.c b/validation/context.c
index 0b45ba3..e8bb125 100644
--- a/validation/context.c
+++ b/validation/context.c
@@ -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 related [flat|nested] 31+ messages in thread* [PATCH 09/16] Add __exact_context__
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (7 preceding siblings ...)
2008-12-18 22:33 ` [PATCH 08/16] Add test for acquire/release Alexey Zaytsev
@ 2008-12-18 22:33 ` Alexey Zaytsev
2008-12-18 22:33 ` [PATCH 10/16] Allow context() attribute on variables Alexey Zaytsev
` (6 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:33 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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(-)
create mode 100644 validation/context-exact.c
diff --git a/ident-list.h b/ident-list.h
index 6104826..13b76d8 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -98,6 +98,7 @@ __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
/* Sparse commands */
IDENT_RESERVED(__context__);
+IDENT_RESERVED(__exact_context__);
IDENT_RESERVED(__range__);
/* Magic function names we recognize */
diff --git a/linearize.c b/linearize.c
index 526a710..111e7af 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1683,6 +1683,7 @@ static pseudo_t linearize_context(struct entrypoint *ep, struct statement *stmt)
value = expr->value;
insn->required = value;
+ insn->exact = stmt->exact;
insn->context_expr = stmt->context;
add_one_insn(ep, insn);
diff --git a/linearize.h b/linearize.h
index 0c5e4ef..69341c2 100644
--- a/linearize.h
+++ b/linearize.h
@@ -117,7 +117,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 */ {
diff --git a/parse.c b/parse.c
index eb31871..5ad8c68 100644
--- a/parse.c
+++ b/parse.c
@@ -52,6 +52,7 @@ static struct token *parse_while_statement(struct token *token, struct statement
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 },
@@ -1811,7 +1817,7 @@ static struct token *parse_goto_statement(struct token *token, struct statement
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;
@@ -1836,6 +1842,8 @@ static struct token *parse_context_statement(struct token *token, struct stateme
stmt->expression = args[0];
stmt->context = NULL;
+ stmt->exact = exact;
+
switch (argc) {
case 0:
sparse_error(token->pos, "__context__ statement needs argument(s)");
@@ -1865,6 +1873,16 @@ static struct token *parse_context_statement(struct token *token, struct stateme
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;
diff --git a/parse.h b/parse.h
index a2b9aa3..ae50720 100644
--- a/parse.h
+++ b/parse.h
@@ -43,6 +43,7 @@ struct statement {
struct expression *expression;
struct expression *context;
struct expression *required;
+ int exact;
};
struct /* return_statement */ {
struct expression *ret_value;
diff --git a/sparse.1 b/sparse.1
index 92a1cae..45eea6f 100644
--- a/sparse.1
+++ b/sparse.1
@@ -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.
diff --git a/sparse.c b/sparse.c
index 785a6f6..a26c8f2 100644
--- a/sparse.c
+++ b/sparse.c
@@ -239,7 +239,7 @@ static int handle_context(struct entrypoint *ep, struct basic_block *bb,
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 entrypoint *ep, struct basic_block *bb,
}
} 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 entrypoint *ep, struct basic_block *bb,
"__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;
diff --git a/validation/context-exact.c b/validation/context-exact.c
new file mode 100644
index 0000000..bacd9a1
--- /dev/null
+++ b/validation/context-exact.c
@@ -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
+ */
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 10/16] Allow context() attribute on variables
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (8 preceding siblings ...)
2008-12-18 22:33 ` [PATCH 09/16] Add __exact_context__ Alexey Zaytsev
@ 2008-12-18 22:33 ` Alexey Zaytsev
2008-12-18 22:34 ` [PATCH 11/16] Evaluate/expand context expressions Alexey Zaytsev
` (5 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:33 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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>
---
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(-)
create mode 100644 validation/context-on-vars.c
diff --git a/linearize.c b/linearize.c
index 111e7af..9f5628a 100644
--- a/linearize.c
+++ b/linearize.c
@@ -30,7 +30,6 @@ static pseudo_t add_setval(struct entrypoint *ep, struct symbol *ctype, struct e
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 = {};
@@ -937,6 +936,8 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
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)) {
@@ -952,6 +953,49 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
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;
}
@@ -989,6 +1033,8 @@ static pseudo_t add_symbol_address(struct entrypoint *ep, struct symbol *sym)
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) {
@@ -996,7 +1042,49 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
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;
}
diff --git a/linearize.h b/linearize.h
index 69341c2..4e9100d 100644
--- a/linearize.h
+++ b/linearize.h
@@ -119,6 +119,7 @@ struct instruction {
struct /* context */ {
int increment, required, inc_false, exact;
struct expression *context_expr;
+ struct symbol *access_var;
};
struct /* asm */ {
const char *string;
diff --git a/parse.c b/parse.c
index 5ad8c68..2ac3d0e 100644
--- a/parse.c
+++ b/parse.c
@@ -884,16 +884,18 @@ static struct token *attribute_mode(struct token *token, struct symbol *attr, st
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++;
@@ -918,6 +920,25 @@ static struct token *_attribute_context(struct token *token, struct symbol *attr
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;
diff --git a/sparse.1 b/sparse.1
index 45eea6f..61c40b8 100644
--- a/sparse.1
+++ b/sparse.1
@@ -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 ))
diff --git a/sparse.c b/sparse.c
index a26c8f2..1149244 100644
--- a/sparse.c
+++ b/sparse.c
@@ -114,6 +114,7 @@ static struct context_check_list *checked_copy(struct context_check_list *ccl)
}
#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 entrypoint *ep, struct basic_block *bb,
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);
diff --git a/symbol.h b/symbol.h
index c4d7f28..f79674f 100644
--- a/symbol.h
+++ b/symbol.h
@@ -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);
diff --git a/validation/context-exact.c b/validation/context-exact.c
index bacd9a1..9b699f8 100644
--- a/validation/context-exact.c
+++ b/validation/context-exact.c
@@ -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
*/
diff --git a/validation/context-on-vars.c b/validation/context-on-vars.c
new file mode 100644
index 0000000..c0fb623
--- /dev/null
+++ b/validation/context-on-vars.c
@@ -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
+ */
diff --git a/validation/context-statement.c b/validation/context-statement.c
index fd79a6a..71d4cae 100644
--- a/validation/context-statement.c
+++ b/validation/context-statement.c
@@ -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
*/
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 11/16] Evaluate/expand context expressions
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (9 preceding siblings ...)
2008-12-18 22:33 ` [PATCH 10/16] Allow context() attribute on variables Alexey Zaytsev
@ 2008-12-18 22:34 ` Alexey Zaytsev
2008-12-18 22:34 ` [PATCH 12/16] Revert the conditional_context patch Alexey Zaytsev
` (4 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:34 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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(-)
diff --git a/evaluate.c b/evaluate.c
index f976645..0c86295 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3364,7 +3364,14 @@ struct symbol *evaluate_statement(struct statement *stmt)
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);
diff --git a/expand.c b/expand.c
index 3e962d1..dece314 100644
--- a/expand.c
+++ b/expand.c
@@ -1169,7 +1169,7 @@ static int expand_statement(struct statement *stmt)
/* 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);
diff --git a/inline.c b/inline.c
index 09d176a..3e984c3 100644
--- a/inline.c
+++ b/inline.c
@@ -328,7 +328,18 @@ static struct statement *copy_one_statement(struct statement *stmt)
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_statement(struct statement *stmt)
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;
}
diff --git a/linearize.c b/linearize.c
index 9f5628a..8e9775d 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1755,22 +1755,11 @@ static pseudo_t linearize_inlined_call(struct entrypoint *ep, struct statement *
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;
diff --git a/parse.c b/parse.c
index 2ac3d0e..fdb7efb 100644
--- a/parse.c
+++ b/parse.c
@@ -1860,9 +1860,7 @@ static struct token *_parse_context_statement(struct token *token, struct statem
token = token->next;
}
- stmt->expression = args[0];
stmt->context = NULL;
-
stmt->exact = exact;
switch (argc) {
@@ -1870,21 +1868,27 @@ static struct token *_parse_context_statement(struct token *token, struct statem
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");
diff --git a/parse.h b/parse.h
index ae50720..fe714b6 100644
--- a/parse.h
+++ b/parse.h
@@ -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 */ {
diff --git a/validation/context.c b/validation/context.c
index e8bb125..2c32ef3 100644
--- a/validation/context.c
+++ b/validation/context.c
@@ -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 problem in 'warn_huge_switch': 'r' expected d
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 related [flat|nested] 31+ messages in thread* [PATCH 12/16] Revert the conditional_context patch
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (10 preceding siblings ...)
2008-12-18 22:34 ` [PATCH 11/16] Evaluate/expand context expressions Alexey Zaytsev
@ 2008-12-18 22:34 ` Alexey Zaytsev
2008-12-18 22:34 ` [PATCH 13/16] Ceck context expressions as expressions Alexey Zaytsev
` (3 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:34 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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(-)
delete mode 100644 validation/context-dynamic.c
diff --git a/linearize.c b/linearize.c
index 8e9775d..34922e9 100644
--- a/linearize.c
+++ b/linearize.c
@@ -439,7 +439,7 @@ const char *show_instruction(struct instruction *insn)
break;
case OP_CONTEXT:
- buf += sprintf(buf, "%s%d,%d", "", insn->increment, insn->inc_false);
+ buf += sprintf(buf, "%d", insn->increment);
break;
case OP_RANGE:
buf += sprintf(buf, "%s between %s..%s", show_pseudo(insn->src1), show_pseudo(insn->src2), show_pseudo(insn->src3));
@@ -960,7 +960,7 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
continue;
insn = alloc_instruction(OP_CONTEXT, 0);
insn->required = context->in;
- insn->increment = insn->inc_false = context->out - context->in;
+ insn->increment = context->out - context->in;
insn->context_expr = context->context;
insn->access_var = ad->source_type;
insn->exact = context->exact;
@@ -973,7 +973,7 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
continue;
insn = alloc_instruction(OP_CONTEXT, 0);
insn->required = context->in;
- insn->increment = insn->inc_false = context->out - context->in;
+ insn->increment = context->out - context->in;
insn->context_expr = context->context;
insn->access_var = ad->result_type;
insn->exact = context->exact;
@@ -988,7 +988,7 @@ static pseudo_t linearize_store_gen(struct entrypoint *ep,
continue;
insn = alloc_instruction(OP_CONTEXT, 0);
insn->required = context->in;
- insn->increment = insn->inc_false = context->out - context->in;
+ insn->increment = context->out - context->in;
insn->context_expr = context->context;
insn->access_var = ad->address->sym;
insn->exact = context->exact;
@@ -1049,7 +1049,7 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
continue;
insn = alloc_instruction(OP_CONTEXT, 0);
insn->required = context->in;
- insn->increment = insn->inc_false = context->out - context->in;
+ insn->increment = context->out - context->in;
insn->context_expr = context->context;
insn->access_var = ad->source_type;
insn->exact = context->exact;
@@ -1062,7 +1062,7 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
continue;
insn = alloc_instruction(OP_CONTEXT, 0);
insn->required = context->in;
- insn->increment = insn->inc_false = context->out - context->in;
+ insn->increment = context->out - context->in;
insn->context_expr = context->context;
insn->access_var = ad->result_type;
insn->exact = context->exact;
@@ -1077,7 +1077,7 @@ static pseudo_t linearize_load_gen(struct entrypoint *ep, struct access_data *ad
continue;
insn = alloc_instruction(OP_CONTEXT, 0);
insn->required = context->in;
- insn->increment = insn->inc_false = context->out - context->in;
+ insn->increment = context->out - context->in;
insn->context_expr = context->context;
insn->access_var = ad->address->sym;
insn->exact = context->exact;
@@ -1322,12 +1322,21 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
FOR_EACH_PTR(ctype->contexts, context) {
int in = context->in;
int out = context->out;
-
- if (out - in || context->out_false - in) {
+ int check = 0;
+ int context_diff;
+ if (in < 0) {
+ check = 1;
+ in = 0;
+ }
+ if (out < 0) {
+ check = 0;
+ out = 0;
+ }
+ context_diff = out - in;
+ if (check || context_diff) {
insn = alloc_instruction(OP_CONTEXT, 0);
- insn->increment = out - in;
+ insn->increment = context_diff;
insn->context_expr = context->context;
- insn->inc_false = context->out_false - in;
add_one_insn(ep, insn);
}
} END_FOR_EACH_PTR(context);
@@ -1757,7 +1766,6 @@ static pseudo_t linearize_context(struct entrypoint *ep, struct statement *stmt)
struct instruction *insn = alloc_instruction(OP_CONTEXT, 0);
insn->increment = stmt->change;
- insn->inc_false = stmt->change;
insn->required = stmt->required;
insn->exact = stmt->exact;
diff --git a/linearize.h b/linearize.h
index 4e9100d..3bd5098 100644
--- a/linearize.h
+++ b/linearize.h
@@ -117,7 +117,7 @@ struct instruction {
struct pseudo_list *arguments;
};
struct /* context */ {
- int increment, required, inc_false, exact;
+ int increment, required, exact;
struct expression *context_expr;
struct symbol *access_var;
};
diff --git a/parse.c b/parse.c
index fdb7efb..5e3354c 100644
--- a/parse.c
+++ b/parse.c
@@ -65,7 +65,6 @@ static struct token *attribute_address_space(struct token *token, struct symbol
static struct token *attribute_aligned(struct token *token, struct symbol *attr, struct ctype *ctype);
static struct token *attribute_mode(struct token *token, struct symbol *attr, struct ctype *ctype);
static struct token *attribute_context(struct token *token, struct symbol *attr, struct ctype *ctype);
-static struct token *attribute_conditional_context(struct token *token, struct symbol *attr, struct ctype *ctype);
static struct token *attribute_exact_context(struct token *token, struct symbol *attr, struct ctype *ctype);
static struct token *attribute_transparent_union(struct token *token, struct symbol *attr, struct ctype *ctype);
static struct token *ignore_attribute(struct token *token, struct symbol *attr, struct ctype *ctype);
@@ -189,10 +188,6 @@ static struct symbol_op context_op = {
.attribute = attribute_context,
};
-static struct symbol_op conditional_context_op = {
- .attribute = attribute_conditional_context,
-};
-
static struct symbol_op exact_context_op = {
.attribute = attribute_exact_context,
};
@@ -279,7 +274,6 @@ static struct init_keyword {
{ "address_space",NS_KEYWORD, .op = &address_space_op },
{ "mode", NS_KEYWORD, .op = &mode_op },
{ "context", NS_KEYWORD, .op = &context_op },
- { "conditional_context", NS_KEYWORD, .op = &conditional_context_op },
{ "exact_context", NS_KEYWORD, .op = &exact_context_op },
{ "__transparent_union__", NS_KEYWORD, .op = &transparent_union_op },
@@ -945,7 +939,6 @@ static struct token *_attribute_context(struct token *token, struct symbol *attr
}
context->exact = exact;
- context->out_false = context->out;
if (argc)
add_ptr_list(&ctype->contexts, context);
@@ -964,51 +957,6 @@ static struct token *attribute_exact_context(struct token *token, struct symbol
return _attribute_context(token, attr, ctype, 1);
}
-static struct token *attribute_conditional_context(struct token *token, struct symbol *attr, struct ctype *ctype)
-{
- struct context *context = alloc_context();
- struct expression *args[4];
- int argc = 0;
-
- token = expect(token, '(', "after conditional_context attribute");
- while (!match_op(token, ')')) {
- struct expression *expr = NULL;
- token = conditional_expression(token, &expr);
- if (!expr)
- break;
- if (argc < 4)
- args[argc++] = expr;
- else
- argc++;
- if (!match_op(token, ','))
- break;
- token = token->next;
- }
-
- switch(argc) {
- case 3:
- context->in = get_expression_value(args[0]);
- context->out = get_expression_value(args[1]);
- context->out_false = get_expression_value(args[2]);
- break;
- case 4:
- context->context = args[0];
- context->in = get_expression_value(args[1]);
- context->out = get_expression_value(args[2]);
- context->out_false = get_expression_value(args[3]);
- break;
- default:
- sparse_error(token->pos, "invalid number of arguments to conditional_context attribute");
- break;
- }
-
- if (argc)
- add_ptr_list(&ctype->contexts, context);
-
- token = expect(token, ')', "after conditional_context attribute");
- return token;
-}
-
static struct token *attribute_transparent_union(struct token *token, struct symbol *attr, struct ctype *ctype)
{
if (Wtransparent_union)
diff --git a/sparse.1 b/sparse.1
index 61c40b8..ebe62f0 100644
--- a/sparse.1
+++ b/sparse.1
@@ -101,15 +101,6 @@ be required on both reads and writes) by using either the token "read" or
the token "write" for an optional fourth argument:
.BI __attribute__((context( expression , in_context , out_context , read|write )).
-To indicate that a certain function acquires a context depending on its
-return value, use
-.BI __attribute__((conditional_context( [expression ,] in_context , out_success , out_failure ))
-where \fIout_success\fR and \fIout_failure\fR indicate the context change
-done depending on success (non-zero) or failure (zero) return of the
-function. Note that currently, using this attribute on a function means that
-the function itself won't be checked for context handling at all. See the
-testsuite for examples.
-
Sparse will warn when it sees a function change a
context without indicating this with a \fBcontext\fR or \fBexact_context\fR attribute, either by
decreasing a context below zero (such as by releasing a lock without acquiring
diff --git a/sparse.c b/sparse.c
index 1149244..76b1db2 100644
--- a/sparse.c
+++ b/sparse.c
@@ -25,7 +25,7 @@
#include "linearize.h"
struct context_check {
- int val, val_false;
+ int val;
char name[32];
};
@@ -44,7 +44,7 @@ static const char *context_name(struct context *context)
}
static void context_add(struct context_check_list **ccl, const char *name,
- int offs, int offs_false)
+ int offs)
{
struct context_check *check, *found = NULL;
@@ -62,7 +62,6 @@ static void context_add(struct context_check_list **ccl, const char *name,
add_ptr_list(ccl, found);
}
found->val += offs;
- found->val_false += offs_false;
}
static int context_list_has(struct context_check_list *ccl,
@@ -73,12 +72,11 @@ static int context_list_has(struct context_check_list *ccl,
FOR_EACH_PTR(ccl, check) {
if (strcmp(c->name, check->name))
continue;
- return check->val == c->val &&
- check->val_false == c->val_false;
+ return check->val == c->val;
} END_FOR_EACH_PTR(check);
/* not found is equal to 0 */
- return c->val == 0 && c->val_false == 0;
+ return c->val == 0;
}
static int context_lists_equal(struct context_check_list *ccl1,
@@ -107,7 +105,7 @@ static struct context_check_list *checked_copy(struct context_check_list *ccl)
struct context_check *c;
FOR_EACH_PTR(ccl, c) {
- context_add(&result, c->name, c->val_false, c->val_false);
+ context_add(&result, c->name, c->val);
} END_FOR_EACH_PTR(c);
return result;
@@ -140,7 +138,7 @@ static int context_list_check(struct entrypoint *ep, struct position pos,
/* make sure the loop below checks all */
FOR_EACH_PTR(ccl_target, c1) {
- context_add(&ccl_cur, c1->name, 0, 0);
+ context_add(&ccl_cur, c1->name, 0);
} END_FOR_EACH_PTR(c1);
FOR_EACH_PTR(ccl_cur, c1) {
@@ -289,15 +287,14 @@ static int handle_context(struct entrypoint *ep, struct basic_block *bb,
return -1;
}
- context_add(combined, name, insn->increment, insn->inc_false);
+ context_add(combined, name, insn->increment);
return 0;
}
static int check_bb_context(struct entrypoint *ep, struct basic_block *bb,
struct context_check_list *ccl_in,
- struct context_check_list *ccl_target,
- int in_false)
+ struct context_check_list *ccl_target)
{
struct context_check_list *combined = NULL, *done;
struct context_check *c;
@@ -327,10 +324,7 @@ static int check_bb_context(struct entrypoint *ep, struct basic_block *bb,
* for the conditional_context() attribute.
*/
FOR_EACH_PTR(ccl_in, c) {
- if (in_false)
- context_add(&combined, c->name, c->val_false, c->val_false);
- else
- context_add(&combined, c->name, c->val, c->val);
+ context_add(&combined, c->name, c->val);
} END_FOR_EACH_PTR(c);
/* Add the new context to the list of already-checked contexts */
@@ -356,18 +350,18 @@ static int check_bb_context(struct entrypoint *ep, struct basic_block *bb,
case OP_BR:
if (insn->bb_true)
if (check_bb_context(ep, insn->bb_true,
- combined, ccl_target, 0))
+ combined, ccl_target))
goto out;
if (insn->bb_false)
if (check_bb_context(ep, insn->bb_false,
- combined, ccl_target, 1))
+ combined, ccl_target))
goto out;
break;
case OP_SWITCH:
case OP_COMPUTEDGOTO:
FOR_EACH_PTR(insn->multijmp_list, mj) {
if (check_bb_context(ep, mj->target,
- combined, ccl_target, 0))
+ combined, ccl_target))
goto out;
} END_FOR_EACH_PTR(mj);
break;
@@ -579,14 +573,11 @@ static void check_context(struct entrypoint *ep)
FOR_EACH_PTR(sym->ctype.contexts, context) {
const char *name = context_name(context);
- context_add(&ccl_in, name, context->in, context->in);
- context_add(&ccl_target, name, context->out, context->out_false);
- /* we don't currently check the body of trylock functions */
- if (context->out != context->out_false)
- return;
+ context_add(&ccl_in, name, context->in);
+ context_add(&ccl_target, name, context->out);
} END_FOR_EACH_PTR(context);
- check_bb_context(ep, ep->entry->bb, ccl_in, ccl_target, 0);
+ check_bb_context(ep, ep->entry->bb, ccl_in, ccl_target);
free_ptr_list(&ccl_in);
free_ptr_list(&ccl_target);
free_bb_context_lists(ep->entry->bb);
diff --git a/symbol.h b/symbol.h
index f79674f..4e605c2 100644
--- a/symbol.h
+++ b/symbol.h
@@ -77,7 +77,7 @@ enum context_read_write_specifier {
struct context {
struct expression *context;
- unsigned int in, out, out_false;
+ unsigned int in, out;
int exact;
enum context_read_write_specifier rws;
};
diff --git a/validation/context-dynamic.c b/validation/context-dynamic.c
deleted file mode 100644
index 5e172f0..0000000
--- a/validation/context-dynamic.c
+++ /dev/null
@@ -1,171 +0,0 @@
-static void a(void) __attribute__ ((context(A, 0, 1)))
-{
- __context__(A, 1);
-}
-
-static void r(void) __attribute__ ((context(A, 1, 0)))
-{
- __context__(A, -1);
-}
-
-extern int condition, condition2;
-
-static int tl(void) __attribute__ ((conditional_context(A, 0, 1, 0)))
-{
- if (condition) {
- a();
- return 1;
- }
- return 0;
-}
-
-static int tl2(void) __attribute__ ((conditional_context(A, 0, 0, 1)))
-{
- if (condition) {
- a();
- return 1;
- }
- return 0;
-}
-
-static int dummy(void)
-{
- return condition + condition2;
-}
-
-static int good_trylock1(void)
-{
- if (tl()) {
- r();
- }
-}
-
-static int good_trylock2(void)
-{
- if (tl()) {
- r();
- }
-
- if (tl()) {
- r();
- }
-}
-static int good_trylock3(void)
-{
- a();
- if (tl()) {
- r();
- }
- r();
- if (tl()) {
- r();
- }
-}
-
-static int good_trylock4(void)
-{
- a();
- if (tl()) {
- r();
- }
- if (tl()) {
- r();
- }
- r();
-}
-
-static void bad_trylock1(void)
-{
- a();
- if (dummy()) {
- r();
- }
- r();
-}
-
-static int good_trylock5(void)
-{
- if (!tl2()) {
- r();
- }
-}
-
-static int good_trylock6(void)
-{
- if (!tl2()) {
- r();
- }
-
- if (!tl2()) {
- r();
- }
-}
-static int good_trylock7(void)
-{
- a();
- if (!tl2()) {
- r();
- }
- r();
- if (!tl2()) {
- r();
- }
-}
-
-static int good_trylock8(void)
-{
- a();
- if (!tl2()) {
- r();
- }
- if (!tl2()) {
- r();
- }
- r();
-}
-
-static void bad_trylock2(void)
-{
- a();
- if (!dummy()) {
- r();
- }
- r();
-}
-
-static int good_switch(void)
-{
- switch (condition) {
- case 1:
- a();
- break;
- case 2:
- a();
- break;
- case 3:
- a();
- break;
- default:
- a();
- }
- r();
-}
-
-static void bad_lock1(void)
-{
- r();
- a();
-}
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 13/16] Ceck context expressions as expressions
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (11 preceding siblings ...)
2008-12-18 22:34 ` [PATCH 12/16] Revert the conditional_context patch Alexey Zaytsev
@ 2008-12-18 22:34 ` Alexey Zaytsev
2008-12-18 22:35 ` [PATCH 14/16] Test conditional result locking Alexey Zaytsev
` (2 subsequent siblings)
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:34 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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>
---
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(-)
create mode 100644 validation/context-vars.c
diff --git a/evaluate.c b/evaluate.c
index 0c86295..0f6402d 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2961,8 +2961,16 @@ struct symbol *evaluate_expression(struct expression *expr)
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);
@@ -3032,6 +3040,7 @@ static void check_duplicates(struct symbol *sym)
static struct symbol *evaluate_symbol(struct symbol *sym)
{
struct symbol *base_type;
+ struct context *c;
if (!sym)
return sym;
@@ -3061,6 +3070,13 @@ static struct symbol *evaluate_symbol(struct symbol *sym)
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;
diff --git a/expand.c b/expand.c
index dece314..2a63f65 100644
--- a/expand.c
+++ b/expand.c
@@ -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;
}
diff --git a/expression.c b/expression.c
index f634b07..198816b 100644
--- a/expression.c
+++ b/expression.c
@@ -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(struct token *token, struct expression_list
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(struct token *token, struct 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;
}
@@ -896,6 +995,7 @@ struct token *conditional_expression(struct token *token, struct expression **tr
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[] = {
@@ -906,15 +1006,19 @@ struct token *assignment_expression(struct token *token, struct expression **tre
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;
}
@@ -931,4 +1035,170 @@ struct token *parse_expression(struct token *token, struct expression **tree)
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;
+}
diff --git a/expression.h b/expression.h
index 5136b9b..5647d88 100644
--- a/expression.h
+++ b/expression.h
@@ -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 token *, struct statement *);
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
diff --git a/ident-list.h b/ident-list.h
index 13b76d8..bf15bca 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -100,6 +100,7 @@ __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
IDENT_RESERVED(__context__);
IDENT_RESERVED(__exact_context__);
IDENT_RESERVED(__range__);
+IDENT_RESERVED(RESULT);
/* Magic function names we recognize */
IDENT(memset); IDENT(memcpy);
diff --git a/inline.c b/inline.c
index 3e984c3..73112ad 100644
--- a/inline.c
+++ b/inline.c
@@ -56,7 +56,7 @@ static struct symbol_list *copy_symbol_list(struct symbol_list *src)
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 *src, struct statement *dst)
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 *expr, struct symbol *sym)
stmt->args = decl;
}
stmt->inline_fn = sym;
+ stmt->inlined_fn_expr = expr;
unset_replace_list(fn_symbol_list);
diff --git a/linearize.c b/linearize.c
index 34922e9..808b1bc 100644
--- a/linearize.c
+++ b/linearize.c
@@ -35,6 +35,7 @@ static pseudo_t linearize_initializer(struct entrypoint *ep, struct expression *
struct pseudo void_pseudo = {};
static struct position current_pos;
+static int in_inline_skip_context = 0;
ALLOCATOR(pseudo_user, "pseudo_user");
@@ -438,9 +439,15 @@ const char *show_instruction(struct instruction *insn)
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;
@@ -1282,7 +1289,7 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
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!");
@@ -1318,29 +1325,17 @@ static pseudo_t linearize_call_expression(struct entrypoint *ep, struct expressi
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;
}
@@ -1714,6 +1709,8 @@ static pseudo_t linearize_compound_statement(struct entrypoint *ep, struct 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);
@@ -1741,6 +1738,7 @@ static pseudo_t linearize_inlined_call(struct entrypoint *ep, struct statement *
struct statement *args = stmt->args;
struct basic_block *bb;
pseudo_t pseudo;
+ struct context *c;
if (args) {
struct symbol *sym;
@@ -1752,12 +1750,27 @@ static pseudo_t linearize_inlined_call(struct entrypoint *ep, struct statement *
} 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;
}
diff --git a/linearize.h b/linearize.h
index 3bd5098..08bc7ec 100644
--- a/linearize.h
+++ b/linearize.h
@@ -120,6 +120,7 @@ struct instruction {
int increment, required, exact;
struct expression *context_expr;
struct symbol *access_var;
+ struct symbol *called_fn;
};
struct /* asm */ {
const char *string;
diff --git a/parse.c b/parse.c
index 5e3354c..25ae267 100644
--- a/parse.c
+++ b/parse.c
@@ -880,12 +880,13 @@ static struct token *_attribute_context(struct token *token, struct symbol *attr
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;
@@ -911,12 +912,14 @@ static struct token *_attribute_context(struct token *token, struct symbol *attr
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]);
@@ -2097,6 +2100,7 @@ static struct token *parse_function_body(struct token *token, struct symbol *dec
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) {
@@ -2125,6 +2129,11 @@ static struct token *parse_function_body(struct token *token, struct symbol *dec
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);
@@ -2287,11 +2296,17 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
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))) {
diff --git a/parse.h b/parse.h
index fe714b6..be7f270 100644
--- a/parse.h
+++ b/parse.h
@@ -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;
diff --git a/sparse.c b/sparse.c
index 76b1db2..9f4ec03 100644
--- a/sparse.c
+++ b/sparse.c
@@ -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, struct context_check);
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_check_list **ccl, const char *name,
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 context_check_list *ccl,
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 *checked_copy(struct context_check_list *ccl)
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 *checked_copy(struct context_check_list *ccl)
#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 entrypoint *ep, struct position pos,
{
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 entrypoint *ep, struct position pos,
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 entrypoint *ep, struct position pos,
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 entrypoint *ep, struct position pos,
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 entrypoint *ep, struct basic_block *bb,
}
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 entrypoint *ep, struct basic_block *bb,
"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 entrypoint *ep, struct basic_block *bb,
}
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 entrypoint *ep, struct basic_block *bb,
* 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 entrypoint *ep, struct basic_block *bb,
*/
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 entrypoint *ep)
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);
diff --git a/symbol.h b/symbol.h
index 4e605c2..b3283c1 100644
--- a/symbol.h
+++ b/symbol.h
@@ -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;
diff --git a/validation/context-vars.c b/validation/context-vars.c
new file mode 100644
index 0000000..a44bfd8
--- /dev/null
+++ b/validation/context-vars.c
@@ -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
+ */
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH 14/16] Test conditional result locking
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (12 preceding siblings ...)
2008-12-18 22:34 ` [PATCH 13/16] Ceck context expressions as expressions Alexey Zaytsev
@ 2008-12-18 22:35 ` Alexey Zaytsev
2008-12-18 22:35 ` [PATCH 15/16] Show required context in instruction output Alexey Zaytsev
2008-12-18 22:35 ` [PATCH 16/16] Check inlines explicitly Alexey Zaytsev
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:35 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/validation/context-vars.c b/validation/context-vars.c
index a44bfd8..a0c966f 100644
--- a/validation/context-vars.c
+++ b/validation/context-vars.c
@@ -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 related [flat|nested] 31+ messages in thread* [PATCH 15/16] Show required context in instruction output
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (13 preceding siblings ...)
2008-12-18 22:35 ` [PATCH 14/16] Test conditional result locking Alexey Zaytsev
@ 2008-12-18 22:35 ` Alexey Zaytsev
2008-12-18 22:35 ` [PATCH 16/16] Check inlines explicitly Alexey Zaytsev
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:35 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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 deletions(-)
diff --git a/expression.c b/expression.c
index 198816b..8c4451d 100644
--- a/expression.c
+++ b/expression.c
@@ -648,6 +648,8 @@ static struct token *postfix_expression(struct token *token, struct 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)
diff --git a/linearize.c b/linearize.c
index 808b1bc..c9fe922 100644
--- a/linearize.c
+++ b/linearize.c
@@ -445,7 +445,7 @@ const char *show_instruction(struct instruction *insn)
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 related [flat|nested] 31+ messages in thread* [PATCH 16/16] Check inlines explicitly
2008-12-18 21:51 [PATCH 00/16] More patches Alexey Zaytsev
` (14 preceding siblings ...)
2008-12-18 22:35 ` [PATCH 15/16] Show required context in instruction output Alexey Zaytsev
@ 2008-12-18 22:35 ` Alexey Zaytsev
15 siblings, 0 replies; 31+ messages in thread
From: Alexey Zaytsev @ 2008-12-18 22:35 UTC (permalink / raw)
To: Josh Triplett; +Cc: Johannes Berg, linux-sparse, Christopher Li
From: Johannes Berg <johannes@sipsolutions.net>
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 | 24 ++++++++++++++++++++++--
1 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/sparse.c b/sparse.c
index 9f4ec03..0bf85fc 100644
--- a/sparse.c
+++ b/sparse.c
@@ -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 entrypoint *ep, struct basic_block *bb,
*/
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_list *list)
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 related [flat|nested] 31+ messages in thread