[Bug default/27418] New: DW_FORM_implicit_const handling in checksum_die missing attribute hashing

[Bug default/27418] New: DW_FORM_implicit_const handling in checksum_die missing attribute hashing

[vries at gcc dot gnu.org]

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

            Bug ID: 27418
           Summary: DW_FORM_implicit_const handling in checksum_die
                    missing attribute hashing
           Product: dwz
           Version: unspecified
            Status: NEW
          Severity: normal
          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: ---

This is something that I don't get:
...
        case DW_FORM_implicit_const:
          if (!handled && die->die_ck_state != CK_BAD)
            {
              handled = true;
              die->u.p1.die_hash
                = iterative_hash_object (t->values[i], die->u.p1.die_hash);
            }
          break;
...

Normally, we have something like:
...
              s = t->attr[i].attr;
              die->u.p1.die_hash
                = iterative_hash_object (s, die->u.p1.die_hash);
              die->u.p1.die_hash
                = iterative_hash_object (value, die->u.p1.die_hash);
...
so we hash in both the attribute tag and the attribute value.

Why don't we hash in the attribute tag for DW_FORM_implicit_const?

This sound like a bug that makes hash collisions more likely.

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

[Bug default/27418] DW_FORM_implicit_const handling in checksum_die missing attribute hashing

[mark at klomp dot org]

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

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org

--- Comment #1 from Mark Wielaard <mark at klomp dot org> ---
I think you are right. The bug seems to be the handled = true; That is not
true, we haven't yet handled the attribute completely. We just added the value
to the hash. The code should then continue so it ends up at the block at the
end after the switch statement:

      if (!handled && die->die_ck_state != CK_BAD)
        {
          s = t->attr[i].attr;
          die->u.p1.die_hash = iterative_hash_object (s, die->u.p1.die_hash);
          die->u.p1.die_hash
            = iterative_hash (old_ptr, ptr - old_ptr, die->u.p1.die_hash);
        }

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

[Bug default/27418] DW_FORM_implicit_const handling in checksum_die missing attribute hashing

[vries at gcc dot gnu.org]

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

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

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

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Mark Wielaard from comment #1)
> I think you are right. The bug seems to be the handled = true; That is not
> true, we haven't yet handled the attribute completely. We just added the
> value to the hash. The code should then continue so it ends up at the block
> at the end after the switch statement:
> 
>       if (!handled && die->die_ck_state != CK_BAD)
>         {
>           s = t->attr[i].attr;
>           die->u.p1.die_hash = iterative_hash_object (s, die->u.p1.die_hash);
>           die->u.p1.die_hash
>             = iterative_hash (old_ptr, ptr - old_ptr, die->u.p1.die_hash);
>         }

That does however revert the attribute-value order.  So I've fixed it
differently:
https://sourceware.org/git/?p=dwz.git;a=commit;h=29823e8858b824906d2630683032491125446961

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

[Bug default/27418] DW_FORM_implicit_const handling in checksum_die missing attribute hashing

[mark at klomp dot org]

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

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
(In reply to Tom de Vries from comment #2)
> That does however revert the attribute-value order.  So I've fixed it
> differently:
> https://sourceware.org/git/?p=dwz.git;a=commit;
> h=29823e8858b824906d2630683032491125446961

Yes, keeping the attribute-value order in hashing makes sense. Your way is more
consistent. Thanks.

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