[rvm-research] Approach for fixing the RVM-1017(Implicit unlock in synchronized method relies on local variable 0 )
Pushparaj Motamari <[email protected]>
| Newsgroups | gmane.comp.java.jikes.rvm.devel |
|---|---|
| Message-ID | <CAKO14CUyR7a-L8pPSBYb4na7P-7SDXw+BPgLEm9MTCcd_cvkog@mail.gmail.com> |
Hi, I am working on the fix for RVM-1017(Implicit unlock in synchronized method relies on local variable 0 ). I tried by reserving one extra localword in NormalMethod.java, now it adds extra word to every method, but it can be changed by putting "if(method.isSynchronized())". I am attaching the patch with the mail. When I do precommit with this patch for development configuration (dacapo), some testcases are failing ..like eclipse. But with prototype onfiguration i.e (basic and opttests) it is successful. I would like to know other approaches to fixing this. Thank You Pushparaj Motamari Instituto Superior Technico Birla Institute of Technology and Science-Pilani ------------------------------------------------------------------------------ Try New Relic Now & We'll Send You this Cool Shirt New Relic is the only SaaS-based application performance monitoring service that delivers powerful full stack analytics. Optimize and monitor your browser, app, & servers with just a few lines of code. Try New Relic and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr _______________________________________________ Jikesrvm-researchers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers
thisLock.patch
(application/octet-stream, 9.2 KB)
diff -uBr /home/pushparaj/New_UnMd/rvm/src/org/jikesrvm/classloader/NormalMethod.java ./org/jikesrvm/classloader/NormalMethod.java
--- /home/pushparaj/New_UnMd/rvm/src/org/jikesrvm/classloader/NormalMethod.java 2013-03-14 15:01:46.000000000 -0700
+++ ./org/jikesrvm/classloader/NormalMethod.java 2013-04-28 16:35:42.285388424 -0700
@@ -187,7 +187,7 @@
*/
@Uninterruptible
public int getLocalWords() {
- return localWords;
+ return localWords+1;
}
/**
Only in ./org/jikesrvm/classloader: NormalMethod.java~
diff -uBr /home/pushparaj/New_UnMd/rvm/src/org/jikesrvm/compilers/baseline/ia32/BaselineCompilerImpl.java ./org/jikesrvm/compilers/baseline/ia32/BaselineCompilerImpl.java
--- /home/pushparaj/New_UnMd/rvm/src/org/jikesrvm/compilers/baseline/ia32/BaselineCompilerImpl.java 2013-03-14 15:01:46.000000000 -0700
+++ ./org/jikesrvm/compilers/baseline/ia32/BaselineCompilerImpl.java 2013-04-27 11:15:39.471952306 -0700
@@ -3468,11 +3468,18 @@
Offset klassOffset = Offset.fromIntSignExtend(Statics.findOrCreateObjectLiteral(klass.getClassForType()));
// push java.lang.Class object for klass
asm.emitPUSH_Abs(Magic.getTocPointer().plus(klassOffset));
+ // asm.emitPOP_RegDisp(ESP,localOffset(method.getLocalWords()-1).plus(WORDSIZE));
} else {
- // push "this" object
- asm.emitPUSH_RegDisp(ESP, localOffset(0));
- }
- // pass 1 parameter
+ if (!(method.getDeclaringClass().getDescriptor().isBootstrapClassDescriptor() ||method.getDeclaringClass().getDescriptor().isRVMDescriptor() ||method.getDeclaringClass().getDescriptor().isReservedMemberName() ||method.getDeclaringClass().getDescriptor().isAnnotationClass()))
+ {
+ asm.emitMOV_Reg_RegDisp(T0, ESP, localOffset(0));
+ asm.emitMOV_RegDisp_Reg(ESP, localOffset(method.getLocalWords()-1), T0);
+ asm.emitPUSH_RegDisp(ESP, localOffset(0));
+ }else{
+ asm.emitPUSH_RegDisp(ESP, localOffset(0));
+ }
+ }
+ // pass 1 parameter
genParameterRegisterLoad(asm, 1);
asm.emitCALL_Abs(Magic.getTocPointer().plus(Entrypoints.lockMethod.getOffset()));
// after this instruction, the method has the monitor
@@ -3488,7 +3495,13 @@
// push java.lang.Class object for klass
asm.emitPUSH_Abs(Magic.getTocPointer().plus(klassOffset));
} else {
- asm.emitPUSH_RegDisp(ESP, localOffset(0)); // push "this" object
+ if (!(method.getDeclaringClass().getDescriptor().isBootstrapClassDescriptor() ||method.getDeclaringClass().getDescriptor().isRVMDescriptor() ||method.getDeclaringClass().getDescriptor().isReservedMemberName() ||method.getDeclaringClass().getDescriptor().isAnnotationClass()))
+ {
+ asm.emitPUSH_RegDisp(ESP, localOffset(method.getLocalWords()-1));
+ }else{
+ asm.emitPUSH_RegDisp(ESP, localOffset(0));
+ }
+ //asm.emitPUSH_RegDisp(ESP, localOffset(0)); // push "this" object
}
genParameterRegisterLoad(asm, 1); // pass 1 parameter
asm.emitCALL_Abs(Magic.getTocPointer().plus(Entrypoints.unlockMethod.getOffset()));
Only in ./org/jikesrvm/compilers/baseline/ia32: BaselineCompilerImpl.java~
diff -uBr /home/pushparaj/New_UnMd/rvm/src/org/jikesrvm/compilers/opt/bc2ir/GenerationContext.java ./org/jikesrvm/compilers/opt/bc2ir/GenerationContext.java
--- /home/pushparaj/New_UnMd/rvm/src/org/jikesrvm/compilers/opt/bc2ir/GenerationContext.java 2013-03-14 15:01:46.000000000 -0700
+++ ./org/jikesrvm/compilers/opt/bc2ir/GenerationContext.java 2013-04-28 16:48:40.297416979 -0700
@@ -83,6 +83,7 @@
*/
CompiledMethod original_cm;
+ public int numberOfArgs;
/**
* The method to be generated
*/
@@ -156,7 +156,7 @@
* synchronized methods.
*/
BasicBlock unlockAndRethrow;
-
+ public TypeReference thisTypeRef;
/**
* The Register to which BC2IR should assign the return value(s)
* of the method. It will be null when the method has a void return.
@@ -249,6 +249,7 @@
int numParams = params.length;
int argIdx = 0;
int localNum = 0;
+ //arguments = new Operand[method.isStatic() ? numParams : numParams + 1];
arguments = new Operand[method.isStatic() ? numParams : numParams + 1];
// Insert IR_PROLOGUE instruction. Loop below will fill in its operands
Instruction prologueInstr = Prologue.create(IR_PROLOGUE, arguments.length);
@@ -257,7 +258,10 @@
if (!method.isStatic()) {
TypeReference thisType = meth.getDeclaringClass().getTypeRef();
RegisterOperand thisOp = makeLocal(localNum, thisType);
- // The this param of a virtual method is by definition non null
+ makeLocal(method.getLocalWords()-1, thisType);
+ thisTypeRef = thisType;
+ numberOfArgs = numParams+1;
+ // The this param of a virtual method is by definition non null
RegisterOperand guard = makeNullCheckGuard(thisOp.getRegister());
BC2IR.setGuard(thisOp, guard);
appendInstruction(prologue, Move.create(GUARD_MOVE, guard.copyRO(), new TrueGuardOperand()), PROLOGUE_BCI);
@@ -268,6 +272,8 @@
}
arguments[0] = thisOp;
Prologue.setFormal(prologueInstr, 0, thisOp.copyU2D());
+ /* simulating aload0 , astore_method.getLocalWords-1*/
+ appendInstruction(prologue, Move.create(IRTools.getMoveOp(thisType),makeLocal(method.getLocalWords()-1, thisType), thisOp), PROLOGUE_BCI);
argIdx++;
localNum++;
}
@@ -330,11 +334,15 @@
// Now inherit state based on callSite
child.inlineSequence = new InlineSequence(child.method, callSite.position, callSite);
child.enclosingHandlers = ebag;
- child.arguments = new Operand[Call.getNumberOfParams(callSite)];
+ child.numberOfArgs = Call.getNumberOfParams(callSite);
+ // child.arguments = new Operand[Call.getNumberOfParams(callSite)];
+ child.arguments = new Operand[child.numberOfArgs];
+ // for (int i = 0; i < child.arguments.length; i++) {
for (int i = 0; i < child.arguments.length; i++) {
child.arguments[i] = Call.getParam(callSite, i).copy(); // copy instead
// of clearing in case inlining aborts.
}
+ //child.arguments[child.numberOfArgs-1]=Call.getParam(callSite, 0).copy();
if (Call.hasResult(callSite)) {
child.resultReg = Call.getResult(callSite).copyD2D().getRegister();
child.resultReg.setSpansBasicBlock(); // it will...
@@ -369,9 +375,28 @@
objPtr.setDeclaredType();
objPtr.setType(child.method.getDeclaringClass().getTypeRef());
}
+ RegisterOperand myOp = objPtr.copyRO();
local = child.makeLocal(localNum, objPtr);
localNum++;
- child.arguments[0] = local; // Avoid confusion in BC2IR of callee
+ child.arguments[0] = local;
+ child.makeLocal(child.method.getLocalWords()-1, myOp);
+ if(local!=null){
+ Instruction n = Move.create(IRTools.getMoveOp(local.getType()),child.makeLocal(child.method.getLocalWords()-1, local.getType()), local);
+ //s.bcIndex = PROLOGUE_BCI;
+ child.prologue.appendInstruction(n);
+ }
+ /*receiver = child.arguments[0];
+ objPtr = receiver.asRegister();
+ if (ClassLoaderProxy.includesType(child.method.getDeclaringClass().getTypeRef(), objPtr.getType()) != YES) {
+ // narrow type of actual to match formal static type implied by method
+ objPtr.clearPreciseType(); // Can be precise but not assignable if enough classes aren't loaded
+ objPtr.setDeclaredType();
+ objPtr.setType(child.method.getDeclaringClass().getTypeRef());
+ }
+ child.arguments[child.numberOfArgs-1]=child.makeLocal(child.method.getLocalWords(), objPtr);
+ */
+ //child.thisTypeRef =child.makeLocal(child.method.getLocalWords(), objPtr).type;
+ //child.arguments[child.numberOfArgs-1] = child.makeLocal(child.method.getLocalWords(),objPtr );// Avoid confusion in BC2IR of callee
// when objPtr is a local in the caller.
} else if (receiver.isConstant()) {
local = child.makeLocal(localNum, receiver.getType());
@@ -388,7 +413,7 @@
s.bcIndex = PROLOGUE_BCI;
s.position = callSite.position;
child.prologue.appendInstruction(s);
- }
+ }
for (int paramIdx = 0; paramIdx < numParams; paramIdx++, argIdx++) {
TypeReference argType = params[paramIdx];
RegisterOperand formal;
@@ -774,8 +798,14 @@
Offset offs = Offset.fromIntSignExtend(Statics.findOrCreateObjectLiteral(klass));
return new ClassConstantOperand(klass, offs);
} else {
- return makeLocal(0, arguments[0].getType());
- }
+ if (!(method.getDeclaringClass().getDescriptor().isBootstrapClassDescriptor() ||method.getDeclaringClass().getDescriptor().isRVMDescriptor() ||method.getDeclaringClass().getDescriptor().isReservedMemberName() ||method.getDeclaringClass().getDescriptor().isAnnotationClass())){
+ return makeLocal(method.getLocalWords()-1, method.getDeclaringClass().getTypeRef());
+ }else{
+ return makeLocal(0, method.getDeclaringClass().getTypeRef());
+ }
+ //return new RegisterOperand(addressLocals[method.getLocalWords()],arguments[arguments.length-1].getType());
+ //new RegisterOperand(, type);
+ }
}
private void appendInstruction(BasicBlock b, Instruction s, int bcIndex) {
Only in ./org/jikesrvm/compilers/opt/bc2ir: GenerationContext.java~
Only in ./: thisLock.patch