From 59183c78216135a2fbe58323b76a4dc5944c65cc Mon Sep 17 00:00:00 2001 From: Joel Dice Date: Wed, 16 Feb 2011 14:29:57 -0700 Subject: [PATCH] fix subroutine stack mapping bug leading to crashes during GC The stack mapping code was broken for cases of stack slots being reused to hold primitives or addresses within subroutines after previously being used to hold object references. We now bitwise "and" the stack map upon return from the subroutine with the map as it existed prior to calling the subroutine, which has the effect of clearing map locations previously marked as GC roots where appropriate. --- src/compile.cpp | 35 +++++++++++++++--- test/Subroutine.java | 84 ++++++++++++++++++++++---------------------- 2 files changed, 72 insertions(+), 47 deletions(-) diff --git a/src/compile.cpp b/src/compile.cpp index 12189c216a..37374f03c0 100644 --- a/src/compile.cpp +++ b/src/compile.cpp @@ -5288,7 +5288,8 @@ calculateTryCatchRoots(Context* context, SubroutinePath* subroutinePath, unsigned calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, - unsigned eventIndex, SubroutinePath* subroutinePath = 0) + unsigned eventIndex, SubroutinePath* subroutinePath = 0, + uintptr_t* resultRoots = 0) { // for each instruction with more than one predecessor, and for each // stack position, determine if there exists a path to that @@ -5321,11 +5322,12 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, switch (e) { case PushContextEvent: { eventIndex = calculateFrameMaps - (t, context, RUNTIME_ARRAY_BODY(roots), eventIndex, subroutinePath); + (t, context, RUNTIME_ARRAY_BODY(roots), eventIndex, subroutinePath, + resultRoots); } break; case PopContextEvent: - return eventIndex; + goto exit; case IpEvent: { ip = context->eventLog.get2(eventIndex); @@ -5491,18 +5493,41 @@ calculateFrameMaps(MyThread* t, Context* context, uintptr_t* originalRoots, makeRootTable(t, &(context->zone), context->method)); } + THREAD_RUNTIME_ARRAY(t, uintptr_t, subroutineRoots, mapSize); + calculateFrameMaps (t, context, RUNTIME_ARRAY_BODY(roots), call->subroutine->logIndex, - path); + path, RUNTIME_ARRAY_BODY(subroutineRoots)); + + for (unsigned wi = 0; wi < mapSize; ++wi) { + RUNTIME_ARRAY_BODY(roots)[wi] + &= RUNTIME_ARRAY_BODY(subroutineRoots)[wi]; + } } break; case PopSubroutineEvent: - return static_cast(-1); + eventIndex = static_cast(-1); + goto exit; default: abort(t); } } + exit: + if (resultRoots and ip != -1) { + if (DebugFrameMaps) { + fprintf(stderr, "result roots at ip %3d: ", ip); + printSet(*RUNTIME_ARRAY_BODY(roots), mapSize); + if (subroutinePath) { + fprintf(stderr, " "); + print(subroutinePath); + } + fprintf(stderr, "\n"); + } + + memcpy(resultRoots, RUNTIME_ARRAY_BODY(roots), mapSize * BytesPerWord); + } + return eventIndex; } diff --git a/test/Subroutine.java b/test/Subroutine.java index c44c5df8ef..b02ec9ba81 100644 --- a/test/Subroutine.java +++ b/test/Subroutine.java @@ -31,7 +31,7 @@ public class Subroutine { // 4: Stream.write1(out, Assembler.invokestatic); Stream.write2(out, ConstantPool.addMethodRef - (pool, "java/lang/System", "gc", "()V") + 1); + (pool, "java/lang/System", "gc", "()V") + 1); // 7: Stream.write1(out, Assembler.goto_); @@ -43,7 +43,7 @@ public class Subroutine { // 11: Stream.write1(out, Assembler.invokestatic); Stream.write2(out, ConstantPool.addMethodRef - (pool, "java/lang/System", "gc", "()V") + 1); + (pool, "java/lang/System", "gc", "()V") + 1); // 14: Stream.write1(out, Assembler.ret); @@ -56,7 +56,7 @@ public class Subroutine { // 19: Stream.write1(out, Assembler.invokestatic); Stream.write2(out, ConstantPool.addMethodRef - (pool, "java/lang/System", "gc", "()V") + 1); + (pool, "java/lang/System", "gc", "()V") + 1); // 22: Stream.write1(out, Assembler.return_); @@ -237,58 +237,58 @@ public class Subroutine { } public static void main(String[] args) throws Exception { - // test(false, false); - // test(false, true); - // test(true, false); + test(false, false); + test(false, true); + test(true, false); - // String.valueOf(test2(1)); - // String.valueOf(test2(2)); - // String.valueOf(test2(3)); + String.valueOf(test2(1)); + String.valueOf(test2(2)); + String.valueOf(test2(3)); - // String.valueOf(test3(1, 1, 1)); - // String.valueOf(test3(2, 1, 1)); - // String.valueOf(test3(3, 1, 1)); + String.valueOf(test3(1, 1, 1)); + String.valueOf(test3(2, 1, 1)); + String.valueOf(test3(3, 1, 1)); - // String.valueOf(test3(1, 2, 1)); - // String.valueOf(test3(2, 2, 1)); - // String.valueOf(test3(3, 2, 1)); + String.valueOf(test3(1, 2, 1)); + String.valueOf(test3(2, 2, 1)); + String.valueOf(test3(3, 2, 1)); - // String.valueOf(test3(1, 3, 1)); - // String.valueOf(test3(2, 3, 1)); - // String.valueOf(test3(3, 3, 1)); + String.valueOf(test3(1, 3, 1)); + String.valueOf(test3(2, 3, 1)); + String.valueOf(test3(3, 3, 1)); - // String.valueOf(test3(1, 1, 2)); - // String.valueOf(test3(2, 1, 2)); - // String.valueOf(test3(3, 1, 2)); + String.valueOf(test3(1, 1, 2)); + String.valueOf(test3(2, 1, 2)); + String.valueOf(test3(3, 1, 2)); - // String.valueOf(test3(1, 2, 2)); - // String.valueOf(test3(2, 2, 2)); - // String.valueOf(test3(3, 2, 2)); + String.valueOf(test3(1, 2, 2)); + String.valueOf(test3(2, 2, 2)); + String.valueOf(test3(3, 2, 2)); - // String.valueOf(test3(1, 3, 2)); - // String.valueOf(test3(2, 3, 2)); - // String.valueOf(test3(3, 3, 2)); + String.valueOf(test3(1, 3, 2)); + String.valueOf(test3(2, 3, 2)); + String.valueOf(test3(3, 3, 2)); - // String.valueOf(test3(1, 1, 3)); - // String.valueOf(test3(2, 1, 3)); - // String.valueOf(test3(3, 1, 3)); + String.valueOf(test3(1, 1, 3)); + String.valueOf(test3(2, 1, 3)); + String.valueOf(test3(3, 1, 3)); - // String.valueOf(test3(1, 2, 3)); - // String.valueOf(test3(2, 2, 3)); - // String.valueOf(test3(3, 2, 3)); + String.valueOf(test3(1, 2, 3)); + String.valueOf(test3(2, 2, 3)); + String.valueOf(test3(3, 2, 3)); - // String.valueOf(test3(1, 3, 3)); - // String.valueOf(test3(2, 3, 3)); - // String.valueOf(test3(3, 3, 3)); + String.valueOf(test3(1, 3, 3)); + String.valueOf(test3(2, 3, 3)); + String.valueOf(test3(3, 3, 3)); - // String.valueOf(test4(1)); - // String.valueOf(test4(2)); - // String.valueOf(test4(3)); + String.valueOf(test4(1)); + String.valueOf(test4(2)); + String.valueOf(test4(3)); - // expect(test4(1) == 0xFABFABFABFL); + expect(test4(1) == 0xFABFABFABFL); - // new Subroutine().test5(true); - // new Subroutine().test5(false); + new Subroutine().test5(true); + new Subroutine().test5(false); makeTestClass().getMethod("test", new Class[0]).invoke (null, new Object[0]);