commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Roberts (JIRA)" <>
Subject [jira] [Commented] (BCEL-195) addition of hashCode() to generic/ breaks Targeters
Date Tue, 25 Aug 2015 18:21:46 GMT


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/ breaks Targeters
> -------------------------------------------------------------------
>                 Key: BCEL-195
>                 URL:
>             Project: Commons BCEL
>          Issue Type: Bug
>          Components: Main
>    Affects Versions: 6.0
>            Reporter: Mark Roberts
>         Attachments: bcel195.diff, targeters.diff
> [Revision 1532198|] 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

View raw message