diff options
| author | Luc Van Oostenryck <luc.vanoostenryck@gmail.com> | 2017-07-04 21:40:13 +0200 |
|---|---|---|
| committer | Luc Van Oostenryck <luc.vanoostenryck@gmail.com> | 2017-07-31 17:25:51 +0200 |
| commit | d254b816c4b6b8a692507567ad9127ca9991596d (patch) | |
| tree | e538b707b12238bf345e8e98ca4c258ce64cc0ff | |
| parent | ec3f72e981792a86a9e002471a06d61ecd5c6675 (diff) | |
| download | sparse-dev-d254b816c4b6b8a692507567ad9127ca9991596d.tar.gz | |
fix ptrlist corruption while killing unreachable BBs
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:
1) refuse to emit the "crazy programmer" warning if there
is a potential dead BB
2) move kill_unreachable_bbs() in the main cleanup loop
which will avoid nested ep->bbs loop.
Note: this solution is preferable to some others because
the "crazy programmer" condition happens very rarely.
It this thus better to delay this check than to call
kill_unreachable_bbs() preventively.
Note: the reproducer is one with very broken syntax but nothing
forbid the same situation to happen with a valid program.
Fixes: 51cfbc90a5e1462fcd624a1598ecd985a508a5d6
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
| -rw-r--r-- | cse.c | 2 | ||||
| -rw-r--r-- | flow.c | 2 | ||||
| -rw-r--r-- | linearize.c | 3 | ||||
| -rw-r--r-- | simplify.c | 9 | ||||
| -rw-r--r-- | validation/crash-ptrlist.c | 23 | ||||
| -rw-r--r-- | validation/crazy02-not-so.c | 18 |
6 files changed, 52 insertions, 5 deletions
@@ -364,6 +364,8 @@ void cleanup_and_cse(struct entrypoint *ep) repeat: repeat_phase = 0; clean_up_insns(ep); + if (repeat_phase & REPEAT_CFG_CLEANUP) + kill_unreachable_bbs(ep); for (i = 0; i < INSN_HASH_SIZE; i++) { struct instruction_list **list = insn_hash_table + i; if (*list) { @@ -840,8 +840,6 @@ void kill_unreachable_bbs(struct entrypoint *ep) DELETE_CURRENT_PTR(bb); } END_FOR_EACH_PTR(bb); PACK_PTR_LIST(&ep->bbs); - - repeat_phase &= ~REPEAT_CFG_CLEANUP; } static int rewrite_parent_branch(struct basic_block *bb, struct basic_block *old, struct basic_block *new) diff --git a/linearize.c b/linearize.c index 7313e72d..a3672077 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); } @@ -879,6 +879,15 @@ offset: if (new == orig) { if (new == VOID) return 0; + /* + * If some BB have been removed it is possible that this + * memop is in fact part of a dead BB. In this case + * we must not warn since nothing is wrong. + * If not part of a dead BB this will be redone after + * the BBs have been cleaned up. + */ + if (repeat_phase & REPEAT_CFG_CLEANUP) + return 0; new = VOID; warning(insn->pos, "crazy programmer"); } diff --git a/validation/crash-ptrlist.c b/validation/crash-ptrlist.c new file mode 100644 index 00000000..8e9b5cb5 --- /dev/null +++ b/validation/crash-ptrlist.c @@ -0,0 +1,23 @@ +a; +char b; +c() { + while () { + int *d; + while () { + char *e = &b; + if (a ?: (*d = f || *0) || g) { + if + ; + } else { + int h = 0; + if (j) { + char **i = &e; + if (0 ? 0 : 0 ?: (**i = 1 || 0 && 0)) h ?: (*e = i && &h + +/* + * check-name: crash ptrlist + * check-command: test-linearize $file + * + * check-error-ignore + * check-output-ignore + */ diff --git a/validation/crazy02-not-so.c b/validation/crazy02-not-so.c index fe713358..19ee2529 100644 --- a/validation/crazy02-not-so.c +++ b/validation/crazy02-not-so.c @@ -16,6 +16,24 @@ int foo(int *ptr, int i) return 1; } +int bar(int *ptr, int i) +{ + int *p; + + switch (i - i) { // will be optimized to 0 + case 0: + return 0; + case 1: // will be optimized away + // p is uninitialized + do { // will be an unreachable loop + *p++ = 123; + } while (--i); + break; + } + + return 1; +} + /* * check-name: crazy02-not-so.c * check-command: sparse -Wno-decl $file |