public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
* [Bug default/25166] Use verify_edges in create_import_tree
  2019-01-01  0:00 [Bug default/25166] New: Use verify_edges in create_import_tree vries at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2019-01-01  0:00 ` vries at gcc dot gnu.org
@ 2019-01-01  0:00 ` vries at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug default/25166] Use verify_edges in create_import_tree
  2019-01-01  0:00 [Bug default/25166] New: Use verify_edges in create_import_tree vries at gcc dot gnu.org
  2019-01-01  0:00 ` [Bug default/25166] " vries at gcc dot gnu.org
  2019-01-01  0:00 ` vries at gcc dot gnu.org
@ 2019-01-01  0:00 ` vries at gcc dot gnu.org
  2019-01-01  0:00 ` vries at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug default/25166] New: Use verify_edges in create_import_tree
@ 2019-01-01  0:00 vries at gcc dot gnu.org
  2019-01-01  0:00 ` [Bug default/25166] " vries at gcc dot gnu.org
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug default/25166] Use verify_edges in create_import_tree
  2019-01-01  0:00 [Bug default/25166] New: Use verify_edges in create_import_tree vries at gcc dot gnu.org
@ 2019-01-01  0:00 ` vries at gcc dot gnu.org
  2019-01-01  0:00 ` vries at gcc dot gnu.org
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug default/25166] Use verify_edges in create_import_tree
  2019-01-01  0:00 [Bug default/25166] New: Use verify_edges in create_import_tree vries at gcc dot gnu.org
  2019-01-01  0:00 ` [Bug default/25166] " vries at gcc dot gnu.org
@ 2019-01-01  0:00 ` vries at gcc dot gnu.org
  2019-01-01  0:00 ` vries at gcc dot gnu.org
  2019-01-01  0:00 ` vries at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: vries at gcc dot gnu.org @ 2019-01-01  0:00 UTC (permalink / raw)
  To: dwz

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-11-17 16:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01  0:00 [Bug default/25166] New: Use verify_edges in create_import_tree vries at gcc dot gnu.org
2019-01-01  0:00 ` [Bug default/25166] " vries at gcc dot gnu.org
2019-01-01  0:00 ` vries at gcc dot gnu.org
2019-01-01  0:00 ` vries at gcc dot gnu.org
2019-01-01  0:00 ` vries at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).