commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Emmanuel Bourg (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (BCEL-69) A bug in LocalVariableGen
Date Thu, 24 Apr 2014 12:09:15 GMT

     [ https://issues.apache.org/jira/browse/BCEL-69?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Emmanuel Bourg updated BCEL-69:
-------------------------------

          Description: 
I'm one of the developers working on the Jakarta commons sandbox javaflow project.

This problem is observed in the trunk (as of 2005/08/09), and it's causing some test failures
in the gump build of the said javaflow project.

Here's what I found while tracing through the code.

When LocalVariableGen is created by using the following constructor:

{code}
>  public LocalVariableGen(int index, String name, Type type,
>                          InstructionHandle start, InstructionHandle end) {
>    if((index < 0) || (index > Constants.MAX_SHORT))
>      throw new ClassGenException("Invalid index index: " + index);
>    
>    this.name  = name;
>    this.type  = type;
>    this.index  = index;
>    setStart(start);
>    setEnd(end);
>  }
{code}

During the setStart method, it registers this newly created LocalVariableGen as a targeter
to the InstructionHandle given by the 'start' parameter. InstructionHandle.targeters is a
HashSet. So adding a LocalVariableGen as a targter means inserting a LocalVariableGen into
a HashSet. This involves in computing the hashCode.

So from within the setStart method, LocalVariableGen.hashCode() is invoked, since the end
instruction isn't set yet, this returns a hashCode based on just index and start, ignoring
the end field.

Then immediately after the setStart method returns, the above constructor now sets the end
instruction. As a result of this, hash code of the LocalVariableGen changes. This makes it
impossible to update the start of LocalVariableGen correctly, because LocalVariableGen can
no longer remove itself from the targeter set of the previous 'start' instruction.

In other words, the problem is that the LocalVariableGen class computes its hash code based
on mutable fields, yet at the same time it is used in a HashSet, where objects are not allowed
to change its hash code. To the eyes of a new comer like me, this seems like a rather fundamental
design flaw in the current BCEL library.



SUGGESTED FIX
=============
implement the hashCode method to return a constant value would fix the problem at the expense
of the performance:
{code}
  public int hashCode() { 
    return 42;
  }
{code}
Alternatively, instead of using HashSet, use a custom implementation of a Set where the identity
hash code is used instead of Object.hashCode. Although I haven't tested this approach, I believe
this also fixes the problem.

There may be other ways to fix the problem.

  was:
I'm one of the developers working on the Jakarta commons sandbox javaflow project.

This problem is observed in the trunk (as of 2005/08/09), and it's causing some
test failures in the gump build of the said javaflow project.

Here's what I found while tracing through the code.

When LocalVariableGen is created by using the following constructor:


>  public LocalVariableGen(int index, String name, Type type,
>                          InstructionHandle start, InstructionHandle end) {
>    if((index < 0) || (index > Constants.MAX_SHORT))
>      throw new ClassGenException("Invalid index index: " + index);
>    
>    this.name  = name;
>    this.type  = type;
>    this.index  = index;
>    setStart(start);
>    setEnd(end);
>  }


During the setStart method, it registers this newly created LocalVariableGen as
a targeter to the InstructionHandle given by the 'start' parameter.
InstructionHandle.targeters is a HashSet. So adding a LocalVariableGen as a
targter means inserting a LocalVariableGen into a HashSet. This involves in
computing the hashCode.

So from within the setStart method, LocalVariableGen.hashCode() is invoked,
since the end instruction isn't set yet, this returns a hashCode based on just
index and start, ignoring the end field.

Then immediately after the setStart method returns, the above constructor now
sets the end instruction. As a result of this, hash code of the LocalVariableGen
changes. This makes it impossible to update the start of LocalVariableGen
correctly, because LocalVariableGen can no longer remove itself from the
targeter set of the previous 'start' instruction.

In other words, the problem is that the LocalVariableGen class computes its hash
code based on mutable fields, yet at the same time it is used in a HashSet,
where objects are not allowed to change its hash code. To the eyes of a new
comer like me, this seems like a rather fundamental design flaw in the current
BCEL library.



SUGGESTED FIX
=============
implement the hashCode method to return a constant value would fix the problem
at the expense of the performance:

  public int hashCode() { 
    return 42;
  }

Alternatively, instead of using HashSet, use a custom implementation of a Set
where the identity hash code is used instead of Object.hashCode. Although I
haven't tested this approach, I believe this also fixes the problem.

There may be other ways to fix the problem.

             Priority: Major
          Environment:     (was: Operating System: other
Platform: Other)
    Affects Version/s:     (was: unspecified)
        Fix Version/s: 5.2
             Priority:   (was: P2)
             Severity:   (was: normal)

> A bug in LocalVariableGen
> -------------------------
>
>                 Key: BCEL-69
>                 URL: https://issues.apache.org/jira/browse/BCEL-69
>             Project: Commons BCEL
>          Issue Type: Bug
>          Components: Main
>            Reporter: Kohsuke Kawaguchi
>            Assignee: Apache Commons Developers
>             Fix For: 5.2
>
>
> I'm one of the developers working on the Jakarta commons sandbox javaflow project.
> This problem is observed in the trunk (as of 2005/08/09), and it's causing some test
failures in the gump build of the said javaflow project.
> Here's what I found while tracing through the code.
> When LocalVariableGen is created by using the following constructor:
> {code}
> >  public LocalVariableGen(int index, String name, Type type,
> >                          InstructionHandle start, InstructionHandle end) {
> >    if((index < 0) || (index > Constants.MAX_SHORT))
> >      throw new ClassGenException("Invalid index index: " + index);
> >    
> >    this.name  = name;
> >    this.type  = type;
> >    this.index  = index;
> >    setStart(start);
> >    setEnd(end);
> >  }
> {code}
> During the setStart method, it registers this newly created LocalVariableGen as a targeter
to the InstructionHandle given by the 'start' parameter. InstructionHandle.targeters is a
HashSet. So adding a LocalVariableGen as a targter means inserting a LocalVariableGen into
a HashSet. This involves in computing the hashCode.
> So from within the setStart method, LocalVariableGen.hashCode() is invoked, since the
end instruction isn't set yet, this returns a hashCode based on just index and start, ignoring
the end field.
> Then immediately after the setStart method returns, the above constructor now sets the
end instruction. As a result of this, hash code of the LocalVariableGen changes. This makes
it impossible to update the start of LocalVariableGen correctly, because LocalVariableGen
can no longer remove itself from the targeter set of the previous 'start' instruction.
> In other words, the problem is that the LocalVariableGen class computes its hash code
based on mutable fields, yet at the same time it is used in a HashSet, where objects are not
allowed to change its hash code. To the eyes of a new comer like me, this seems like a rather
fundamental design flaw in the current BCEL library.
> SUGGESTED FIX
> =============
> implement the hashCode method to return a constant value would fix the problem at the
expense of the performance:
> {code}
>   public int hashCode() { 
>     return 42;
>   }
> {code}
> Alternatively, instead of using HashSet, use a custom implementation of a Set where the
identity hash code is used instead of Object.hashCode. Although I haven't tested this approach,
I believe this also fixes the problem.
> There may be other ways to fix the problem.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message