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

emu_stop segfault #65

Closed
SeanHeelan opened this issue Aug 28, 2015 · 13 comments
Closed

emu_stop segfault #65

SeanHeelan opened this issue Aug 28, 2015 · 13 comments
Labels

Comments

@SeanHeelan
Copy link
Contributor

I'm not sure if this is due to a misunderstanding on my behalf of how this API is supposed to be used, but earlier today when playing around with something I found the following causes a NULL pointer dereference.

import unicorn
ADDR = 0x10101000
mu = unicorn.Uc(unicorn.UC_ARCH_X86, unicorn.UC_MODE_32)
mu.mem_map(ADDR, 1024 * 4)
mu.mem_write(ADDR, b'\x41')
mu.emu_start(ADDR, ADDR + 1, count=1)
mu.emu_stop()

The crash details are as follows

* thread #1: tid = 0xd25de, 0x000000010295564c libunicorn.dylib`cpu_exit(cpu=0x0000000000000000) + 12 at cpu.c:112, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xac)
    frame #0: 0x000000010295564c libunicorn.dylib`cpu_exit(cpu=0x0000000000000000) + 12 at cpu.c:112
   109
   110  void cpu_exit(CPUState *cpu)
   111  {
-> 112      cpu->exit_request = 1;
   113      cpu->tcg_exit_req = 1;
   114  }
   115
(lldb) bt
* thread #1: tid = 0xd25de, 0x000000010295564c libunicorn.dylib`cpu_exit(cpu=0x0000000000000000) + 12 at cpu.c:112, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xac)
  * frame #0: 0x000000010295564c libunicorn.dylib`cpu_exit(cpu=0x0000000000000000) + 12 at cpu.c:112
    frame #1: 0x0000000102ff7d99 libunicorn.dylib`uc_emu_stop(handle=4311769600) + 73 at uc.c:495
    frame #2: 0x00007fff8c9d3f44 libffi.dylib`ffi_call_unix64 + 76
    frame #3: 0x00007fff8c9d4781 libffi.dylib`ffi_call + 853
    frame #4: 0x00000001007c8762 _ctypes.so`_ctypes_callproc + 874
    frame #5: 0x00000001007c2bad _ctypes.so`___lldb_unnamed_function18$$_ctypes.so + 1092
    frame #6: 0x000000010000e2ac Python`PyObject_Call + 99
    frame #7: 0x000000010008ac00 Python`PyEval_EvalFrameEx + 11370
    frame #8: 0x000000010008e60e Python`___lldb_unnamed_function1477$$Python + 262
    frame #9: 0x000000010008b3e3 Python`PyEval_EvalFrameEx + 13389
    frame #10: 0x0000000100087d62 Python`PyEval_EvalCodeEx + 1413
    frame #11: 0x00000001000877d7 Python`PyEval_EvalCode + 54
    frame #12: 0x00000001000a77bd Python`___lldb_unnamed_function1599$$Python + 53
    frame #13: 0x00000001000a7860 Python`PyRun_FileExFlags + 133
    frame #14: 0x00000001000a73fd Python`PyRun_SimpleFileExFlags + 769
    frame #15: 0x00000001000b8b23 Python`Py_Main + 3051
    frame #16: 0x00007fff8ed0f5c9 libdyld.dylib`start + 1
(lldb) f 1
frame #1: 0x0000000102ff7d99 libunicorn.dylib`uc_emu_stop(handle=4311769600) + 73 at uc.c:495
   492
   493      uc->stop_request = true;
   494      // exit the current TB
-> 495      cpu_exit(uc->current_cpu);
   496
   497      return UC_ERR_OK;
   498  }
(lldb) p uc->current_cpu
(CPUState *) $0 = 0x0000000000000000

I'm not familiar enough with Unicorn yet to guess at why current_cpu would be 0x0.

@aquynh
Copy link
Member

aquynh commented Aug 28, 2015

actually there is no reason to call emu_stop() because after emu_start() returns, emulation already stopped.

this is a bug anyway, so please could you send a PR to put your script under regress/ directory?

thanks.

@lunixbochs
Copy link
Contributor

It makes sense to me to call emu_stop from a debugger or a callback.

@aquynh
Copy link
Member

aquynh commented Aug 28, 2015

yes, we already had a bunch of samples calling emu_stop() from callbacks.
but there is no reason to do this after emu_start() returns.

@gaffe23
Copy link
Contributor

gaffe23 commented Aug 28, 2015

I ran into this as well. Other than callbacks, when is it valid to call
emu_stop()?
Le 28 août 2015 06:45, "Nguyen Anh Quynh" notifications@github.com a
écrit :

yes, we already had a bunch of samples calling emu_stop from callbacks.
but there is no reason to do this after emu_start() returns.


Reply to this email directly or view it on GitHub
#65 (comment)
.

@aquynh
Copy link
Member

aquynh commented Aug 28, 2015

the only place to use emu_stop() is from inside the callbacks.

note that for emu_start(), there are 3 choices to stop emulation: by until-address, by timeout and by number of instructions emulated.

@aquynh
Copy link
Member

aquynh commented Aug 28, 2015

actually this is already documented for uc_emu_stop() inside unicorn.h at https://github.com/unicorn-engine/unicorn/blob/master/include/unicorn/unicorn.h#L343

@lunixbochs
Copy link
Contributor

It looks roughly safe to call from a separate thread as well, as it just registers a request to stop by setting two integers. I care about this because I might want to force the CPU to arbitrarily stop when I'm not inside a callback.

@aquynh
Copy link
Member

aquynh commented Aug 28, 2015

ah yes, calling from another thread to stop emulation in the middle is also another choice.

@SeanHeelan
Copy link
Contributor Author

PR sent.

Btw, is there some mechanism right now for running regression tests or other tests automatically? On a similar note, is there a mechanism or a desired pattern you wish to follow for Python unit tests? I encountered a minor bug (which I've yet to report) and I was going to send along a fix but I can't see any existing Python unit tests so I wasn't exactly sure where you'd prefer them.

(If this requires its own discussion thread let me know and I'll start one)

@JonathonReinhart
Copy link
Member

It looks roughly safe to call from a separate thread as well, as it just registers a request to stop by setting two integers.

I haven't yet looked at the code, but if we're considering calling this from another thread, we might have to worry about consistency. In other words, will Unicorn be confused or behave poorly if it "sees" those two integers in an intermediate state, where one has been changed, and the other has not?

@lunixbochs
Copy link
Contributor

No, I don't believe so.

aquynh added a commit that referenced this issue Aug 29, 2015
Added a regression script for issue #65
@aquynh
Copy link
Member

aquynh commented Aug 29, 2015

this issue is fixed now. please confirm, thanks.

@SeanHeelan
Copy link
Contributor Author

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants