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 Tue, 25 Aug 2015 08:57:45 GMT

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

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

I assume that the intention of re-using Instructions is partly to save space, but also so
that BIs with the same target can be updated more easily, as only one instruction has to be
changed. Though another way to do this is of course to move the IH to a different part of
the list.

Changing the design to require use of an Instruction Factory would probably require substantial
changes to user code, so I think should be avoided unless absolutely essential. In which case,
I would start again and make the Instructions immutable (I'm still not sure it's necessary
to be able to change them). Likewise, I'm not sure the IH cache is a good idea.

As to considerations of memory usage or time: we need to get the code working properly first.
Which means we really need unit tests that properly represent a wide range of existing use
cases.


was (Author: sebb@apache.org):
I assume that the intention of re-using Instructions is partly to save space, but also so
that BIs with the same target can be updated more easily, as only one instruction has to be
changed. Though another way to do this is of course to move the IH to a different part of
the list.

Changing the design to require use of an Instruction Factory would probably require substantial
changes to user code, so I think should be avoided unless absolutely essential. In which case,
I would start again and make the Instructions immutable (I'm still not sure it's necessary
to be able to change them). Likewise, I'm not sure the IH cache is a good idea.

As to considerations of memory usage or time: we need to get the code working properly first.

> 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, targeters.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