Current sparse code doesn't handle very gracefully
code with undefined pseudos. These can occurs, of
course because the dev has not initialize them,
maybe by error, but they can also occurs when they
are only set (and used) in a single path or even worse,
hen initiializing a bitfield.
The origin of the problem is the single store shortcut
in simplify_one_symbol() which doesn't take in account
the (absence of) dominance when loads exist without
stores.
Fix this by removing this single-store shortcut and leave
all the work for the general case which handle this
situation quite well (and/but set those variables to 0).
Warning: this change impact the performance.
Note: this patch will also avoid internal infinite loops
occuring when processing undefined vars present
during the SSA construction.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
---
flow.c | 39 ++-------------------------------------
validation/infinite-loop0.c | 1 -
2 files changed, 2 insertions(+), 38 deletions(-)
diff --git a/flow.c b/flow.c
index c7161d47e..5a3d55f76 100644
--- a/flow.c
+++ b/flow.c
@@ -650,11 +650,10 @@ void check_access(struct instruction *insn)
static void simplify_one_symbol(struct entrypoint *ep, struct symbol *sym)
{
- pseudo_t pseudo, src;
+ pseudo_t pseudo;
struct pseudo_user *pu;
- struct instruction *def;
unsigned long mod;
- int all, stores, complex;
+ int all;
/* Never used as a symbol? */
pseudo = sym->pseudo;
@@ -670,17 +669,12 @@ static void simplify_one_symbol(struct entrypoint *ep, struct symbol *sym)
if (mod)
goto external_visibility;
- def = NULL;
- stores = 0;
- complex = 0;
FOR_EACH_PTR(pseudo->users, pu) {
/* We know that the symbol-pseudo use is the "src" in the instruction */
struct instruction *insn = pu->insn;
switch (insn->opcode) {
case OP_STORE:
- stores++;
- def = insn;
break;
case OP_LOAD:
break;
@@ -697,37 +691,8 @@ static void simplify_one_symbol(struct entrypoint *ep, struct symbol *sym)
default:
warning(sym->pos, "symbol '%s' pseudo used in unexpected way", show_ident(sym->ident));
}
- complex |= insn->offset;
} END_FOR_EACH_PTR(pu);
- if (complex)
- goto complex_def;
- if (stores > 1)
- goto multi_def;
-
- /*
- * Goodie, we have a single store (if even that) in the whole
- * thing. Replace all loads with moves from the pseudo,
- * replace the store with a def.
- */
- src = VOID;
- if (def)
- src = def->target;
-
- FOR_EACH_PTR(pseudo->users, pu) {
- struct instruction *insn = pu->insn;
- if (insn->opcode == OP_LOAD) {
- check_access(insn);
- convert_load_instruction(insn, src);
- }
- } END_FOR_EACH_PTR(pu);
-
- /* Turn the store into a no-op */
- kill_store(def);
- return;
-
-multi_def:
-complex_def:
external_visibility:
all = 1;
FOR_EACH_PTR_REVERSE(pseudo->users, pu) {
diff --git a/validation/infinite-loop0.c b/validation/infinite-loop0.c
index a28492307..0e3e3805c 100644
--- a/validation/infinite-loop0.c
+++ b/validation/infinite-loop0.c
@@ -8,5 +8,4 @@ void foo(void)
* check-name: internal infinite loop (0)
* check-command: sparse -Wno-decl $file
* check-timeout:
- * check-known-to-fail
*/
--
2.14.0
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
