commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sebb (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (BCEL-195) addition of hashCode() to generic/Instruction.java breaks Targeters
Date Mon, 17 Aug 2015 08:15:45 GMT

    [ https://issues.apache.org/jira/browse/BCEL-195?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14698644#comment-14698644
] 

Sebb edited comment on BCEL-195 at 8/17/15 8:15 AM:
----------------------------------------------------

I think the fact that all branch instructions have the same hash is a red herring.
So long as one GOTO 50 compares equal with another GOTO 50, and does not compare equal with
a GOTO 51, then it's possible to ensure that one, and only one, instance is added to the list.
So I don't understand the comment
{quote}
However, one 'goto 50' is not the same as another 'goto 50' due to the way Targeters are implemented.
{quote}

As I see it, the problem here is that when two Instructions have the same hashcode, then the
code needs to use the equals method to distinguish them, and the equals method needs data
that has not yet been set up. 

Removing the definition of the hashCode does not solve this problem, it just makes it very
unlikely to occur (because Object hashCode collisions are deliberately rare). 

An alternative solution would be to ensure that the hashCode for branches took account of
the target somehow.

Or at least allow non-variable instructions to be shared by keeping the existing hashcode
method and overriding it with a call to System.identityHashCode() for variable instructions
such as branches.
This should be easy to test.


was (Author: sebb@apache.org):
I think the fact that all branch instructions have the same hash is a red-herring.
So long as one GOTO 50 compares equal with another GOTO 50, and does not compare equal with
a GOTO 51, then it's possible to ensure that one, and only one, instance is added to the list.
So I don't understand the comment
{quote}
However, one 'goto 50' is not the same as another 'goto 50' due to the way Targeters are implemented.
{quote}

As I see it, the problem here is that when two Instructions have the same hashcode, then the
code needs to use the equals method to distinguish them, and the equals method needs data
that has not yet been set up. 

Removing the definition of the hashCode does not solve this problem, it just makes it very
unlikely to occur (because Object hashCode collisions are deliberately rare). 

An alternative solution would be to ensure that the hashCode for branches took account of
the target somehow.

Or at least allow non-variable instructions to be shared by using System.identityHashCode()
for variable instructions such as branches.
This should be easy to test.

> addition of hashCode() to generic/Instruction.java breaks Targeters
> -------------------------------------------------------------------
>
>                 Key: BCEL-195
>                 URL: https://issues.apache.org/jira/browse/BCEL-195
>             Project: Commons BCEL
>          Issue Type: Bug
>          Components: Main
>    Affects Versions: 6.0
>            Reporter: Mark Roberts
>         Attachments: bcel195.diff
>
>
> [Revision 1532198|http://svn.apache.org/r1532198] added a {{hashCode()}} function to
the Instruction class.  Unfortunately, this breaks the Instruction targeting mechanism. I
understand the goal of trying to reuse instructions - an 'iadd' is the same as any other 'iadd'.
 However,  one 'goto 50' is not the same as another 'goto 50' due to the way Targeters are
implemented.  If branch instructions are reused, then only one entry gets put on the Targeter
list.  So when some api is used to modify the instruction list and location 50 becomes location
52 ONLY ONE of the branches gets updated. A very bad thing.  So unless you modify the hash
to special case branch instructions (and there might be other instructions needing special
treatment as well) its broken.  We fixed it by simply commenting the hash out to make things
like they used to be and all works great.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message