Attachment 56874 Details for Bug 169017 – Proposed fix (first draft)

[patch] Proposed fix (first draft)

patch_169017_4.txt (text/plain), 18.15 KB, created by Olivier Thomann

on 2007-01-13 22:02:04 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Olivier Thomann

Created: 2007-01-13 22:02:04 EST

Size: 18.15 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java,v
>retrieving revision 1.104
>diff -u -r1.104 TryStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java	8 Jan 2007 15:52:42 -0000	1.104
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java	13 Jan 2007 19:45:12 -0000
>@@ -57,7 +57,7 @@
> 	// for local variables table attributes
> 	int mergedInitStateIndex = -1;
> 	int preTryInitStateIndex = -1;
>-	int postTryInitStateIndex = -1;
>+	int naturalExitMergeInitStateIndex = -1;
> 
> public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {
> 
>@@ -95,12 +95,10 @@
> 		FlowInfo tryInfo;
> 		if (this.tryBlock.isEmptyBlock()) {
> 			tryInfo = flowInfo;
>-			this.postTryInitStateIndex = currentScope.methodScope().recordInitializationStates(tryInfo);
> 		} else {
> 			tryInfo = this.tryBlock.analyseCode(currentScope, handlingContext, flowInfo.copy());
> 			if ((tryInfo.tagBits & FlowInfo.UNREACHABLE) != 0)
> 				this.bits |= ASTNode.IsTryBlockExiting;
>-			this.postTryInitStateIndex = currentScope.methodScope().recordInitializationStates(tryInfo);
> 		}
> 
> 		// check unreachable catch blocks
>@@ -207,12 +205,10 @@
> 		FlowInfo tryInfo;
> 		if (this.tryBlock.isEmptyBlock()) {
> 			tryInfo = flowInfo;
>-			this.postTryInitStateIndex = currentScope.methodScope().recordInitializationStates(tryInfo);
> 		} else {
> 			tryInfo = this.tryBlock.analyseCode(currentScope, handlingContext, flowInfo.copy());
> 			if ((tryInfo.tagBits & FlowInfo.UNREACHABLE) != 0)
> 				this.bits |= ASTNode.IsTryBlockExiting;
>-			this.postTryInitStateIndex = currentScope.methodScope().recordInitializationStates(tryInfo);
> 		}
> 
> 		// check unreachable catch blocks
>@@ -298,6 +294,8 @@
> 				currentScope.methodScope().recordInitializationStates(subInfo);
> 			return subInfo;
> 		} else {
>+			this.naturalExitMergeInitStateIndex =
>+				currentScope.methodScope().recordInitializationStates(tryInfo);
> 			FlowInfo mergedInfo = tryInfo.addInitializationsFrom(subInfo);
> 			this.mergedInitStateIndex =
> 				currentScope.methodScope().recordInitializationStates(mergedInfo);
>@@ -404,16 +402,16 @@
> 				case FINALLY_SUBROUTINE :
> 				case FINALLY_INLINE :
> 					requiresNaturalExit = true;
>-					if (this.postTryInitStateIndex != -1) {
>-						codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>-						codeStream.addDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>+					if (this.naturalExitMergeInitStateIndex != -1) {
>+						codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
>+						codeStream.addDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
> 					}
> 					codeStream.goto_(naturalExitLabel);
> 					break;
> 				case NO_FINALLY :
>-					if (this.postTryInitStateIndex != -1) {
>-						codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>-						codeStream.addDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>+					if (this.naturalExitMergeInitStateIndex != -1) {
>+						codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
>+						codeStream.addDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
> 					}
> 					codeStream.goto_(naturalExitLabel);
> 					break;
>@@ -464,9 +462,9 @@
> 							requiresNaturalExit = true;
> 							// fall through
> 						case NO_FINALLY :
>-							if (this.postTryInitStateIndex != -1) {
>-								codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>-								codeStream.addDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>+							if (this.naturalExitMergeInitStateIndex != -1) {
>+								codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
>+								codeStream.addDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
> 							}
> 							codeStream.goto_(naturalExitLabel);
> 							break;
>@@ -491,6 +489,7 @@
> 			if (this.preTryInitStateIndex != -1) {
> 				// reset initialization state, as for a normal catch block
> 				codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.preTryInitStateIndex);
>+				codeStream.addDefinitelyAssignedVariables(currentScope, this.preTryInitStateIndex);
> 			}
> 			this.placeAllAnyExceptionHandler();
> 			if (naturalExitExceptionHandler != null) naturalExitExceptionHandler.place();
>@@ -557,9 +556,13 @@
> 						break;
> 					case FINALLY_INLINE :
> 						// inlined finally here can see all merged variables
>-						if (this.postTryInitStateIndex != -1) {
>-							codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>-							codeStream.addDefinitelyAssignedVariables(currentScope, this.postTryInitStateIndex);
>+						boolean isStackMapFrameCodeStream = codeStream instanceof StackMapFrameCodeStream;
>+						if (isStackMapFrameCodeStream) {
>+							((StackMapFrameCodeStream) codeStream).pushStateIndex(this.naturalExitMergeInitStateIndex);
>+						}
>+						if (this.naturalExitMergeInitStateIndex != -1) {
>+							codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
>+							codeStream.addDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
> 						}
> 						naturalExitLabel.place();
> 						// entire sequence for finally is associated to finally block
>@@ -572,6 +575,9 @@
> 									position,
> 									this.finallyBlock.sourceEnd);
> 						}
>+						if (isStackMapFrameCodeStream) {
>+							((StackMapFrameCodeStream) codeStream).popStateIndex();
>+						}
> 						break;
> 					case FINALLY_DOES_NOT_COMPLETE :
> 						break;
>@@ -617,9 +623,9 @@
> }
> 
> /**
>- * @see SubRoutineStatement#generateSubRoutineInvocation(BlockScope, CodeStream, Object)
>+ * @see SubRoutineStatement#generateSubRoutineInvocation(BlockScope, CodeStream, Object, int, LocalVariableBinding)
>  */
>-public boolean generateSubRoutineInvocation(BlockScope currentScope, CodeStream codeStream, Object targetLocation) {
>+public boolean generateSubRoutineInvocation(BlockScope currentScope, CodeStream codeStream, Object targetLocation, int stateIndex, LocalVariableBinding secretLocal) {
> 
> 	int finallyMode = finallyMode();
> 	switch(finallyMode) {
>@@ -663,17 +669,28 @@
> 		BranchLabel reusableJSRSequenceStartLabel = new BranchLabel(codeStream);
> 		reusableJSRSequenceStartLabel.place();
> 		this.reusableJSRSequenceStartLabels[this.reusableJSRTargetsCount++] = reusableJSRSequenceStartLabel;
>-	}			
>+	}
> 	if (finallyMode == FINALLY_INLINE) {
>-		if (this.preTryInitStateIndex != -1) {
>+		boolean isStackMapFrameCodeStream = codeStream instanceof StackMapFrameCodeStream;
>+		if (isStackMapFrameCodeStream) {
>+			((StackMapFrameCodeStream) codeStream).pushStateIndex(stateIndex);
>+		}
>+		if (this.naturalExitMergeInitStateIndex != -1 || stateIndex != -1) {
> 			// reset initialization state, as for a normal catch block
>-			codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.preTryInitStateIndex);
>+			codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
>+			codeStream.addDefinitelyAssignedVariables(currentScope, this.naturalExitMergeInitStateIndex);
>+		}
>+		if (secretLocal != null) {
>+			codeStream.addVariable(secretLocal);
> 		}
> 		// cannot use jsr bytecode, then simply inline the subroutine
> 		// inside try block, ensure to deactivate all catch block exception handlers while inlining finally block
> 		exitAnyExceptionHandler();
> 		exitDeclaredExceptionHandlers(codeStream);
> 		this.finallyBlock.generateCode(currentScope, codeStream);
>+		if (isStackMapFrameCodeStream) {
>+			((StackMapFrameCodeStream) codeStream).popStateIndex();
>+		}
> 	} else {
> 		// classic subroutine invocation, distinguish case of non-returning subroutine
> 		codeStream.jsr(this.subRoutineStartLabel);
>@@ -682,7 +699,6 @@
> 	}
> 	return false;
> }
>-
> public boolean isSubRoutineEscaping() {
> 	return (this.bits & ASTNode.IsSubRoutineEscaping) != 0;
> }
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/SubRoutineStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SubRoutineStatement.java,v
>retrieving revision 1.12
>diff -u -r1.12 SubRoutineStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/SubRoutineStatement.java	23 Aug 2006 16:32:56 -0000	1.12
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/SubRoutineStatement.java	13 Jan 2007 19:45:11 -0000
>@@ -13,6 +13,7 @@
> import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
> import org.eclipse.jdt.internal.compiler.codegen.ExceptionLabel;
> import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
>+import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
> 
> /**
>  * Extra behavior for statements which are generating subroutines
>@@ -55,7 +56,7 @@
> 	}
> 	
> 
>-	public abstract boolean generateSubRoutineInvocation(BlockScope currentScope, CodeStream codeStream, Object targetLocation);	
>+	public abstract boolean generateSubRoutineInvocation(BlockScope currentScope, CodeStream codeStream, Object targetLocation, int stateIndex, LocalVariableBinding secretLocal);	
> 	
> 	public abstract boolean isSubRoutineEscaping();
> 	
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/BranchStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BranchStatement.java,v
>retrieving revision 1.20
>diff -u -r1.20 BranchStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/BranchStatement.java	23 Aug 2006 16:32:56 -0000	1.20
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/BranchStatement.java	13 Jan 2007 19:45:11 -0000
>@@ -45,7 +45,7 @@
> 	if (this.subroutines != null){
> 		for (int i = 0, max = this.subroutines.length; i < max; i++){
> 			SubRoutineStatement sub = this.subroutines[i];
>-			boolean didEscape = sub.generateSubRoutineInvocation(currentScope, codeStream, this.targetLabel);
>+			boolean didEscape = sub.generateSubRoutineInvocation(currentScope, codeStream, null, this.initStateIndex, null);
> 			if (didEscape) {
> 					codeStream.recordPositionsFrom(pc, this.sourceStart);
> 					SubRoutineStatement.reenterAllExceptionHandlers(this.subroutines, i, codeStream);
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java,v
>retrieving revision 1.45
>diff -u -r1.45 SynchronizedStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java	23 Aug 2006 16:32:56 -0000	1.45
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/SynchronizedStatement.java	13 Jan 2007 19:45:11 -0000
>@@ -152,9 +152,9 @@
> }
> 
> /**
>- * @see SubRoutineStatement#generateSubRoutineInvocation(BlockScope, CodeStream, Object)
>+ * @see SubRoutineStatement#generateSubRoutineInvocation(BlockScope, CodeStream, Object, int, LocalVariableBinding)
>  */
>-public boolean generateSubRoutineInvocation(BlockScope currentScope, CodeStream codeStream, Object targetLocation) {
>+public boolean generateSubRoutineInvocation(BlockScope currentScope, CodeStream codeStream, Object targetLocation, int stateIndex, LocalVariableBinding secretLocal) {
> 	codeStream.load(this.synchroVariable);
> 	codeStream.monitorexit();
> 	exitAnyExceptionHandler();
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java,v
>retrieving revision 1.54
>diff -u -r1.54 ReturnStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java	4 Sep 2006 17:05:32 -0000	1.54
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java	13 Jan 2007 19:45:11 -0000
>@@ -129,10 +129,9 @@
> 	
> 	// generation of code responsible for invoking the finally blocks in sequence
> 	if (this.subroutines != null) {
>-		Object reusableJSRTarget = this.expression == null ? (Object)TypeBinding.VOID : this.expression.reusableJSRTarget();
> 		for (int i = 0, max = this.subroutines.length; i < max; i++) {
> 			SubRoutineStatement sub = this.subroutines[i];
>-			boolean didEscape = sub.generateSubRoutineInvocation(currentScope, codeStream, reusableJSRTarget);
>+			boolean didEscape = sub.generateSubRoutineInvocation(currentScope, codeStream, null, this.initStateIndex, this.saveValueVariable);
> 			if (didEscape) {
> 					if (this.initStateIndex != -1) {
> 						codeStream.removeNotDefinitelyAssignedVariables(currentScope, this.initStateIndex);
>@@ -174,7 +173,9 @@
> }
> 
> public void generateStoreSaveValueIfNecessary(CodeStream codeStream){
>-	if (this.saveValueVariable != null) codeStream.store(this.saveValueVariable, false);
>+	if (this.saveValueVariable != null) {
>+		codeStream.store(this.saveValueVariable, false);
>+	}
> }
> 
> public boolean needValue() {
>Index: compiler/org/eclipse/jdt/internal/compiler/codegen/StackMapFrameCodeStream.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/codegen/StackMapFrameCodeStream.java,v
>retrieving revision 1.16
>diff -u -r1.16 StackMapFrameCodeStream.java
>--- compiler/org/eclipse/jdt/internal/compiler/codegen/StackMapFrameCodeStream.java	5 Jan 2007 04:26:55 -0000	1.16
>+++ compiler/org/eclipse/jdt/internal/compiler/codegen/StackMapFrameCodeStream.java	13 Jan 2007 19:45:13 -0000
>@@ -38,6 +38,9 @@
> 	
> 	public ArrayList variablesModificationsPositions;
> 	
>+	public int[] stateIndexes;
>+	public int stateIndexesCounter;
>+	
> public StackMapFrameCodeStream(ClassFile givenClassFile) {
> 	super(givenClassFile);
> }
>@@ -56,23 +59,22 @@
> }
> public void addDefinitelyAssignedVariables(Scope scope, int initStateIndex) {
> 	// Required to fix 1PR0XVS: LFRE:WINNT - Compiler: variable table for method appears incorrect
>-	for (int i = 0; i < visibleLocalsCount; i++) {
>+	loop: for (int i = 0; i < visibleLocalsCount; i++) {
> 		LocalVariableBinding localBinding = visibleLocals[i];
> 		if (localBinding != null) {
> 			// Check if the local is definitely assigned
>-			if (isDefinitelyAssigned(scope, initStateIndex, localBinding)) {
>-				if ((localBinding.initializationCount == 0) || (localBinding.initializationPCs[((localBinding.initializationCount - 1) << 1) + 1] != -1)) {
>-					/* There are two cases:
>-					 * 1) there is no initialization interval opened ==> add an opened interval
>-					 * 2) there is already some initialization intervals but the last one is closed ==> add an opened interval
>-					 * An opened interval means that the value at localBinding.initializationPCs[localBinding.initializationCount - 1][1]
>-					 * is equals to -1.
>-					 * initializationPCs is a collection of pairs of int:
>-					 * 	first value is the startPC and second value is the endPC. -1 one for the last value means that the interval
>-					 * 	is not closed yet.
>-					 */
>-					currentFrame.putLocal(localBinding.resolvedPosition, new VerificationTypeInfo(localBinding.type));
>+			boolean isDefinitelyAssigned = isDefinitelyAssigned(scope, initStateIndex, localBinding);
>+			if (!isDefinitelyAssigned) {
>+				if (this.stateIndexes != null) {
>+					for (int j = 0, max = this.stateIndexesCounter; j < max; j++) {
>+						if (isDefinitelyAssigned(scope, this.stateIndexes[j], localBinding)) {
>+							currentFrame.putLocal(localBinding.resolvedPosition, new VerificationTypeInfo(localBinding.type));
>+							continue loop;
>+						}
>+					}
> 				}
>+			} else {
>+				currentFrame.putLocal(localBinding.resolvedPosition, new VerificationTypeInfo(localBinding.type));
> 			}
> 		}
> 	}
>@@ -1746,6 +1748,9 @@
> 			this.currentFrame.numberOfStackItems -= 2;
> 	}
> }
>+public void popStateIndex() {
>+	this.stateIndexesCounter--;
>+}
> public void pushOnStack(TypeBinding binding) {
> 	super.pushOnStack(binding);
> 	this.currentFrame.addStackItem(binding);
>@@ -1754,6 +1759,18 @@
> 	super.putfield(fieldBinding);
> 	this.currentFrame.numberOfStackItems -= 2;	
> }
>+
>+public void pushStateIndex(int naturalExitMergeInitStateIndex) {
>+	if (this.stateIndexes == null) {
>+		this.stateIndexes = new int[3];
>+	}
>+	int length = this.stateIndexes.length;
>+	if (length == this.stateIndexesCounter) {
>+		// resize
>+		System.arraycopy(this.stateIndexes, 0, (this.stateIndexes = new int[length * 2]), 0, length);
>+	}
>+	this.stateIndexes[this.stateIndexesCounter++] = naturalExitMergeInitStateIndex;
>+}
> public void putstatic(FieldBinding fieldBinding) {
> 	super.putstatic(fieldBinding);
> 	this.currentFrame.numberOfStackItems--;	
>@@ -1768,11 +1785,20 @@
> }
> public void removeNotDefinitelyAssignedVariables(Scope scope, int initStateIndex) {
> 	int index = this.visibleLocalsCount;
>-	for (int i = 0; i < index; i++) {
>+	loop : for (int i = 0; i < index; i++) {
> 		LocalVariableBinding localBinding = visibleLocals[i];
>-		if (localBinding != null && !isDefinitelyAssigned(scope, initStateIndex, localBinding)
>-				&& localBinding.initializationCount > 0) {
>-			this.currentFrame.removeLocals(localBinding.resolvedPosition);
>+		if (localBinding != null && localBinding.initializationCount > 0) {
>+			boolean isDefinitelyAssigned = isDefinitelyAssigned(scope, initStateIndex, localBinding);
>+			if (!isDefinitelyAssigned) {
>+				if (this.stateIndexes != null) {
>+					for (int j = 0, max = this.stateIndexesCounter; j < max; j++) {
>+						if (isDefinitelyAssigned(scope, this.stateIndexes[j], localBinding)) {
>+							continue loop;
>+						}
>+					}
>+				}
>+				this.currentFrame.removeLocals(localBinding.resolvedPosition);
>+			}
> 		}
> 	}
> 	Integer newValue = new Integer(this.position);

Actions: View | Diff

Attachments on bug 169017: 56207 | 56874 | 57204 | 57205 | 57386