Fix -Wtypesign
RFC PATCH 1/1 Fix -Wtypesign
Hello!
The current sparse has problems with the -Wtypesign flag. If causes bogus warnings. Take for example following test.c:
static unsigned char foobar(void);
static unsigned char foobar(void)
{
return 0;
}$ sparse -Wtypesign test.c test.c:2:22: error: symbol 'foobar' redeclared with different type (originally declared at test.c:1) - different signedness
Debugging shows that the function definition doesn't have the signed/unsigned modifiers, unlike the function declaration:
2412 typediff = type_difference(sym, next, 0, 0);
(gdb) p/ sym->ctype.modifiers
$1 = 0x800004
(gdb) p/ next->ctype.modifiers
$2 = 0x800084
0x80 is MOD_UNSIGNED.
The signedness modifiers are set in evaluate_symbol(), which hasn't been called for sym yet. I have devised following patch, that appears to fix the problem:
---
Fix -Wtypesign
Run evaluate_symbol() before check_duplicates() so that the signedness of the token is known by the time the token is compared with other tokens.
---
evaluate.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
evaluate.c +1 -1
@@ -2473,8 +2473,8 @@ void evaluate_symbol_list(struct symbol_struct symbol *sym; FOR_EACH_PTR(list, sym) { -check_duplicates (sym);evaluate_symbol (sym); +check_duplicates (sym); }END_FOR_EACH_PTR (sym); }
I tested -Wtypesign on some Linux drivers. It found some real sign mismatches and no bogus warnings.
Still, I haven't seen sparse code until today, so the patch should be evaluated carefully.
Signature · 2 lines
Regards, Pavel Roskin
Quoted 18 lines · Pavel Roskin
Fix -Wtypesign
Run
evaluate_symbol()beforecheck_duplicates()so that the signedness of the token is known by the time the token is compared with other tokens.Signed-off-by Pavel Roskin <[email protected]>---
evaluate.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)evaluate.c +0
-0
evaluate.c void evaluate_symbol_list(struct symbol_ · line 2477
Quoted 6 lines
evaluate.c +1
-1@@ -2473,8 +2473,8 @@ void evaluate_symbol_list(struct symbol_struct symbol *sym; FOR_EACH_PTR(list, sym) { -check_duplicates (sym);evaluate_symbol (sym); +check_duplicates (sym);
That, on the face of it, looks "Obviously Correct(tm)".
I'm wondering if I had some reason for doing them in what is obviously the wrong order, which worries me a bit, but quite frankly, the most likely reasons is just that it was a thinko.
(Long long ago, I did "check_duplicates()" from _within_ evaluation, which had serious recursion issues, so there's been confusion here before).
Linus