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

SIGSEGV due to wrong stolen reg value after synchall #4497

Closed
abhinav92003 opened this issue Oct 28, 2020 · 0 comments · Fixed by #4507
Closed

SIGSEGV due to wrong stolen reg value after synchall #4497

abhinav92003 opened this issue Oct 28, 2020 · 0 comments · Fixed by #4507
Assignees

Comments

@abhinav92003
Copy link
Contributor

While working on PR #4491 which adds a synchall to drcachesim while enabling tracing, I encountered a crash due to the stolen reg having a value that doesn't seem like an address.

The segfaulting threads were redirected to the reset exit stub by DR at [1]. The SIGSEGV occurs in the exit stub shown below.

Thread xxx received signal SIGSEGV, Segmentation fault.
0x0000000046fc2458 in ?? ()
(gdb) x/50i 0x46fc2458
=> 0x46fc2458:	stp	x0, x1, [x28]
   0x46fc245c:	mov	x0, #0x4c20                	// #19488
   0x46fc2460:	movk	x0, #0xf7f0, lsl #16
   0x46fc2464:	movk	x0, #0xffff, lsl #32
   0x46fc2468:	ldr	x1, [x28, #64]
   0x46fc246c:	br	x1
   ...

The exit stub expects the stolen reg to be set up already, but it doesn't seem to be -- see x28 below.

(gdb) i r 
x0             0xfffffffffffffffc  -4
x1             0xfffff45bcc80      281474781400192
...
x28            0x30000             196608
...
pc             0x46fc2458          0x46fc2458

The issue was fixed by adding a stolen reg store. Also there's a bug in the reset tests linux.thread-reset and linux.clone-reset, due to which they don't fail on AArch64 when they should.

Filing and fixing these separately from the PR #4491.

[1]: translate_from_synchall_to_dispatch

mc->pc = (app_pc)get_reset_exit_stub(dcontext);

@abhinav92003 abhinav92003 self-assigned this Oct 28, 2020
abhinav92003 added a commit that referenced this issue Oct 28, 2020
Loads stolen reg with TLS base address before entering exit stub after synchall.

Fixes linux.thread-reset and linux.clone-reset tests which should fail on AArch64 due to missing stolen reg load but don't. Adds a new option to reset_at_created_thread_count and use that instead of reset_at_fragment_count. This was required because the latter requires to be tuned to make sure that reset happens when multiple threads are active; otherwise, the failing path is not invoked at all. Also, the ideal reset_at_fragment_count value is different for x86 and AArch64. Using reset_at_created_thread_count makes these tests more robust.

Fixes: #4497
derekbruening added a commit that referenced this issue Oct 30, 2020
Enables the diagnostic option -steal_reg_at_reset for AArch64,
generalizing the existing ARM code.

Switches -reset_at_fragment_count to work in release build by using
the sum of the release stats num_bbs and num_traces.

Adds a release-build syslog for an informational notification when any
reset occurs.

Adds a test of -steal_reg_at_reset.

Includes the key fix for #4497 since it could show up on the new test or with use of this now-enabled option; the full fix for that with better tests for its code path will come in separately.

Issue: #1569, #4497
abhinav92003 added a commit that referenced this issue Nov 1, 2020
Changes reset trigger for these tests to -reset_at_nth_thread. To increase coverage of these tests, we need to trigger reset at a time when multiple threads are active. This is difficult to do using the -reset_at_fragment_count option, which may need to be tuned regularly to its ideal value, which also differs based on platform.

Implements some missing AArch64 pieces for dynamorio_sigreturn.

Relaxes dispatch assert to allow threads to enter dispatch after reset. The current condition allows it only when sending threads native.

Fixes #4496
Fixes #4497
abhinav92003 added a commit that referenced this issue Nov 5, 2020
Implements some missing AArch64 pieces for invoking dynamorio_sigreturn.

Fixes dispatch assert to allow threads to enter dispatch after reset. The current condition allows it only when sending threads native. This affected x86 and AArch64 both.

Fixes a bug in setting xsp for sigreturn on AArchXX.

Skips save_fpstate for AArch64 as that is not needed.

Changes reset trigger for Linux reset tests to -reset_at_nth_thread. To increase coverage of these tests, we need to trigger reset at a time when multiple threads are active. This is difficult to do using the -reset_at_fragment_count option, which may need to be tuned regularly to its ideal value, which also differs based on platform.


Issue #2629 
Fixes #4496
Fixes #4497
gregcawthorne pushed a commit that referenced this issue Nov 28, 2020
Implements some missing AArch64 pieces for invoking dynamorio_sigreturn.

Fixes dispatch assert to allow threads to enter dispatch after reset. The current condition allows it only when sending threads native. This affected x86 and AArch64 both.

Fixes a bug in setting xsp for sigreturn on AArchXX.

Skips save_fpstate for AArch64 as that is not needed.

Changes reset trigger for Linux reset tests to -reset_at_nth_thread. To increase coverage of these tests, we need to trigger reset at a time when multiple threads are active. This is difficult to do using the -reset_at_fragment_count option, which may need to be tuned regularly to its ideal value, which also differs based on platform.


Issue #2629 
Fixes #4496
Fixes #4497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant