[
https://issues.apache.org/jira/browse/BCEL-195?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14329936#comment-14329936
]
Mark Roberts commented on BCEL-195:
-----------------------------------
I think it would not occur if there was no Instruction.equals as then
InstructionComparator would no longer be called.(?)
The initialization order issue is hidden in the super call to BranchInstruction
init
which calls setTarget
which calls notifyTarget
which calls addTargeter
which calls targeters.add
which calls the java runtime for HashSet
which leads back to Instruction.equals
which calls InstructionComparator.equals
which sees it has a Select and calls getTargets
which returns null as the rest of the select instruction fields have not been
set
and t1.length goes boom
I believe I never saw it before as we had the hash function commented out. You
probably never saw it due to lack of code coverage.
Given that (I believe) InstructionComparator is fixed, I think you want to
leave in Instruction.equals so that the simple instructions do get cached.
And after thinking about this some more to write this email - I'm wondering if
InstructionComparator should always return false for Select as well? The odds
of two identical switches located at different locations in the code is
probably close to zero. Certainly never happens in our exhaustive test suite.
What do you think?
Mark
> 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: b195.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)
