[
https://issues.apache.org/jira/browse/BCEL-195?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14711725#comment-14711725
]
Mark Roberts commented on BCEL-195:
-----------------------------------
This problem is very complicated and there are a number of factors in play.
One is that the objects in the HashSet of targeters are modified which violates
the basic Set contract. BranchInstructions are added to the set with their
target==null. Then the branch target is set with the correct value. This
means that there are duplicates in the set if the target is branched to from
more than one location. BUT, this turns out to be a good thing because you
really do want a separate targeter for each branch so they can be modified
independently of each other (more on this later). The problem arises when we
replace a branch instruction and, hence, need to remove its targeter from the
target set. Since all the branch instructions in the targeters set now compare
equal, an arbitrary one is removed and is disposed. The correct solution
(ignoring the whole issue of Factories for now) is that the targeter entries
really should be the instruction handle of the branch source not the branch
instruction itself. OR, and much simpler, we go back to my original solution
of changing InstructionComparator to say that two branches are never equal.
This works because when we dispose the branch instruction nothing happens BUT
we set the target back to null. Then when we dispose the containing
InstructionHandle it calls dispose on the instruction again and now it matches
how it was originally added to the set and gets deleted. I understand this is
nasty but I would really like to adopt this solution for now. I have used it
for months and I know it works. I've spent two additional days on this issue
and would really like to work on the other open items. Thank you.
> 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)
