Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recent Fn change cause use after free. #597

Open
mhermier opened this issue Sep 1, 2018 · 1 comment
Open

Recent Fn change cause use after free. #597

mhermier opened this issue Sep 1, 2018 · 1 comment

Comments

@mhermier
Copy link
Contributor

mhermier commented Sep 1, 2018

Hi,
I tried to hack simpilify the core library with the simple change:

---------------------------- src/vm/wren_core.wren ----------------------------
index c2d0af92..a10afc0c 100644
@@ -111,9 +111,7 @@ class Sequence {
 
   toList {
     var result = List.new()
-    for (element in this) {
-      result.add(element)
-    }
+    result.addAll(this)
     return result
   }
 }

---------

and its triggering an error in the following:

FAIL: test/io/directory/list.wren
      Expected output "[a.txt, b.txt, c.txt]" on line 8 and got "[b.txt, c.txt]".

Oddly, trying to trace the issue within WhereSequence::iterate(_) by adding System.print(iterator) seems to fix the error. So I higly suspect it is a compiler error.
Infortunately I have no time to investigate this issue more for now.

@mhermier
Copy link
Contributor Author

I traced this error, and is related to: 5f29a72

Valgrind shows a use after free:

==22903== Invalid write of size 8
==22903==    at 0x1297E1: runInterpreter (wren_vm.c:1001)
==22903==    by 0x12BC5C: wrenCall (wren_vm.c:1387)
==22903==    by 0x10FE7C: resume (scheduler.c:22)
==22903==    by 0x10FFF8: schedulerFinishResume (scheduler.c:59)
==22903==    by 0x10E941: directoryListCallback (io.c:141)
==22903==    by 0x143553: uv__work_done (threadpool.c:236)
==22903==    by 0x1438B0: uv__async_event (async.c:98)
==22903==    by 0x14398F: uv__async_io (async.c:138)
==22903==    by 0x13FD7E: uv__io_poll (linux-core.c:382)
==22903==    by 0x1316B3: uv_run (core.c:352)
==22903==    by 0x12E93D: runFile (vm.c:345)
==22903==    by 0x112169: main (main.c:129)
==22903==  Address 0x5a031d8 is 72 bytes inside a block of size 96 free'd
==22903==    at 0x4C2F13F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22903==    by 0x127618: defaultReallocate (wren_vm.c:35)
==22903==    by 0x127B81: wrenReallocate (wren_vm.c:222)
==22903==    by 0x11B3FA: wrenCallFunction (wren_vm.h:186)
==22903==    by 0x11BECC: call (wren_core.c:259)
==22903==    by 0x11BF2A: prim_fn_call1 (wren_core.c:270)
==22903==    by 0x129797: runInterpreter (wren_vm.c:994)
==22903==    by 0x12BC5C: wrenCall (wren_vm.c:1387)
==22903==    by 0x10FE7C: resume (scheduler.c:22)
==22903==    by 0x10FFF8: schedulerFinishResume (scheduler.c:59)
==22903==    by 0x10E941: directoryListCallback (io.c:141)
==22903==    by 0x143553: uv__work_done (threadpool.c:236)
==22903==    by 0x1438B0: uv__async_event (async.c:98)
==22903==    by 0x14398F: uv__async_io (async.c:138)
==22903==    by 0x13FD7E: uv__io_poll (linux-core.c:382)
==22903==    by 0x1316B3: uv_run (core.c:352)
==22903==    by 0x12E93D: runFile (vm.c:345)
==22903==    by 0x112169: main (main.c:129)
==22903==  Block was alloc'd at
==22903==    at 0x4C2CD8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22903==    by 0x4C2F195: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22903==    by 0x127618: defaultReallocate (wren_vm.c:35)
==22903==    by 0x127B81: wrenReallocate (wren_vm.c:222)
==22903==    by 0x124498: wrenNewFiber (wren_value.c:151)
==22903==    by 0x12BEC9: wrenInterpret (wren_vm.c:1435)
==22903==    by 0x12E901: runFile (vm.c:339)
==22903==    by 0x112169: main (main.c:129)

and the following patch (costly) fix the issue:

------------------------------- src/vm/wren_vm.c -------------------------------
index 910e5ca27..eab9a355c 100644
@@ -991,6 +991,7 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, ObjFiber* fiber)
       switch (method->type)
       {
         case METHOD_PRIMITIVE:
+          STORE_FRAME();
           if (method->as.primitive(vm, args))
           {
             // The result is now in the first arg slot. Discard the other
@@ -998,14 +999,13 @@ static WrenInterpretResult runInterpreter(WrenVM* vm, ObjFiber* fiber)
             fiber->stackTop -= numArgs - 1;
           } else {
             // An error, fiber switch, or call frame change occurred.
-            STORE_FRAME();
 
             // If we don't have a fiber to switch to, stop interpreting.
             fiber = vm->fiber;
             if (fiber == NULL) return WREN_RESULT_SUCCESS;
             if (!IS_NULL(fiber->error)) RUNTIME_ERROR();
-            LOAD_FRAME();
           }
+          LOAD_FRAME();
           break;
 
         case METHOD_FOREIGN:

@mhermier mhermier changed the title Compiler issue? Recent Fn change cause use after free. Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant