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.

Sign up for GitHub

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

Jump to bottom

ASSERT core/nudge.c:521 hthread != INVALID_HANDLE_VALUE #1309

Closed
derekbruening opened this issue Nov 28, 2014 · 2 comments · Fixed by #2840
Closed

ASSERT core/nudge.c:521 hthread != INVALID_HANDLE_VALUE #1309

derekbruening opened this issue Nov 28, 2014 · 2 comments · Fixed by #2840
Assignees
@derekbruening
Labels
Bug-Assert Migrated OpSys-Windows Priority-Medium

Comments

@derekbruening
Copy link
Contributor

From [email protected] on November 04, 2013 23:31:10

$ time ~/Workspace/DrMemory/builds/build_x86_drm_dbg.git/bin/drmemory.exe -debug -handle_leaks_only -callstack_max_frames 40 -- ./out/Debug/unit_tests.exe --gtest_filter=NativeMessagingTest.*
WARNING: using debug DynamoRIO since release not found
Dr.M Dr. Memory version 1.6.1605
Dr.M Running "./out/Debug/unit_tests.exe --gtest_filter=NativeMessagingTest."
Note: Google Test filter = NativeMessagingTest.
[==========] Running 3 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 3 tests from NativeMessagingTest
[ RUN ] NativeMessagingTest.SingleSendMessageRead
[ OK ] NativeMessagingTest.SingleSendMessageRead (5526 ms)
[ RUN ] NativeMessagingTest.SingleSendMessageWrite
[ OK ] NativeMessagingTest.SingleSendMessageWrite (639 ms)
[ RUN ] NativeMessagingTest.EchoConnect
Dr.M Dr. Memory version 1.6.1605
Dr.M Running "C:\Windows\system32\cmd.exe /c "D:\src\chrome\src\chrome\test\data\native_messaging\native_hosts\echo.bat" --parent-window=0 chrome-extension://knldjmfmopnpolahpmmgbagdohdnhkik/ < .\pipe\chrome.nativeMessaging.in.d1194e2a79c1824c > .\pipe\chrome.nativeMessaging.out.d1194e2a79c1824c"
Dr.M WARNING: application is missing line number information.

ASSERT error on core\nudge.c:521 hthread != INVALID_HANDLE_VALUE

It looks like the nudge_internal fail to create a new threads.

Original issue: http://code.google.com/p/dynamorio/issues/detail?id=1309
@derekbruening derekbruening mentioned this issue Feb 18, 2018
Open
@derekbruening derekbruening self-assigned this Feb 19, 2018
@derekbruening
Copy link
Contributor Author

This is causing tests on win8 through win10 to fail: #1432, #2835.

NtCreateThread is returning 0xc0000022 == STATUS_ACCESS_DENIED
It seems we have to use NtCreateThreadEx, added in Vista.
There are some issues there: the kernel creates and frees the stack, which means we need to put multi-arg buffers in a separate alloc and figure out how to free it.

derekbruening added a commit that referenced this issue Feb 19, 2018
@derekbruening
i#1309 win8+ threads: use NtCreateThreadEx for internal nudges
a3e8a82 
Since NtCreateThread returns STATUS_ACCESS_DENIED on win8+, we implement a
solution for internal nudges using NtCreateThreadEx.  We already have
support for varying who frees the stack in NUDGE_NUDGER_FREE_STACK, which
we leverage here.  The other complication is who frees the arg_buf for a
remote process (since we can't easily put it on the stack): again, we
already have NUDGE_FREE_ARG which we use here.

We also fix a bug in cleanup_and_terminate that results in a double-free of
a nudge dstack: a simple 1-byte-bool error where our asm checks all 4
bytes.  A similar asm bug found by inspection with sig_should_swap_stack()
is also fixed.

This fixes internal nudges.  Client threads and external nudges will be
fixed separately.

Issue: #1309, #1432, #2835
@derekbruening derekbruening mentioned this issue Feb 19, 2018
Merged
derekbruening added a commit that referenced this issue Feb 19, 2018
@derekbruening
i#1309 win8+ threads: use NtCreateThreadEx for internal nudges (#2838)
b565a05 
Since NtCreateThread returns STATUS_ACCESS_DENIED on win8+, we implement a
solution for internal nudges using NtCreateThreadEx.  We already have
support for varying who frees the stack in NUDGE_NUDGER_FREE_STACK, which
we leverage here.  The other complication is who frees the arg_buf for a
remote process (since we can't easily put it on the stack): again, we
already have NUDGE_FREE_ARG which we use here.

We also fix a bug in cleanup_and_terminate that results in a double-free of
a nudge dstack: a simple 1-byte-bool error where our asm checks all 4
bytes.  A similar asm bug found by inspection with sig_should_swap_stack()
is also fixed.

This fixes internal nudges.  Client threads and external nudges will be
fixed separately.

Issue: #1309, #1432, #2835
derekbruening added a commit that referenced this issue Feb 19, 2018
@derekbruening
i#1309 win8+ threads: use NtCreateThreadEx for client threads
84bf4af 
For client threads where we want to use our dstack yet we're forced to use
NtCreateThreadEx and its kernel-provided stack, we target a wrapper that
switches stacks and calls the client thread function.  We pass data to the
wrapper on the dstack and pass the dstack as its parameter.

Issue: #1309, #1432, #2835
@derekbruening
Copy link
Contributor Author

We need to use NtCreateThreadEx for 3 things: internal nudges, client threads, and external nudges. For external nudges we should get the tests re-enabled: #120.

@derekbruening derekbruening mentioned this issue Feb 19, 2018
Merged
derekbruening added a commit that referenced this issue Feb 19, 2018
@derekbruening
i#1309 win8+ threads: use NtCreateThreadEx for client threads (#2839)
6b226f7 
For client threads where we want to use our dstack yet we're forced to use
NtCreateThreadEx and its kernel-provided stack, we target a wrapper that
switches stacks and calls the client thread function.  We pass data to the
wrapper on the dstack and pass the dstack as its parameter.

Issue: #1309, #1432, #2835
derekbruening added a commit that referenced this issue Feb 20, 2018
@derekbruening
i#1309 win8+ threads: use NtCreateThreadEx for external nudges
f27c13f 
Changes the libutil nudge code to use NtCreateThreadEx on Win8+.

For a test, enables the client.nudge_test test, part of i#120: enable
.runall tests on Windows.  Here I added a new app win32.infloop to parallel
linux.infloop, rather than launching calc.exe or notepad.exe like we did in
the past (not yet tackling systemwide injection here).  It has a MessageBox
with a title containing the pid, allowing us to use tools/closewnd with a
unique target name for a clean and race-free exit.  win32.infloop also has
a 3-minute timeout to avoid leaving stale processes behind in case of
issues closing it externally.

Revamped runall.cmake to work on Windows, using "ping" to sleep,
updating nudge and close commands, removing stale pid files, etc.

Ported suite code to use run_in_bg on Windows as well as Linux,
but with the pidfile coming from drrun.

Fixes #1309
Fixes #1432
Issue: #120, #1835
@derekbruening derekbruening mentioned this issue Feb 20, 2018
Merged
derekbruening added a commit that referenced this issue Feb 20, 2018
@derekbruening
i#1309 win8+ threads: use NtCreateThreadEx for external nudges (#2840)
b726e89 
Changes the libutil nudge code to use NtCreateThreadEx on Win8+.

For a test, enables the client.nudge_test test, part of i#120: enable
.runall tests on Windows.  Here I added a new app win32.infloop to parallel
linux.infloop, rather than launching calc.exe or notepad.exe like we did in
the past (not yet tackling systemwide injection here).  It has a MessageBox
with a title containing the pid, allowing us to use tools/closewnd with a
unique target name for a clean and race-free exit.  win32.infloop also has
a 3-minute timeout to avoid leaving stale processes behind in case of
issues closing it externally.

Revamped runall.cmake to work on Windows, using "ping" to sleep,
updating nudge and close commands, removing stale pid files, etc.

Ported suite code to use run_in_bg on Windows as well as Linux,
but with the pidfile coming from drrun.

Fixes #1309
Fixes #1432
Issue: #120, #1835
derekbruening added a commit that referenced this issue Feb 21, 2018
@derekbruening
i#2845: fix racy nudge_test test output
4ab2141 
Fixes the nudge_test output to not have two "done" message that result in a
race in runall.cmake.

Issue: #1309
Fixes #2845
@derekbruening derekbruening mentioned this issue Feb 21, 2018
Merged
derekbruening added a commit that referenced this issue Feb 22, 2018
@derekbruening
i#2845: fix racy nudge_test test output (#2846)
4b2eec1 
Fixes the nudge_test output to not have two "done" message that result in a
race in runall.cmake.

Issue: #1309
Fixes #2845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@derekbruening derekbruening

Labels
Bug-Assert Migrated OpSys-Windows Priority-Medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

i#1309 win8+ threads: use NtCreateThreadEx for external nudges DynamoRIO/dynamorio
1 participant
@derekbruening