Re: Re: Re: Analyzer does not use stack map frames

2010/11/3 Marcin Rzeźnicki <[email protected]>:
> On Wed, Nov 3, 2010 at 5:31 PM, Eugene Kuleshov <[email protected]> wrote:

>>  Have you studied a corresponding chapter in the ASMs User Guide book?
>>  Basically the intent is that you can implement/extend the
>> Interpreter, leaving the Analyzer unchanged. There is some examples in
>> the mentioned book.
>
> Yes, but in fact you cannot do that. Just take a look at my example
> from the first mail. You cannot determine correctly type of the new
> value in merge. This design'd work truly great if either interpreter
> would have been given interface to do anything it needs or Analyzer
> would have been "good" enough to handle all cases. Here both are not:
> Interpreter cannot mask deficiencies of analysis algorithm because it
> even does not know whenever frame is encountered and merge method does
> not specify where it is being taken (if it had you could have looked
> it up yourself), Analyzer simply ignores this, and in practice is not
> extendable.So in practice it just cannot perform correct analysis in
> all cases, no matter what the intended pattern is. Could you give any
> advice on how to handle this? Many thanks.
>

Hi
I managed to add proper stack map handling to Analyzer - unfortunately
it is not doable via extending this class - mainly because analyze
method constructs Frame array which holds deduced frames so one cannot
prepare frames from stack maps in advance (analyze would throw it
away). I thought I could share this piece of code, maybe someone will
find it useful. The basic idea is to loop through the code before
"real' analysis takes place and prepare all these frames for which
stack map is known. After this has been done analysis will take care
of merging. I also found it useful to propagate precomputed frames
backward to label nodes before an instance of FrameNode (it seems
logical to me, after all - stack map frames are mostly placed after
some label - be it a jump target or exception handler). And one more
thing, in order to properly analyze frames I added private field
called stackMapTop which marks where stack map frame data ends (it is
needed for F-CHOP and F_APPEND frames, otherwise algorithm would not
know where to chop or append). This field is first set after original
code computes initial Frame from method arguments;

        for (int i = 0; i < args.length; ++i) {
            current.setLocal(local++, interpreter.newValue(args[i]));
            if (args[i].getSize() == 2) {
                current.setLocal(local++, interpreter.newValue(null));
            }
        }
        // addition: set stack map top
        stackMapTop = local;
        while (local < m.maxLocals) {
            current.setLocal(local++, interpreter.newValue(null));
        }

then I added this loop:

       // addition: iterate through all instructions and set frames for all
        // stack map frames (and for neighbour labels if there are any)
        Frame stackmapFrame = current;
        for (int ic = 0; ic < n; ic++) {
            final AbstractInsnNode insn = insns.get(ic);
            if (insn.getType() == AbstractInsnNode.FRAME) {
                stackmapFrame = newFrame((FrameNode) insn, stackmapFrame);
                merge(ic, stackmapFrame);
                int j = 1;
                AbstractInsnNode prevInsn = insn.getPrevious();
                while (prevInsn != null && prevInsn.getOpcode() == -1) {
                    prevInsn = prevInsn.getPrevious();
                    frames[ic - j] = stackmapFrame;
                    j += 1;
                }
            }
        }
        merge(0, current , null);

newFrame(FrameNode, Frame) is the new overload of newFrame which does the job;

   private Frame newFrame(FrameNode node, Frame previousFrame) {
        final Frame result = newFrame(previousFrame);
        switch (node.type) {
            case F_APPEND:
                setFrameLocals(result, node.local);
                result.clearStack();
            break;
            case F_CHOP:
                int newStackMapTop = stackMapTop - node.local.size();
                for (int i = newStackMapTop; i < stackMapTop; i++) {
                    result.setLocal(i, interpreter.newValue(null));
                }
                stackMapTop = newStackMapTop;
                result.clearStack();
            break;
            case F_FULL:
                stackMapTop = 0;
                setFrameLocals(result, node.local);
                int maxLocals = result.getLocals();
                for (int i = stackMapTop; i < maxLocals; i++) {
                    result.setLocal(i, interpreter.newValue(null));
                }
                List<Object> stack = node.stack;
                int nStack = stack.size();
                for (int i = 0; i < nStack; i++) {

result.push(interpreter.newValue(getTypeFromFrameOpcode(stack.get(i))));
                }
            break;
            case F_SAME:
                result.clearStack();
            break;
            case F_SAME1:
                result.clearStack();

result.push(interpreter.newValue(getTypeFromFrameOpcode(node.stack.get(0))));
            break;
            default:
                throw new CoroutineGenerationException("Expanded
frames not allowed");
        }
        return result;
    }

setFrameLocals and getTypeFromFrameOpcode are just helpers;

    private void setFrameLocals(final Frame frame, List<Object> locals) {
        int nLocals = locals.size();
        for (int i = 0; i < nLocals; i++) {
            Value newValue =
interpreter.newValue(getTypeFromFrameOpcode(locals.get(i)));
            frame.setLocal(stackMapTop, newValue);
            if (newValue.getSize() == 2) {
                frame.setLocal(stackMapTop + 1, interpreter.newValue(null));
                stackMapTop += 2;
            } else {
                stackMapTop += 1;
            }
        }
    }

    private static Type getTypeFromFrameOpcode(Object opcode) {
        if (opcode instanceof String) {
            return Type.getObjectType((String) opcode);
        }
        if (opcode == INTEGER) {
            return Type.INT_TYPE;
        }
        if (opcode == FLOAT) {
            return Type.FLOAT_TYPE;
        }
        if (opcode == LONG) {
            return Type.LONG_TYPE;
        }
        if (opcode == DOUBLE) {
            return Type.DOUBLE_TYPE;
        }
        if (opcode == NULL) {
            return Type.VOID_TYPE;
        }
        return null;
    }

and that's all - in my tests it seems to work fine. Hope this helps
someone. The main benefit is that this addition sets correctly types
of variables which is not possible in general with current Analyzer,
no matter how hard one would try to achieve this in Interpreter's
subclass. This code of course makes analysis little slower, but I did
not measure how much - I do not expect it to be really bad anyway.



-- 
Pozdrawiam
Marcin Rzeźnicki

-- 
You receive this message as a subscriber of the [email protected] mailing list.
To unsubscribe: mailto:[email protected]
For general help: mailto:[email protected]?subject=help
OW2 mailing lists service home page: http://www.ow2.org/wws