On Thu, Jul 6, 2017 at 12:19 PM, Luc Van Oostenryck
<luc.vanoostenryck@xxxxxxxxx> wrote:
> In commit (51cfbc90a5 "fix: kill unreachable BBs after killing a child")
> kill_unreachable_bbs() was called during simplification in order
> to avoid creating fake cycles between phis and issuing bogus
> "crazy programmer" warnings.
>
> However, simplification is done via cse.c:clean_up_insns()
> which is looping over all BBs while kill_unreachable_bbs()
> loops over all BBs *and* can delete some of them which may
> corrupt the looping in clean_up_insns().
>
> Fix this, by adding a flag to kill_unreachable_bbs(), telling
> if it is safe to delete the BBs or if we can just mark them
> as unreachable (set bb->ep to NULL and unlink any parent and/or
> chilren), in which case the deletion will be done later.
>
> Note: the reproducer is one with very broken syntax but nothing
> seems to forbid the same situation to happen with a valid
> program.
>
> Fixes: 51cfbc90a5e1462fcd624a1598ecd985a508a5d6
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx>
Thanks for catching it. That is a very serious bug.
The release will be on hold until this bug has been fix.
It is actually more than one of them. I will send out a separate patch
catching this kind of behavior.
When the parent is doing the loop on this, the
lower level caller remove the list element is very hasty.
> -void kill_unreachable_bbs(struct entrypoint *ep)
> +void kill_unreachable_bbs(struct entrypoint *ep, int del)
> {
> struct basic_block *bb;
> unsigned long generation = ++bb_generation;
> @@ -837,7 +837,8 @@ void kill_unreachable_bbs(struct entrypoint *ep)
> /* Mark it as being dead */
> kill_bb(bb);
> bb->ep = NULL;
> - DELETE_CURRENT_PTR(bb);
> + if (del)
> + DELETE_CURRENT_PTR(bb);
> } END_FOR_EACH_PTR(bb);
> PACK_PTR_LIST(&ep->bbs);
>
Well, it can happen to other ptr list as well. And it did.
For this one, I thing it is simpler just move the kill_unreachable_bbs()
to the outer stage.
Some thing like this, what do you say?
It did pass the validation test you send out on this patch.
>From c4ef6086b07cdf1d0d1790a274ba056910b00b44 Mon Sep 17 00:00:00 2001
From: Christopher Li <sparse@xxxxxxxxxxx>
Date: Thu, 6 Jul 2017 16:25:38 -0700
Subject: [PATCH 1/2] move kill_unreachable_bbs to outer cse stage
---
cse.c | 3 +++
linearize.c | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cse.c b/cse.c
index 0d3815c..af9863f 100644
--- a/cse.c
+++ b/cse.c
@@ -387,6 +387,9 @@ repeat:
}
}
+ if (repeat_phase & REPEAT_CFG_CLEANUP)
+ kill_unreachable_bbs(ep);
+
if (repeat_phase & REPEAT_SYMBOL_CLEANUP)
simplify_memops(ep);
diff --git a/linearize.c b/linearize.c
index 7313e72..a367207 100644
--- a/linearize.c
+++ b/linearize.c
@@ -671,9 +671,6 @@ void insert_branch(struct basic_block *bb, struct
instruction *jmp, struct basic
remove_parent(child, bb);
} END_FOR_EACH_PTR(child);
PACK_PTR_LIST(&bb->children);
-
- if (repeat_phase & REPEAT_CFG_CLEANUP)
- kill_unreachable_bbs(bb->ep);
}
--
2.9.4
--
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
