[Bug default/25166] Use verify_edges in create_import_tree

[Bug default/25166] Use verify_edges in create_import_tree

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25166

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #0)
> We need to figure out whether the assert needs fixing, or whether the check
> is simply not valid anymore at that point.
> 

Assertion failure fixed here:
https://sourceware.org/git/?p=dwz.git;a=commit;h=a4668e17b685a905a2f45668f075f2ba068b77c0
.

> After that we can enable this functionality using --devel-verify-edges or
> some such.

Added --devel-verify-edges here:
https://sourceware.org/git/?p=dwz.git;a=commit;h=88546aea51cb2382107d9e6751380620fa6ca056
.

Marking resolved-fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25166] Use verify_edges in create_import_tree

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25166

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
> 3.
> PRINT_EDGES
> PUs
> idx: 0
> cu: 0x0
> incoming: 0
> incoming: 4
> idx: 1
> cu: 0x1
> incoming: 2
> incoming: 3
> outgoing: 0

Hmm, actually, the self edge is probably supposed to refer to PU 1.

Because PU 1 has an outgoing edge to PU 0, but there's no corresponding
incoming edge.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25166] New: Use verify_edges in create_import_tree

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25166

            Bug ID: 25166
           Summary: Use verify_edges in create_import_tree
           Product: dwz
           Version: unspecified
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: default
          Assignee: nobody at sourceware dot org
          Reporter: vries at gcc dot gnu.org
                CC: dwz at sourceware dot org
  Target Milestone: ---

There's a function verify_edges, which is enabled by -DDEBUG_VERIFY_EDGES, but
is not called anywhere.

Calling verify_edges here:
...
@@ -6249,6 +6250,7 @@ create_import_tree (void)
   for (; puidx < npus + ncus; puidx++)
     ipus[puidx]->idx = puidx;
   last_pu = ipus[npus - 1];
+  verify_edges (ipus, npus, ncus);
   /* Now, for the above constructed bipartite graph, find K x,2 components
      where x >= 5 (for DWARF3 and above or ptr_size 4, for DWARF2 and
      ptr_size 8 it can be even x == 4) and add a new PU node, where all
@@ -6407,6 +6409,7 @@ create_import_tree (void)
            }
        }
     }
+  verify_edges (ipus, npus, ncus);
   /* Try to merge PUs which have the same set of referrers if
      beneficial, or if one PU has a subset of referrers of the
      other, attempt to replace all the incoming edges from the
...
doesn't give any errors in the dwz testsuite (and in the external one).

OTOH, calling verify_edges one step later:
...
@@ -6633,6 +6636,7 @@ create_import_tree (void)
       for (icu = ipu->next; icu; icu = icu->next)
        icu->seen = false;
     }
+  verify_edges (ipus, npus, ncus);
   /* Create DW_TAG_partial_unit (and containing dw_cu structures).  */
   if (fi_multifile)
     {
...
gives us:
...
$ cp hello 1; cp 1 2; ./dwz -m 3 1 2
dwz: dwz.c:6042: verify_edges_1: Assertion `e2' failed.
...

We need to figure out whether the assert needs fixing, or whether the check is
simply not valid anymore at that point.

After that we can enable this functionality using --devel-verify-edges or some
such.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25166] Use verify_edges in create_import_tree

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25166

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
I added some printing of the edges before verification.

...
1.
PRINT_EDGES
PUs
idx: 0
cu: 0x0
incoming: 2
incoming: 3
incoming: 4
idx: 1
cu: 0x1
incoming: 2
incoming: 3
CUs
idx: 2
cu: 0x2e
outgoing: 0
outgoing: 1
idx: 3
cu: 0xc7
outgoing: 0
outgoing: 1
idx: 4
cu: 0x3e1
outgoing: 0

2.
PRINT_EDGES
PUs
idx: 0
cu: 0x0
incoming: 2
incoming: 3
incoming: 4
idx: 1
cu: 0x1
incoming: 2
incoming: 3
CUs
idx: 2
cu: 0x2e
outgoing: 0
outgoing: 1
idx: 3
cu: 0xc7
outgoing: 0
outgoing: 1
idx: 4
cu: 0x3e1
outgoing: 0

3.
PRINT_EDGES
PUs
idx: 0
cu: 0x0
incoming: 0
incoming: 4
idx: 1
cu: 0x1
incoming: 2
incoming: 3
outgoing: 0
CUs
idx: 2
cu: 0x2e
outgoing: 1
idx: 3
cu: 0xc7
outgoing: 1
idx: 4
cu: 0x3e1
outgoing: 0
dwz: /data/dwz/dwz.git/dwz.c:6071: verify_edges_1: Assertion `e2' failed.
...

The first two calls give identical output.

The third show a transformed import tree, which looks good apart from one
oddity:
...
PUs
idx: 0
cu: 0x0
incoming: 0
incoming: 4
...
PU 0 has an incoming edge from itself.

The verification assert trips on this, because there's no corresponding
outgoing edge.

In the code that is generated, there's no import statement in PU 0 that imports
itself, so it looks like this oddity doesn't have negative consequences in code
generation.

So, this merely looks like insufficient cleanup, which is not terribly
important.

It would be good to fix though, because it would allow us to run the edge
verification at the point where it is now asserting.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/25166] Use verify_edges in create_import_tree

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=25166

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed by:
...
diff --git a/dwz.c b/dwz.c
index ac02e82..867b217 100644
--- a/dwz.c
+++ b/dwz.c
@@ -6655,7 +6655,7 @@ create_import_tree (void)
                  assert (*ep == NULL || (*ep)->icu != ipusub);
                  e4 = edge_freelist;
                  edge_freelist = edge_freelist->next;
-                 e4->icu = ipusup;
+                 e4->icu = ipusub;
                  e4->next = *ep;
                  *ep = e4;
                  ipusup->incoming_count -= ipusub->incoming_count - 1;
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.