Re: [PATCH RFC] Let pseudo->users loop on duplicate version of list

[Christopher Li <sparse@chrisli.org>]

On Wed, Jul 12, 2017 at 11:05 AM, Dibyendu Majumdar
<mobile@majumdar.org.uk> wrote:
>
> I did raise this before
> (http://marc.info/?l=linux-sparse&m=149943353732715&w=2) but maybe it
> got lost in the other stuff.

Sorry I must miss that part. Yes. the ptrlist ref count will not able to
handle the insert and split the node. The whole idea is def the modify to
the last owner of holding the node. Insert need to split the node right there.

Even lock will not work. The inner loop guy try to get the lock and failed.
What can it do? It can't just retry because the parent is holding the lock.
It is going to be very nasty.

Any way, the patch has been updated to use a full duplicated list.
The git branch is at:
https://git.kernel.org/pub/scm/devel/sparse/sparse.git/log/?h=sparse-next-20170712

The patch follows.

Chris

From 7264a822d9d9e545152ac675fbc5b5e970ce254b Mon Sep 17 00:00:00 2001
From: Christopher Li <sparse@chrisli.org>
Date: Wed, 12 Jul 2017 14:46:50 -0700
Subject: [PATCH] Let pseudo->users loop on duplicate versin of list

pseudo->users list will change during find dominator.
That cause a bug in the ptrlist because the outer loop iterator
is not award of the deletion of the entry.

Let the outer loop using a duplicate version of entry
to avoid this problem for now.

This is to fix the bug report by the ptrlist ref counting check.
With this change, the ptrlist ref counting check can complete
the kernel compile without reporting an error.

Signed-of-By: Christopher Li <sparse@chrisli.org>
---
 flow.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/flow.c b/flow.c
index fce8bde..bfe54f7 100644
--- a/flow.c
+++ b/flow.c
@@ -729,12 +729,21 @@ static void simplify_one_symbol(struct
entrypoint *ep, struct symbol *sym)
 multi_def:
 complex_def:
 external_visibility:
- all = 1;
- FOR_EACH_PTR_REVERSE(pseudo->users, pu) {
- struct instruction *insn = pu->insn;
- if (insn->opcode == OP_LOAD)
- all &= find_dominating_stores(pseudo, insn, ++bb_generation, !mod);
- } END_FOR_EACH_PTR_REVERSE(pu);
+ {
+ /*
+ * find_dominating_stores() will modify the pesudo->users list.
+ * Make a duplicate copy before using it.
+ */
+ struct pseudo_user_list *users = NULL;
+ all = 1;
+ concat_user_list(pseudo->users, &users);
+ FOR_EACH_PTR_REVERSE(users, pu) {
+ struct instruction *insn = pu->insn;
+ if (insn->opcode == OP_LOAD)
+ all &= find_dominating_stores(pseudo, insn, ++bb_generation, !mod);
+ } END_FOR_EACH_PTR_REVERSE(pu);
+ free_ptr_list(&users);
+ }

  /* If we converted all the loads, remove the stores. They are dead */
  if (all && !mod) {
-- 
2.9.4