From d786f269834c89e6de4a6a90e7a9f22c583bc30a Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 22 Mar 2012 10:39:29 +0100 Subject: [vpm] avoid verifyerror: leave jump to tail-pos label the following commit deals with the fall-out in basicblocks (double closing of blocks in ignore mode) --- test/files/run/virtpatmat_tailcalls_verifyerror.check | 1 + test/files/run/virtpatmat_tailcalls_verifyerror.flags | 1 + test/files/run/virtpatmat_tailcalls_verifyerror.scala | 13 +++++++++++++ 3 files changed, 15 insertions(+) create mode 100644 test/files/run/virtpatmat_tailcalls_verifyerror.check create mode 100644 test/files/run/virtpatmat_tailcalls_verifyerror.flags create mode 100644 test/files/run/virtpatmat_tailcalls_verifyerror.scala (limited to 'test/files/run') diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.check b/test/files/run/virtpatmat_tailcalls_verifyerror.check new file mode 100644 index 0000000000..c508d5366f --- /dev/null +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.check @@ -0,0 +1 @@ +false diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.flags b/test/files/run/virtpatmat_tailcalls_verifyerror.flags new file mode 100644 index 0000000000..9769db9257 --- /dev/null +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.flags @@ -0,0 +1 @@ + -Yvirtpatmat -Xexperimental diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.scala b/test/files/run/virtpatmat_tailcalls_verifyerror.scala new file mode 100644 index 0000000000..1ee613f09e --- /dev/null +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.scala @@ -0,0 +1,13 @@ +// shouldn't result in a verify error when run... +object Test extends App { + @annotation.tailrec + final def test(meh: Boolean): Boolean = { + Some("a") match { + case x => + x match { + case _ => if(meh) test(false) else false + } + } + } + println(test(true)) +} \ No newline at end of file -- cgit v1.2.3 From 09eda4ef92168685ef3d301439bb8b6df76f982e Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 22 Mar 2012 17:36:12 +0100 Subject: do nothing when closing closed block in ignoremode this came to light with the virtual pattern matcher, which emits jumps like `matchEnd3(_test(Test.this, false))`, where _test is a tailcall the nested jumping caused double-closing (the second time in ignore mode) thus. when closing a closed block in ignore mode, simply do nothing from genLoad for label-jumps: note: when one of the args to genLoadLabelArguments is a jump to a label, it will call back into genLoad and arrive at this case, which will then set ctx1.bb.ignore to true, this is okay, since we're jumping unconditionally, so the loads and jumps emitted by the outer call to genLoad (by calling genLoadLabelArguments and emitOnly) can safely be ignored, however, as emitOnly will close the block, which reverses its instructions (when it's still open), we better not reverse when the block has already been closed but is in ignore mode (if it's not in ignore mode, double-closing is an error) @dragos figured it out, all I did was write the comment and the `if` test case to repro basic blocks crasher the tailcall in the forward jump `matchEnd3(_test(Test.this, false))` in the following program crashes the back-end (error below) @scala.annotation.tailrec final def test(meh: Boolean): Boolean = { val _$this: Test.type = Test.this; _test(_$this,meh){ case val x1: Some[String] = new Some[String]("a"); case3(){ matchEnd2({ case val x1: Some[String] = x1; case4(){ if (x1.ne(null)) matchEnd3(if (meh) _test(Test.this, false) else false) else case5() }; case5(){ matchEnd3(_test(Test.this, false)) }; matchEnd3(x){ x } }) }; matchEnd2(x){ x } } }; The last instruction (of basic block 11) is not a control flow instruction: CONSTANT(false) // methods def test(meh: Boolean (BOOL)): Boolean { locals: value meh, value _$this, value x1, value x, value x, value x1 startBlock: 1 blocks: [1,2,3,4,5,6,7,8,9,10,11,12,13] 1: 4 JUMP 2 2: 5 NEW REF(class Some) 5 DUP(REF(class Some)) 5 CONSTANT("a") 5 CALL_METHOD scala.Some. (static-instance) 5 STORE_LOCAL(value x1) 5 SCOPE_ENTER value x1 5 JUMP 3 3: 5 LOAD_LOCAL(value x1) 7 STORE_LOCAL(value x1) 7 SCOPE_ENTER value x1 7 JUMP 4 4: 7 LOAD_LOCAL(value x1) 7 CZJUMP (REF(class Object))NE ? 5 : 6 5: 8 LOAD_LOCAL(value meh) 8 CZJUMP (BOOL)NE ? 8 : 9 6: ? JUMP 11 7: 7 DROP BOOL 7 JUMP 11 8: 8 CONSTANT(false) 8 STORE_LOCAL(value meh) 8 JUMP 2 9: 8 CONSTANT(false) 8 JUMP 10 10: 8 STORE_LOCAL(value x) 8 JUMP 12 11: 9 JUMP 2 9 STORE_LOCAL(value meh) 9 CONSTANT(false) 12: 7 LOAD_LOCAL(value x) 7 SCOPE_EXIT value x1 7 STORE_LOCAL(value x) 7 JUMP 13 13: 5 LOAD_LOCAL(value x) 5 SCOPE_EXIT value x1 5 RETURN(BOOL) --- .../scala/tools/nsc/backend/icode/BasicBlocks.scala | 14 ++++++++++---- src/compiler/scala/tools/nsc/backend/icode/GenICode.scala | 7 +++++++ test/files/run/virtpatmat_tailcalls_verifyerror.scala | 3 ++- 3 files changed, 19 insertions(+), 5 deletions(-) (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala index 68c4ac03f6..4f3b0bf951 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala @@ -386,10 +386,16 @@ trait BasicBlocks { def close() { assert(!closed || ignore, this) assert(instructionList.nonEmpty, "Empty block: " + this) - closed = true - setFlag(DIRTYSUCCS) - instructionList = instructionList.reverse - instrs = instructionList.toArray + if (ignore && closed) { // redundant `ignore &&` for clarity -- we should never be in state `!ignore && closed` + // not doing anything to this block is important... + // because the else branch reverses innocent blocks, which is wrong when they're in ignore mode (and closed) + // reversing the instructions when (closed && ignore) wreaks havoc for nested label jumps (see comments in genLoad) + } else { + closed = true + setFlag(DIRTYSUCCS) + instructionList = instructionList.reverse + instrs = instructionList.toArray + } } def open() { diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala index 9e801e3ea8..41d9d93e7a 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala @@ -868,6 +868,13 @@ abstract class GenICode extends SubComponent { abort("Unknown label target: " + sym + " at: " + (fun.pos) + ": ctx: " + ctx) } }) + // note: when one of the args to genLoadLabelArguments is a jump to a label, + // it will call back into genLoad and arrive at this case, which will then set ctx1.bb.ignore to true, + // this is okay, since we're jumping unconditionally, so the loads and jumps emitted by the outer + // call to genLoad (by calling genLoadLabelArguments and emitOnly) can safely be ignored, + // however, as emitOnly will close the block, which reverses its instructions (when it's still open), + // we better not reverse when the block has already been closed but is in ignore mode + // (if it's not in ignore mode, double-closing is an error) val ctx1 = genLoadLabelArguments(args, label, ctx) ctx1.bb.emitOnly(if (label.anchored) JUMP(label.block) else PJUMP(label)) ctx1.bb.enterIgnoreMode diff --git a/test/files/run/virtpatmat_tailcalls_verifyerror.scala b/test/files/run/virtpatmat_tailcalls_verifyerror.scala index 1ee613f09e..5ce91e8dce 100644 --- a/test/files/run/virtpatmat_tailcalls_verifyerror.scala +++ b/test/files/run/virtpatmat_tailcalls_verifyerror.scala @@ -5,7 +5,8 @@ object Test extends App { Some("a") match { case x => x match { - case _ => if(meh) test(false) else false + case Some(_) => if(meh) test(false) else false + case _ => test(false) } } } -- cgit v1.2.3