Return-Path: X-Original-To: apmail-commons-issues-archive@minotaur.apache.org Delivered-To: apmail-commons-issues-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 75292184DE for ; Mon, 24 Aug 2015 22:46:46 +0000 (UTC) Received: (qmail 96148 invoked by uid 500); 24 Aug 2015 22:46:46 -0000 Delivered-To: apmail-commons-issues-archive@commons.apache.org Received: (qmail 96058 invoked by uid 500); 24 Aug 2015 22:46:46 -0000 Mailing-List: contact issues-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: issues@commons.apache.org Delivered-To: mailing list issues@commons.apache.org Received: (qmail 96043 invoked by uid 99); 24 Aug 2015 22:46:46 -0000 Received: from arcas.apache.org (HELO arcas.apache.org) (140.211.11.28) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 24 Aug 2015 22:46:46 +0000 Date: Mon, 24 Aug 2015 22:46:46 +0000 (UTC) From: "Mark Roberts (JIRA)" To: issues@commons.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Comment Edited] (BCEL-195) addition of hashCode() to generic/Instruction.java breaks Targeters MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/BCEL-195?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14710190#comment-14710190 ] Mark Roberts edited comment on BCEL-195 at 8/24/15 10:45 PM: ------------------------------------------------------------- Okay, I think I grok this now. In a sense, we were both correct. I think you are correct that you can share branches with the same target; but it didn't work properly due to problems in other code. My change to InstructionComparator (inadvertently) worked around these problems. There were two issues if you are going to share branch instructions: - dispose cannot set the target to null because somebody else might be using it. - you cannot allow duplicate targeters (interestingly, the 'fix' was in the code, but commented out) targeters.diff has these changes. Please review and see what you think. I have not had a chance to review the changes to Select.java; I will try to get that done tomorrow. was (Author: markro): Okay, I think I grok this now. In a sense, we were both correct. I think you are correct that you can share branches with the same target; but it didn't work properly due to problems in other code. My change to InstructionComparator (inadvertently) worked around these problems. There were two issues if you are going to share branch instructions: - dispose cannot set the target to null because somebody else might be using it. - you cannot allow duplicate targeters (interestingly, the 'fix' was in the code, but commented out) targeters.diff has these changes. Please review and see what you think. I have not had a chance to review the changes to Select.java; I will try to get that done tomorrow. > 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)