You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Mark Roberts <ma...@cs.washington.edu> on 2015/01/26 19:02:18 UTC
[bcel] PLSE changes to BCEL
I'm currently not familiar with your release process and, hence, not sure what sorts of changes you are willing to consider at this time. I thought I would start with a rough outline of all of our changes and then the group could suggest which ones I should open in JIRA.
We cloned a version of the BCEL sources at Revision 1514334.
We updated to Revision 1646789 (2014-12-19) week of January 19-23, 2015.
Here is a list of files and reasons we differ from standard BCEL:
(The 'reason' codes in left hand column are enumerated below the file list.)
C /bcel/classfile/AnnotationDefault.java
I /bcel/classfile/AnnotationEntry.java
D,I /bcel/classfile/Attribute.java
I /bcel/classfile/AttributeReader.java
BM,JD /bcel/classfile/BootstrapMethod.java
BM,JD /bcel/classfile/BootstrapMethods.java
I /bcel/classfile/ClassParser.java
I /bcel/classfile/Code.java
BM,ID,TS /bcel/classfile/ConstantCP.java
ID /bcel/classfile/ConstantInvokeDynamic.java
P /bcel/classfile/Constant.java
T /bcel/classfile/ConstantLong.java
BM,ID,TS /bcel/classfile/ConstantPool.java
BM,ID /bcel/classfile/DescendingVisitor.java
BM,ID /bcel/classfile/EmptyVisitor.java
C /bcel/classfile/EnclosingMethod.java
I /bcel/classfile/Field.java
I /bcel/classfile/FieldOrMethod.java
TS /bcel/classfile/InnerClasses.java
TS /bcel/classfile/InnerClass.java
T /bcel/classfile/JavaClass.java
JD /bcel/classfile/LineNumber.java
JD /bcel/classfile/LineNumberTable.java
TS /bcel/classfile/LocalVariable.java
TS /bcel/classfile/LocalVariableTypeTable.java
I /bcel/classfile/Method.java
T /bcel/classfile/RuntimeInvisibleParameterAnnotations.java
T /bcel/classfile/RuntimeVisibleParameterAnnotations.java
TS /bcel/classfile/SourceFile.java
S,I /bcel/classfile/StackMapTableEntry.java
I /bcel/classfile/StackMapTable.java
V,I,ID /bcel/classfile/Utility.java
BM,ID /bcel/classfile/Visitor.java
T /bcel/Constants.java
TS /bcel/generic/BranchInstruction.java
ID /bcel/generic/ConstantPoolGen.java
ID /bcel/generic/EmptyVisitor.java
P,ID /bcel/generic/FieldOrMethod.java
ID /bcel/generic/GETFIELD.java
ID /bcel/generic/GETSTATIC.java
ID /bcel/generic/InstructionFactory.java
P,U /bcel/generic/Instruction.java
ID /bcel/generic/INVOKEDYNAMIC.java
ID /bcel/generic/INVOKEINTERFACE.java
ID /bcel/generic/INVOKESPECIAL.java
ID /bcel/generic/INVOKESTATIC.java
ID /bcel/generic/INVOKEVIRTUAL.java
ID /bcel/generic/InvokeInstruction.java
D /bcel/generic/LineNumberGen.java
H /bcel/generic/LocalVariableGen.java
B,LV,TS /bcel/generic/MethodGen.java
ID /bcel/generic/PUTFIELD.java
ID /bcel/generic/PUTSTATIC.java
ID /bcel/generic/Visitor.java
M /bcel/PLSEMarker.java (new file)
P,TS /bcel/util/BCELifier.java
ID /bcel/util/CodeHTML.java
ID /bcel/util/InstructionFinder.java
D /bcel/util/SyntheticRepository.java
D /bcel/verifier/statics/Pass1Verifier.java
ID /bcel/verifier/statics/Pass3aVerifier.java
ID /bcel/verifier/structurals/ExecutionVisitor.java
P /bcel/verifier/structurals/Frame.java
D,ID,UT /bcel/verifier/structurals/InstConstraintVisitor.java
C,P,ML /bcel/verifier/structurals/LocalVariables.java
P /bcel/verifier/structurals/OperandStack.java
RS /bcel/verifier/structurals/Subroutines.java
B - BCEL bug #39695 (and #44083) (now BCEL-79)
BM - add support for BootstrapMethod(s)
C - better support for clone/copy methods
D - add some debugging code
H - fix problem with hash function
I - finish/complete change from DataInputStream to DataInput
ID - add/improve support for InvokeDynamic
JD - javadoc additions
LV - correct problem with missing local vars to anonymous init methods
M - marked class to identify our version of BCEL
ML - deal with superclass of type Object in merge of local variables
P - make some methods public (or not deprecated) for our use
RS - reduce JustIce restrictions on subroutines
S - improve StackMap support
T - fix typo, source formatting or cut/paste error
TS - improve toString or other output formatting
U - we need instructions to be unique
UT - allow unitialized types in PUTFIELD (can happen in a constructor)
V - add support for TypeVariable Signature
A few additional thoughts:
You will definitely not want change 'M', probably not change 'P' either.
I was surprised by 'U' as that change had a huge side effect, possibly due to a design flaw in how Targeters work; I was unable to find any discussion in your archives.
During the past year, both the Apache Commons BCEL team and our group here at UW independently completed the support for InvokeDynamic . Some things we chose to do the same way, but there are a few items that are quite different. Trying not to be too biased, I believe our method is better. One of the larger differences is your decision to add the abstract class NameSignatureMethod between FieldOrMethod and CPInstruction and then have InvokeDynamic extend from that instead of from InvokeInstruction. To me this seems wrong. In addition to forcing InvokeDynamic to duplicate all the methods from InvokeInstruction and FieldOrMethod, it is awkward that it is the only form of the Invoke Instruction that doesn't derive from InvokeInstruction. (‘ID’ and ‘BM’ as well.)
‘B’, ‘H’, and ‘LV’ are important bug fixes.
I not sure why you chose not to complete the change from DataInputStream to DataInput (‘I’). This was another item we did here at PLSE independent of your work. Going all the way is nice, because tools that use BCEL can then backup and reprocess the input class file.
A lot of the ‘TS’ changes were to make the BCEL output look more like the ‘javap’ output; you may not care, but this was useful to some of our clients.
Sorry for length of post and amount of information. We can divide into separate threads If you wish.
Mark
-----Original Message-----
From: Emmanuel Bourg [mailto:emmanuel.bourg@gmail.com] On Behalf Of Emmanuel Bourg
Sent: Friday, January 23, 2015 2:11 PM
To: Commons Developers List
Subject: Re: [bcel] new to list - introduction
Le 23/01/2015 20:55, Mark Roberts a écrit :
> My name is Mark Roberts and I work in the Programming Languages and
> Software Engineering group (PLSE) at the University of Washington.
> (http://www.cs.washington.edu/research/plse)
>
> Our team develops and supports a number of open source tools. One of the main ones, Daikon (http://plse.cs.washington.edu/daikon/) makes extensive use of BCEL to manipulate Java class files. We have been using BCEL since 2001 and have made a few changes along the way. I have just completed re-merging our BCEL source tree with a fairly recent mainline version (r1646789 2014-12-19).
>
> I have exchanged email with Torsten Curdt and he suggested I join this mailing list as a first step in sharing our changes and fixes.
Hi Mark and welcome to Apache Commons. We are about to release BCEL 6.0 but it's still time to commit some minor changes. I plan to merge the recent verifier improvement contributed by Jérôme Leroux and cut a new release candidate shortly after.
What kind of improvements would you like to contribute? We can start discussing them on the list and then open related issues in JIRA.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Ben McCann <be...@benmccann.com>.
Dave, we'd still appreciate your comment on BCEL-209 if you can take a look:
https://issues.apache.org/jira/browse/BCEL-209
Thanks,
Ben
On Mon, Feb 23, 2015 at 5:51 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 23/02/2015 14:11, Dave Brosius a écrit :
>
> > One specific case is that annotations are now coming in as annotations
> > as opposed to unknown code attributes. Code that relies on parsing
> > annotations thru unknown code attributes will break.
>
> Thank you for the insight Dave, I'll add that to the release notes. If
> you have a moment to comment on BCEL-209 that would be really nice since
> you were involved in the implementation of invokedynamic.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
about.me/benmccann
Re: [bcel] PLSE changes to BCEL
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 23/02/2015 14:11, Dave Brosius a écrit :
> One specific case is that annotations are now coming in as annotations
> as opposed to unknown code attributes. Code that relies on parsing
> annotations thru unknown code attributes will break.
Thank you for the insight Dave, I'll add that to the release notes. If
you have a moment to comment on BCEL-209 that would be really nice since
you were involved in the implementation of invokedynamic.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Dave Brosius <db...@mebigfatguy.com>.
6.0 already has breaking changes, thus the move to version 6.0.
One specific case is that annotations are now coming in as annotations
as opposed to unknown code attributes. Code that relies on parsing
annotations thru unknown code attributes will break.
On 02/23/2015 07:30 AM, Ben McCann wrote:
> Ok, thanks. The part I wasn't sure about is if that was the only breaking
> change. It sounds like you're saying the other changes aren't breaking.
>
> -Ben
> On Feb 22, 2015 11:14 PM, "Emmanuel Bourg" <eb...@apache.org> wrote:
>
>> Le 23/02/2015 05:39, Ben McCann a écrit :
>>
>>> Are many of your changes breaking backward compatibility? I'm wondering
>> if
>>> there are only a few that do if we could prioritize them in order to get
>> a
>>> 6.0 release out with the rest of the changes coming in a 6.1.
>> Hi Ben,
>>
>> I think we already answered this question, BCEL-209 is a breaking change
>> because it affects the public API. Your help resolving this issue would
>> be welcome. We have to understand why the current design was chosen and
>> if the design proposed by Mark may cause any issue later.
>>
>> Emmanuel Bourg
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Ben McCann <be...@benmccann.com>.
Ok, thanks. The part I wasn't sure about is if that was the only breaking
change. It sounds like you're saying the other changes aren't breaking.
-Ben
On Feb 22, 2015 11:14 PM, "Emmanuel Bourg" <eb...@apache.org> wrote:
> Le 23/02/2015 05:39, Ben McCann a écrit :
>
> > Are many of your changes breaking backward compatibility? I'm wondering
> if
> > there are only a few that do if we could prioritize them in order to get
> a
> > 6.0 release out with the rest of the changes coming in a 6.1.
>
> Hi Ben,
>
> I think we already answered this question, BCEL-209 is a breaking change
> because it affects the public API. Your help resolving this issue would
> be welcome. We have to understand why the current design was chosen and
> if the design proposed by Mark may cause any issue later.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [bcel] PLSE changes to BCEL
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 23/02/2015 05:39, Ben McCann a écrit :
> Are many of your changes breaking backward compatibility? I'm wondering if
> there are only a few that do if we could prioritize them in order to get a
> 6.0 release out with the rest of the changes coming in a 6.1.
Hi Ben,
I think we already answered this question, BCEL-209 is a breaking change
because it affects the public API. Your help resolving this issue would
be welcome. We have to understand why the current design was chosen and
if the design proposed by Mark may cause any issue later.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Ben McCann <be...@benmccann.com>.
Hi Mark,
Are many of your changes breaking backward compatibility? I'm wondering if
there are only a few that do if we could prioritize them in order to get a
6.0 release out with the rest of the changes coming in a 6.1.
Thanks,
Ben
On Thu, Feb 19, 2015 at 9:06 AM, Mark Roberts <ma...@cs.washington.edu>
wrote:
> I am going to try to spend most of today working on BCEL; I am currently
> looking at 195, 200,203,208 and 210.
>
> Mark
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
about.me/benmccann
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
I am going to try to spend most of today working on BCEL; I am currently looking at 195, 200,203,208 and 210.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 04/02/2015 20:57, Mark Roberts a écrit :
> I would like to reopen BCEL-79, but I'm new to JIRA and I can't figure out how to do it. Reopen does not appear to be one of the choices on the drop down tools menu. Could someone let me know how to do this?
I reopened it, you can go ahead.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
I would like to reopen BCEL-79, but I'm new to JIRA and I can't figure out how to do it. Reopen does not appear to be one of the choices on the drop down tools menu. Could someone let me know how to do this?
Thank you,
Mark Roberts
-----Original Message-----
From: benjamin.j.mccann@gmail.com [mailto:benjamin.j.mccann@gmail.com] On Behalf Of Ben McCann
Sent: Wednesday, February 04, 2015 10:49 AM
To: Commons Developers List
Subject: Re: [bcel] PLSE changes to BCEL
BCEL-79 has not been reopened yet
The new issues have been filed as BCEL-187 <https://issues.apache.org/jira/browse/BCEL-187> through BCEL-193 <https://issues.apache.org/jira/browse/BCEL-193>.
Are any of these blocking the 6.0 release or do you think we might be able to cut that now?
Thanks,
Ben
On Tue, Jan 27, 2015 at 1:58 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 27/01/2015 05:36, Mark Roberts a écrit :
>
> > To repeat - constructing the local variables from the local variable
> types
> > is totally bogus. I have no idea how that fixed the original problem
> but
> > it is clearly incorrect.
>
> Ok, could you reopen BCEL-79 and copy your comment in JIRA please?
> Would you be able to craft a test case demonstrating the issue?
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
about.me/benmccann
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
Thank you for setting these up for me! I am going to attach my diffs today.
I would really like to see this material in 6.0. Most all of the changes are bug fixes or feature enhancements (in the case of StackMap support). They all, in my opinion, make BCEL a better product. And on a personal note, given that this is the first update in forever (nine years?) - if we don't merge now, how much longer will I continue to be out of sync with the mainline?
Thank you,
Mark
-----Original Message-----
From: benjamin.j.mccann@gmail.com [mailto:benjamin.j.mccann@gmail.com] On Behalf Of Ben McCann
Sent: Wednesday, February 04, 2015 10:49 AM
To: Commons Developers List
Subject: Re: [bcel] PLSE changes to BCEL
BCEL-79 has not been reopened yet
The new issues have been filed as BCEL-187 <https://issues.apache.org/jira/browse/BCEL-187> through BCEL-193 <https://issues.apache.org/jira/browse/BCEL-193>.
Are any of these blocking the 6.0 release or do you think we might be able to cut that now?
Thanks,
Ben
On Tue, Jan 27, 2015 at 1:58 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 27/01/2015 05:36, Mark Roberts a écrit :
>
> > To repeat - constructing the local variables from the local variable
> types
> > is totally bogus. I have no idea how that fixed the original problem
> but
> > it is clearly incorrect.
>
> Ok, could you reopen BCEL-79 and copy your comment in JIRA please?
> Would you be able to craft a test case demonstrating the issue?
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
about.me/benmccann
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
Sorry - I clearly misunderstood Ben's email about issues 187-193 (the reference to issue 79 made me think the mail was specific to me).
However, my comments about 6.0 are still valid and I will open new issues for my work immediately.
Thank you, Mark
-----Original Message-----
From: Mark Roberts [mailto:markro@cs.washington.edu]
Sent: Wednesday, February 04, 2015 11:09 AM
To: 'Commons Developers List'
Subject: RE: [bcel] PLSE changes to BCEL
Thank you for setting these up for me! I am going to attach my diffs today.
I would really like to see this material in 6.0. Most all of the changes are bug fixes or feature enhancements (in the case of StackMap support). They all, in my opinion, make BCEL a better product. And on a personal note, given that this is the first update in forever (nine years?) - if we don't merge now, how much longer will I continue to be out of sync with the mainline?
Thank you,
Mark
-----Original Message-----
From: benjamin.j.mccann@gmail.com [mailto:benjamin.j.mccann@gmail.com] On Behalf Of Ben McCann
Sent: Wednesday, February 04, 2015 10:49 AM
To: Commons Developers List
Subject: Re: [bcel] PLSE changes to BCEL
BCEL-79 has not been reopened yet
The new issues have been filed as BCEL-187 <https://issues.apache.org/jira/browse/BCEL-187> through BCEL-193 <https://issues.apache.org/jira/browse/BCEL-193>.
Are any of these blocking the 6.0 release or do you think we might be able to cut that now?
Thanks,
Ben
On Tue, Jan 27, 2015 at 1:58 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 27/01/2015 05:36, Mark Roberts a écrit :
>
> > To repeat - constructing the local variables from the local variable
> types
> > is totally bogus. I have no idea how that fixed the original problem
> but
> > it is clearly incorrect.
>
> Ok, could you reopen BCEL-79 and copy your comment in JIRA please?
> Would you be able to craft a test case demonstrating the issue?
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
about.me/benmccann
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Ben McCann <be...@benmccann.com>.
BCEL-79 has not been reopened yet
The new issues have been filed as BCEL-187
<https://issues.apache.org/jira/browse/BCEL-187> through BCEL-193
<https://issues.apache.org/jira/browse/BCEL-193>.
Are any of these blocking the 6.0 release or do you think we might be able
to cut that now?
Thanks,
Ben
On Tue, Jan 27, 2015 at 1:58 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 27/01/2015 05:36, Mark Roberts a écrit :
>
> > To repeat - constructing the local variables from the local variable
> types
> > is totally bogus. I have no idea how that fixed the original problem
> but
> > it is clearly incorrect.
>
> Ok, could you reopen BCEL-79 and copy your comment in JIRA please? Would
> you be able to craft a test case demonstrating the issue?
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
about.me/benmccann
Re: [bcel] PLSE changes to BCEL
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 27/01/2015 05:36, Mark Roberts a écrit :
> To repeat - constructing the local variables from the local variable types
> is totally bogus. I have no idea how that fixed the original problem but
> it is clearly incorrect.
Ok, could you reopen BCEL-79 and copy your comment in JIRA please? Would
you be able to craft a test case demonstrating the issue?
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
In our opinion, the fix is worse than the disease. Here are the comments
from our version:
The following piece of code is new since our 5.2 based version.
It was added in revision 524610 on 2007-04-01, more information is at:
http://issues.apache.org/bugzilla/show_bug.cgi?id=39695 (now BCEL-79)
However, it seems quite wrong to me and I have no idea how it solved the
bug.
There is no requirement for there to be as many LocalVariableTypeTable
entries
as there are LocalVariables. In fact, there will almost always be fewer.
So
deleting the local variables and reconstructing them from types makes no
sense.
Was it a bad cut and paste job from above and meant to reconstruct the
types
table? Then how did it fix the bug? More likely, the test case had the
same
number of vars and types. If so, the correct code would seem to be only
modify
those local vars that have a matching type. I cannot figure out what was
intended
so I'm just going to comment the whole thing out. (markro)
To repeat - constructing the local variables from the local variable types
is totally bogus. I have no idea how that fixed the original problem but
it is clearly incorrect.
Mark
> -----Original Message-----
> From: Emmanuel Bourg [mailto:emmanuel.bourg@gmail.com] On
> Behalf Of Emmanuel Bourg
>
> Isn't BCEL-79 already fixed?
>
>
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
I've probably missed something - but I think I've got all of our (PLSE)
changes into the jira database.
I'm going to look at the signatureToString issue now.
Thanks to everybody who helped me along the way...
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 27/01/2015 05:02, Mark Roberts a écrit :
> Which takes us to another question I was going to ask - is there a BCEL test
> suite? 'mvn verify' is ok for build verification - but it doesn't begin to
> cover the bases of what the api can do - especially in the area of code
> modification.
You can run the tests with "mvn test", but the coverage is pretty low
and new test cases would be very welcome.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
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.
Which takes us to another question I was going to ask - is there a BCEL test
suite? 'mvn verify' is ok for build verification - but it doesn't begin to
cover the bases of what the api can do - especially in the area of code
modification.
Mark
> -----Original Message-----
> From: Emmanuel Bourg [mailto:emmanuel.bourg@gmail.com] On
> Behalf Of Emmanuel Bourg
> Sent: Monday, January 26, 2015 4:14 PM
>
> > I was surprised by 'U' as that change had a huge side
> effect, possibly due to a design flaw in how Targeters work;
> I was unable to find any discussion in your archives.
>
> Could you elaborate a bit on this change?
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Emmanuel Bourg <eb...@apache.org>.
Thank you for the detailed summary Mark.
Le 26/01/2015 19:02, Mark Roberts a écrit :
> I'm currently not familiar with your release process and, hence, not sure what sorts of changes you are willing to consider at this time. I thought I would start with a rough outline of all of our changes and then the group could suggest which ones I should open in JIRA.
An important rule that drives many design decisions is that we never
break the binary compatibility. And if we really have to, well... we
don't :) Instead we roll out a new version under a new package name.
Considering that we would like to publish BCEL 6.0 soon we will probably
not merge involved changes now, but smalls fixes are welcome.
> We cloned a version of the BCEL sources at Revision 1514334.
> We updated to Revision 1646789 (2014-12-19) week of January 19-23, 2015.
I don't know if you are familiar with Git but BCEL also has a mirror on
Github. You may find it useful to rebase your changes as the trunk evolves.
https://github.com/apache/commons-bcel
> I was surprised by 'U' as that change had a huge side effect, possibly due to a design flaw in how Targeters work; I was unable to find any discussion in your archives.
Could you elaborate a bit on this change?
> During the past year, both the Apache Commons BCEL team and our group here at UW independently completed the support for InvokeDynamic . Some things we chose to do the same way, but there are a few items that are quite different. Trying not to be too biased, I believe our method is better. One of the larger differences is your decision to add the abstract class NameSignatureMethod between FieldOrMethod and CPInstruction and then have InvokeDynamic extend from that instead of from InvokeInstruction. To me this seems wrong. In addition to forcing InvokeDynamic to duplicate all the methods from InvokeInstruction and FieldOrMethod, it is awkward that it is the only form of the Invoke Instruction that doesn't derive from InvokeInstruction. (‘ID’ and ‘BM’ as well.)
Good point, this should be investigated.
> ‘B’, ‘H’, and ‘LV’ are important bug fixes.
Isn't BCEL-79 already fixed?
> I not sure why you chose not to complete the change from DataInputStream to DataInput (‘I’). This was another item we did here at PLSE independent of your work. Going all the way is nice, because tools that use BCEL can then backup and reprocess the input class file.
I stopped because some changes touched the public API. I reviewed that
again today and managed to replace more occurrences, but the one in the
AttributeReader interface can't be changed without breaking the binary
compatibility and this affects some other classes.
> A lot of the ‘TS’ changes were to make the BCEL output look more like the ‘javap’ output; you may not care, but this was useful to some of our clients.
This is a good idea.
> Sorry for length of post and amount of information. We can divide into separate threads If you wish.
That's fine. These changes sound excellent, could you open one JIRA
issue per change please?
Thank you,
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
The InvokeDynamic and BootstrapMethod changes move some class members around. So any app that writes a serialized version of the BCEL internal data structures and re-reads later would have to regenerate those files. Our tools do not do this; I have no idea if this is common. On the other hand, these changes are the largest single set of diffs so if you are close to release this would probably mean starting the test/verification cycle from the top.
Mark
-----Original Message-----
From: Gary Gregory [mailto:garydgregory@gmail.com]
Sent: Monday, January 26, 2015 1:32 PM
To: Commons Developers List
Subject: Re: [bcel] PLSE changes to BCEL
The only caveat here, since we are going for a major version bump is whether you have some proposal for changes that would be binary incompatible with 5.x, which we should therefore consider for 6.0.
Gary
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by Gary Gregory <ga...@gmail.com>.
Mark,
This is awesome info. Thank you for taking the time to compile it.
My preference would be to finalize and cut 6.0 ASAP, then start a new
cycle. I am a RERO proponent.
The only caveat here, since we are going for a major version bump is
whether you have some proposal for changes that would be binary
incompatible with 5.x, which we should therefore consider for 6.0.
Gary
On Mon, Jan 26, 2015 at 1:02 PM, Mark Roberts <ma...@cs.washington.edu>
wrote:
> I'm currently not familiar with your release process and, hence, not sure
> what sorts of changes you are willing to consider at this time. I thought
> I would start with a rough outline of all of our changes and then the group
> could suggest which ones I should open in JIRA.
>
> We cloned a version of the BCEL sources at Revision 1514334.
> We updated to Revision 1646789 (2014-12-19) week of January 19-23, 2015.
>
> Here is a list of files and reasons we differ from standard BCEL:
> (The 'reason' codes in left hand column are enumerated below the file
> list.)
>
> C /bcel/classfile/AnnotationDefault.java
> I /bcel/classfile/AnnotationEntry.java
> D,I /bcel/classfile/Attribute.java
> I /bcel/classfile/AttributeReader.java
> BM,JD /bcel/classfile/BootstrapMethod.java
> BM,JD /bcel/classfile/BootstrapMethods.java
> I /bcel/classfile/ClassParser.java
> I /bcel/classfile/Code.java
> BM,ID,TS /bcel/classfile/ConstantCP.java
> ID /bcel/classfile/ConstantInvokeDynamic.java
> P /bcel/classfile/Constant.java
> T /bcel/classfile/ConstantLong.java
> BM,ID,TS /bcel/classfile/ConstantPool.java
> BM,ID /bcel/classfile/DescendingVisitor.java
> BM,ID /bcel/classfile/EmptyVisitor.java
> C /bcel/classfile/EnclosingMethod.java
> I /bcel/classfile/Field.java
> I /bcel/classfile/FieldOrMethod.java
> TS /bcel/classfile/InnerClasses.java
> TS /bcel/classfile/InnerClass.java
> T /bcel/classfile/JavaClass.java
> JD /bcel/classfile/LineNumber.java
> JD /bcel/classfile/LineNumberTable.java
> TS /bcel/classfile/LocalVariable.java
> TS /bcel/classfile/LocalVariableTypeTable.java
> I /bcel/classfile/Method.java
> T /bcel/classfile/RuntimeInvisibleParameterAnnotations.java
> T /bcel/classfile/RuntimeVisibleParameterAnnotations.java
> TS /bcel/classfile/SourceFile.java
> S,I /bcel/classfile/StackMapTableEntry.java
> I /bcel/classfile/StackMapTable.java
> V,I,ID /bcel/classfile/Utility.java
> BM,ID /bcel/classfile/Visitor.java
> T /bcel/Constants.java
> TS /bcel/generic/BranchInstruction.java
> ID /bcel/generic/ConstantPoolGen.java
> ID /bcel/generic/EmptyVisitor.java
> P,ID /bcel/generic/FieldOrMethod.java
> ID /bcel/generic/GETFIELD.java
> ID /bcel/generic/GETSTATIC.java
> ID /bcel/generic/InstructionFactory.java
> P,U /bcel/generic/Instruction.java
> ID /bcel/generic/INVOKEDYNAMIC.java
> ID /bcel/generic/INVOKEINTERFACE.java
> ID /bcel/generic/INVOKESPECIAL.java
> ID /bcel/generic/INVOKESTATIC.java
> ID /bcel/generic/INVOKEVIRTUAL.java
> ID /bcel/generic/InvokeInstruction.java
> D /bcel/generic/LineNumberGen.java
> H /bcel/generic/LocalVariableGen.java
> B,LV,TS /bcel/generic/MethodGen.java
> ID /bcel/generic/PUTFIELD.java
> ID /bcel/generic/PUTSTATIC.java
> ID /bcel/generic/Visitor.java
> M /bcel/PLSEMarker.java (new file)
> P,TS /bcel/util/BCELifier.java
> ID /bcel/util/CodeHTML.java
> ID /bcel/util/InstructionFinder.java
> D /bcel/util/SyntheticRepository.java
> D /bcel/verifier/statics/Pass1Verifier.java
> ID /bcel/verifier/statics/Pass3aVerifier.java
> ID /bcel/verifier/structurals/ExecutionVisitor.java
> P /bcel/verifier/structurals/Frame.java
> D,ID,UT /bcel/verifier/structurals/InstConstraintVisitor.java
> C,P,ML /bcel/verifier/structurals/LocalVariables.java
> P /bcel/verifier/structurals/OperandStack.java
> RS /bcel/verifier/structurals/Subroutines.java
>
>
> B - BCEL bug #39695 (and #44083) (now BCEL-79)
> BM - add support for BootstrapMethod(s)
> C - better support for clone/copy methods
> D - add some debugging code
> H - fix problem with hash function
> I - finish/complete change from DataInputStream to DataInput
> ID - add/improve support for InvokeDynamic
> JD - javadoc additions
> LV - correct problem with missing local vars to anonymous init methods
> M - marked class to identify our version of BCEL
> ML - deal with superclass of type Object in merge of local variables
> P - make some methods public (or not deprecated) for our use
> RS - reduce JustIce restrictions on subroutines
> S - improve StackMap support
> T - fix typo, source formatting or cut/paste error
> TS - improve toString or other output formatting
> U - we need instructions to be unique
> UT - allow unitialized types in PUTFIELD (can happen in a constructor)
> V - add support for TypeVariable Signature
>
>
> A few additional thoughts:
>
> You will definitely not want change 'M', probably not change 'P' either.
>
> I was surprised by 'U' as that change had a huge side effect, possibly due
> to a design flaw in how Targeters work; I was unable to find any discussion
> in your archives.
>
> During the past year, both the Apache Commons BCEL team and our group here
> at UW independently completed the support for InvokeDynamic . Some things
> we chose to do the same way, but there are a few items that are quite
> different. Trying not to be too biased, I believe our method is better.
> One of the larger differences is your decision to add the abstract class
> NameSignatureMethod between FieldOrMethod and CPInstruction and then have
> InvokeDynamic extend from that instead of from InvokeInstruction. To me
> this seems wrong. In addition to forcing InvokeDynamic to duplicate all
> the methods from InvokeInstruction and FieldOrMethod, it is awkward that it
> is the only form of the Invoke Instruction that doesn't derive from
> InvokeInstruction. (‘ID’ and ‘BM’ as well.)
>
> ‘B’, ‘H’, and ‘LV’ are important bug fixes.
>
> I not sure why you chose not to complete the change from DataInputStream
> to DataInput (‘I’). This was another item we did here at PLSE independent
> of your work. Going all the way is nice, because tools that use BCEL can
> then backup and reprocess the input class file.
>
> A lot of the ‘TS’ changes were to make the BCEL output look more like the
> ‘javap’ output; you may not care, but this was useful to some of our
> clients.
>
> Sorry for length of post and amount of information. We can divide into
> separate threads If you wish.
>
> Mark
>
>
> -----Original Message-----
> From: Emmanuel Bourg [mailto:emmanuel.bourg@gmail.com] On Behalf Of
> Emmanuel Bourg
> Sent: Friday, January 23, 2015 2:11 PM
> To: Commons Developers List
> Subject: Re: [bcel] new to list - introduction
>
> Le 23/01/2015 20:55, Mark Roberts a écrit :
> > My name is Mark Roberts and I work in the Programming Languages and
> > Software Engineering group (PLSE) at the University of Washington.
> > (http://www.cs.washington.edu/research/plse)
> >
> > Our team develops and supports a number of open source tools. One of
> the main ones, Daikon (http://plse.cs.washington.edu/daikon/) makes
> extensive use of BCEL to manipulate Java class files. We have been using
> BCEL since 2001 and have made a few changes along the way. I have just
> completed re-merging our BCEL source tree with a fairly recent mainline
> version (r1646789 2014-12-19).
> >
> > I have exchanged email with Torsten Curdt and he suggested I join this
> mailing list as a first step in sharing our changes and fixes.
>
> Hi Mark and welcome to Apache Commons. We are about to release BCEL 6.0
> but it's still time to commit some minor changes. I plan to merge the
> recent verifier improvement contributed by Jérôme Leroux and cut a new
> release candidate shortly after.
>
> What kind of improvements would you like to contribute? We can start
> discussing them on the list and then open related issues in JIRA.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
RE: [bcel] PLSE changes to BCEL
Posted by Mark Roberts <ma...@cs.washington.edu>.
Good question. When I started work here (at PLSE) in Jan 2013, we had been using BCEL for some time. We had not released a version of our product in about three years and one of my tasks was to get back to making regular releases of the Daikon toolset. So in about May of 2013 I picked up the latest Revision of BCEL. I got it working and noticed we had a few existing differences from the mainline that had apparently been there for quite some time. As time went on, I found and fixed a couple of problems with BCEL. I noticed that after several months there had been no activity on the product at all. I sent email to one of the developers and was told that the project was in limbo and 5.2 might be the last release. I promised to send what changes I had, but never got around to it. Over the next year I noticed a few checkins, but no signs of a release. Recently, I was encouraged by my supervisor (Professor Michael Ernst) to make a better effort at sharing our work. I was quite taken by surprise by the amount of work in the last 6 months or so on the project. So I got the latest version of BCEL, went through the integration process and here we are.
So, if the intention is to start making regular releases of BCEL again, I'm all for it and more than willing to help.
Mark
-----Original Message-----
From: jcarman@carmanconsulting.com [mailto:jcarman@carmanconsulting.com] On Behalf Of James Carman
Sent: Monday, January 26, 2015 10:20 AM
To: Commons Developers List
Subject: Re: [bcel] PLSE changes to BCEL
Out of curiosity, what was the reason you rolled your own as opposed to engaging with the community on these changes?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [bcel] PLSE changes to BCEL
Posted by James Carman <ja...@carmanconsulting.com>.
Attaching a patch (or pull request) to a JIRA would be a great way for
one of us to take a look at what you have. This is great stuff, man!
We are always glad to have new folks come in and provide patches.
Out of curiosity, what was the reason you rolled your own as opposed
to engaging with the community on these changes?
On Mon, Jan 26, 2015 at 1:02 PM, Mark Roberts <ma...@cs.washington.edu> wrote:
> I'm currently not familiar with your release process and, hence, not sure what sorts of changes you are willing to consider at this time. I thought I would start with a rough outline of all of our changes and then the group could suggest which ones I should open in JIRA.
>
> We cloned a version of the BCEL sources at Revision 1514334.
> We updated to Revision 1646789 (2014-12-19) week of January 19-23, 2015.
>
> Here is a list of files and reasons we differ from standard BCEL:
> (The 'reason' codes in left hand column are enumerated below the file list.)
>
> C /bcel/classfile/AnnotationDefault.java
> I /bcel/classfile/AnnotationEntry.java
> D,I /bcel/classfile/Attribute.java
> I /bcel/classfile/AttributeReader.java
> BM,JD /bcel/classfile/BootstrapMethod.java
> BM,JD /bcel/classfile/BootstrapMethods.java
> I /bcel/classfile/ClassParser.java
> I /bcel/classfile/Code.java
> BM,ID,TS /bcel/classfile/ConstantCP.java
> ID /bcel/classfile/ConstantInvokeDynamic.java
> P /bcel/classfile/Constant.java
> T /bcel/classfile/ConstantLong.java
> BM,ID,TS /bcel/classfile/ConstantPool.java
> BM,ID /bcel/classfile/DescendingVisitor.java
> BM,ID /bcel/classfile/EmptyVisitor.java
> C /bcel/classfile/EnclosingMethod.java
> I /bcel/classfile/Field.java
> I /bcel/classfile/FieldOrMethod.java
> TS /bcel/classfile/InnerClasses.java
> TS /bcel/classfile/InnerClass.java
> T /bcel/classfile/JavaClass.java
> JD /bcel/classfile/LineNumber.java
> JD /bcel/classfile/LineNumberTable.java
> TS /bcel/classfile/LocalVariable.java
> TS /bcel/classfile/LocalVariableTypeTable.java
> I /bcel/classfile/Method.java
> T /bcel/classfile/RuntimeInvisibleParameterAnnotations.java
> T /bcel/classfile/RuntimeVisibleParameterAnnotations.java
> TS /bcel/classfile/SourceFile.java
> S,I /bcel/classfile/StackMapTableEntry.java
> I /bcel/classfile/StackMapTable.java
> V,I,ID /bcel/classfile/Utility.java
> BM,ID /bcel/classfile/Visitor.java
> T /bcel/Constants.java
> TS /bcel/generic/BranchInstruction.java
> ID /bcel/generic/ConstantPoolGen.java
> ID /bcel/generic/EmptyVisitor.java
> P,ID /bcel/generic/FieldOrMethod.java
> ID /bcel/generic/GETFIELD.java
> ID /bcel/generic/GETSTATIC.java
> ID /bcel/generic/InstructionFactory.java
> P,U /bcel/generic/Instruction.java
> ID /bcel/generic/INVOKEDYNAMIC.java
> ID /bcel/generic/INVOKEINTERFACE.java
> ID /bcel/generic/INVOKESPECIAL.java
> ID /bcel/generic/INVOKESTATIC.java
> ID /bcel/generic/INVOKEVIRTUAL.java
> ID /bcel/generic/InvokeInstruction.java
> D /bcel/generic/LineNumberGen.java
> H /bcel/generic/LocalVariableGen.java
> B,LV,TS /bcel/generic/MethodGen.java
> ID /bcel/generic/PUTFIELD.java
> ID /bcel/generic/PUTSTATIC.java
> ID /bcel/generic/Visitor.java
> M /bcel/PLSEMarker.java (new file)
> P,TS /bcel/util/BCELifier.java
> ID /bcel/util/CodeHTML.java
> ID /bcel/util/InstructionFinder.java
> D /bcel/util/SyntheticRepository.java
> D /bcel/verifier/statics/Pass1Verifier.java
> ID /bcel/verifier/statics/Pass3aVerifier.java
> ID /bcel/verifier/structurals/ExecutionVisitor.java
> P /bcel/verifier/structurals/Frame.java
> D,ID,UT /bcel/verifier/structurals/InstConstraintVisitor.java
> C,P,ML /bcel/verifier/structurals/LocalVariables.java
> P /bcel/verifier/structurals/OperandStack.java
> RS /bcel/verifier/structurals/Subroutines.java
>
>
> B - BCEL bug #39695 (and #44083) (now BCEL-79)
> BM - add support for BootstrapMethod(s)
> C - better support for clone/copy methods
> D - add some debugging code
> H - fix problem with hash function
> I - finish/complete change from DataInputStream to DataInput
> ID - add/improve support for InvokeDynamic
> JD - javadoc additions
> LV - correct problem with missing local vars to anonymous init methods
> M - marked class to identify our version of BCEL
> ML - deal with superclass of type Object in merge of local variables
> P - make some methods public (or not deprecated) for our use
> RS - reduce JustIce restrictions on subroutines
> S - improve StackMap support
> T - fix typo, source formatting or cut/paste error
> TS - improve toString or other output formatting
> U - we need instructions to be unique
> UT - allow unitialized types in PUTFIELD (can happen in a constructor)
> V - add support for TypeVariable Signature
>
>
> A few additional thoughts:
>
> You will definitely not want change 'M', probably not change 'P' either.
>
> I was surprised by 'U' as that change had a huge side effect, possibly due to a design flaw in how Targeters work; I was unable to find any discussion in your archives.
>
> During the past year, both the Apache Commons BCEL team and our group here at UW independently completed the support for InvokeDynamic . Some things we chose to do the same way, but there are a few items that are quite different. Trying not to be too biased, I believe our method is better. One of the larger differences is your decision to add the abstract class NameSignatureMethod between FieldOrMethod and CPInstruction and then have InvokeDynamic extend from that instead of from InvokeInstruction. To me this seems wrong. In addition to forcing InvokeDynamic to duplicate all the methods from InvokeInstruction and FieldOrMethod, it is awkward that it is the only form of the Invoke Instruction that doesn't derive from InvokeInstruction. (‘ID’ and ‘BM’ as well.)
>
> ‘B’, ‘H’, and ‘LV’ are important bug fixes.
>
> I not sure why you chose not to complete the change from DataInputStream to DataInput (‘I’). This was another item we did here at PLSE independent of your work. Going all the way is nice, because tools that use BCEL can then backup and reprocess the input class file.
>
> A lot of the ‘TS’ changes were to make the BCEL output look more like the ‘javap’ output; you may not care, but this was useful to some of our clients.
>
> Sorry for length of post and amount of information. We can divide into separate threads If you wish.
>
> Mark
>
>
> -----Original Message-----
> From: Emmanuel Bourg [mailto:emmanuel.bourg@gmail.com] On Behalf Of Emmanuel Bourg
> Sent: Friday, January 23, 2015 2:11 PM
> To: Commons Developers List
> Subject: Re: [bcel] new to list - introduction
>
> Le 23/01/2015 20:55, Mark Roberts a écrit :
>> My name is Mark Roberts and I work in the Programming Languages and
>> Software Engineering group (PLSE) at the University of Washington.
>> (http://www.cs.washington.edu/research/plse)
>>
>> Our team develops and supports a number of open source tools. One of the main ones, Daikon (http://plse.cs.washington.edu/daikon/) makes extensive use of BCEL to manipulate Java class files. We have been using BCEL since 2001 and have made a few changes along the way. I have just completed re-merging our BCEL source tree with a fairly recent mainline version (r1646789 2014-12-19).
>>
>> I have exchanged email with Torsten Curdt and he suggested I join this mailing list as a first step in sharing our changes and fixes.
>
> Hi Mark and welcome to Apache Commons. We are about to release BCEL 6.0 but it's still time to commit some minor changes. I plan to merge the recent verifier improvement contributed by Jérôme Leroux and cut a new release candidate shortly after.
>
> What kind of improvements would you like to contribute? We can start discussing them on the list and then open related issues in JIRA.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org