commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sebb (JIRA)" <>
Subject [jira] [Commented] (BCEL-127) InstructionFactory, design issue, createDUP returns static reference
Date Thu, 20 Aug 2015 11:29:45 GMT


Sebb commented on BCEL-127:

BCEL-127 Document that Instruction Factory returns singleton instances


> InstructionFactory, design issue, createDUP returns static reference
> --------------------------------------------------------------------
>                 Key: BCEL-127
>                 URL:
>             Project: Commons BCEL
>          Issue Type: Bug
>          Components: Main
>    Affects Versions: 5.2
>         Environment: Operating System: Linux
> Platform: PC
>            Reporter: zagi
>            Assignee: Apache Commons Developers
> 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

This message was sent by Atlassian JIRA

View raw message