https://issues.apache.org/bugzilla/show_bug.cgi?id=46254
Summary: InstructionFactory, design issue, createDUP returns
static reference
Product: BCEL
Version: 5.2
Platform: PC
OS/Version: Linux
Status: NEW
Severity: normal
Priority: P5
Component: Main
AssignedTo: bcel-dev@jakarta.apache.org
ReportedBy: [EMAIL PROTECTED]
The implementation of the creation of some instructions e.g. DUPs / NOPs is
really misleading. If you create an invokeinstruction or an constant like ldc a
new instruction object is created. Pretty straight forward! But if you create a
DUP instruction you always will get the same object back. This is of course is
great for saving memory, but in the following example it leads to lots of
problems.
Imagine you instrument a code sequence and add your own instructions one after
the other. You add invokestatic instructionsbut also the dups. At the end you
iterate again over the whole list to add some missing instructions ( doing it
in the first iteration is not possible or too much work, due to the semantics
of the algorithm). If you add new instructions again it will not work properly
if your insertion point is a DUP! The new instruction will be inserted at the
first match of the list.
e.g.
i1,... are instructions other than dup,
dup_ are DUP instructions created with InstructionFactory.createDUP();
you iterate from i1 to i6;
init: i1, i2, i3, dup_1, i4, i5, dup_2, i6
1.) insert(i3, new_i)
result: i1, i2, new_i, i3, dup_1, i4, i5, dup_2, i6
2.) insert(dup_1, new_i2)
result: i1, i2, new_i, i3, new_i2, dup_1, i4, i5, dup_2, i6
3.) insert(dup_2, new_i3)
result: i1, i2, new_i, i3, new_i2, new_i3, dup_1, i4, i5, dup_2, i6
The reason for this is that dup_1 dup_2 point to the same static DUP instance
(there is always one). Your call to insert()
calls a method findInstruction() and this method only compares references (==)
and returns the first match.
Even if you use copy() you get the same reference! To omit this problem you
have to create an instance without the factory by using a constructor.
My point is, you can implement it this way BUT you have to mention this in the
API doc that this createDUP method does not behave like a createInvoke method
and always returns the same reference. I know usually the code is the
documentation but I think a lot of pain could be spared with a tiny little
sentence in the doc.
My suggestions for the doc of those methods:
"Returns always a static reference. Note: may cause issues when used as
insertion points"
or something like that
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
