Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 353936

Summary:

LocalVariableTable is incorrect for methods that are advised by 'before' advice

Product:

[Tools] AspectJ

Reporter:

yoav <yoav_>

Component:

Compiler

Assignee:

aspectj inbox <aspectj-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

aclement, yoav_

Version:

1.6.12

Target Milestone:

1.6.12

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
patch for an AspectJ class	none

Description yoav

2011-08-04 15:20:13 EDT

Build Identifier: 20110301-1815

The 'this' variable in the line number table is undefined in the first instructions of the advised method. This can confuse the Java debugging I/S.

I suspect the problem is BcelAdvice.getAdviceInstructions. At the end of 
this method we go over this table and fix any variables that start at 
position 0:

// Fix up the local variables: find any that have a startPC of 0 and ensure 
they target the new start of the method
LocalVariableTable lvt = 
shadow.getEnclosingMethod().getMemberView().getMethod().getLocalVariableTable();
if (lvt != null) {
    LocalVariable[] lvTable = lvt.getLocalVariableTable();
    for (int i = 0; i < lvTable.length; i++) {
        LocalVariable lv = lvTable[i];
        if (lv.getStartPC() == 0) {
            start.addTargeter(new LocalVariableTag(lv.getSignature(), 
lv.getName(), lv.getIndex(), 0));
        }
    }

perhaps we can skip here the 'this' variable?


Reproducible: Always

Steps to Reproduce:
1. advise a method with a before advice
2. At some point when examining a stack frame inside an advice, I am trying to get a mirror to the ‘this’ object using StackFrame.thisObject(). I get an exception: 
Exception in thread "active event system driver" com.sun.jdi.InternalException: Unexpected JDWP Error: 35
    at com.sun.tools.jdi.JDWPException.toJDIException(JDWPException.java:47)
    at com.sun.tools.jdi.StackFrameImpl.thisObject(StackFrameImpl.java:127)

Comment 1 Andrew Clement

2011-08-04 15:24:08 EDT

Created attachment 200942 [details]
patch for an AspectJ class

patch for weaver LazyMethodGen

Comment 2 Andrew Clement

2011-08-05 12:32:01 EDT

changes are in, with testcase, also extended their range so parameter names range until the end of the method.

Comment 3 yoav

2011-08-05 12:33:41 EDT

(In reply to comment #2)
> changes are in, with testcase, also extended their range so parameter names
> range until the end of the method.

could you post the final patch?

Comment 4 Andrew Clement

2011-08-05 12:41:42 EDT

not easily now I've committed it.  Here is the complete method now:

private void addLocalVariables(MethodGen gen, Map<LocalVariableTag, LVPosition> localVariables) {
		// now add local variables
		gen.removeLocalVariables();

		// this next iteration _might_ be overkill, but we had problems with
		// bcel before with duplicate local variables. Now that we're patching
		// bcel we should be able to do without it if we're paranoid enough
		// through the rest of the compiler.
		InstructionHandle methodStart = gen.getInstructionList().getStart();
		InstructionHandle methodEnd = gen.getInstructionList().getEnd();

		// Determine how many 'slots' are used by parameters to the method.
		// Then below we can determine if a local variable is a parameter variable, if it is
		// we force its range to from the method start (as it may have been shuffled down
		// due to insertion of advice like cflow entry)
		int paramSlots = gen.isStatic() ? 0 : 1;
		Type[] argTypes = gen.getArgumentTypes();
		if (argTypes!=null) {
			for (int i = 0; i < argTypes.length; i++) {
				if (argTypes[i].getSize() == 2) {
					paramSlots += 2;
				} else {
					paramSlots += 1;
				}
			}
		}

		Map<InstructionHandle, Set<Integer>> duplicatedLocalMap = new HashMap<InstructionHandle, Set<Integer>>();
		for (LocalVariableTag tag : localVariables.keySet()) {
			// have we already added one with the same slot number and start
			// location?
			// if so, just continue.
			LVPosition lvpos = localVariables.get(tag);
			InstructionHandle start = (tag.getSlot() < paramSlots ? methodStart : lvpos.start);
			InstructionHandle end = (tag.getSlot() < paramSlots ? methodEnd : lvpos.end);
			Set<Integer> slots = duplicatedLocalMap.get(start);
			if (slots == null) {
				slots = new HashSet<Integer>();
				duplicatedLocalMap.put(start, slots);
			} else if (slots.contains(new Integer(tag.getSlot()))) {
				// we already have a var starting at this tag with this slot
				continue;
			}
			slots.add(Integer.valueOf(tag.getSlot()));
			Type t = tag.getRealType();
			if (t == null) {
				t = BcelWorld.makeBcelType(UnresolvedType.forSignature(tag.getType()));
			}
			gen.addLocalVariable(tag.getName(), t, tag.getSlot(), start, end);
		}
	}