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

i#3336: increase determinism of linux.syscall_pwait test. #3337

Merged
merged 21 commits into from
Jan 16, 2019

Conversation

hgreving2304
Copy link

@hgreving2304 hgreving2304 commented Jan 9, 2019

Stabilize linux.syscall_pwait test that currently (sporadically-) fails on Android.

Checks for pending signals in emulation of p* versions of system calls and aborts call if necessary.

Changes linux.syscall_pwait to using pthreads and cond_vars.

Adds subtest to check signal in "pending" state as well as signals arriving during system call execution.

Fixes #3336, #3344

Attempt to stabilize linux.syscall_pwait test that currently (sporadically-) fails on Android.

Add simple shared synch variable among processes, decrease excessive sleep timeout.

Fixes #3336
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporarily undoing the LGTM since I think it best to resolve this ARM synchronization issue.

Hendrik Greving added 8 commits January 10, 2019 11:27
It also exposed the fact that handle_pre_extended_syscall_sigmasks() does not check_signals_pending() causing the signals not being delivered.
Re-adding the "signal received" messages to the test since they are correctly received now under DR.
@hgreving2304
Copy link
Author

PTAL.
The pending signals seem like they don't get delivered in AArch64. I'd be interested in if they do get delivered when not running under DR. I don't have an explanation why. I can/would add XXX comments to the test mentioning this.

core/unix/signal_linux.c Outdated Show resolved Hide resolved
core/unix/os.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
Hendrik Greving added 3 commits January 11, 2019 11:29
Fix i#3336 by aborting system call if a signal is pending.
Adds another subtest that sends signals that are not in 'pending'.
@hgreving2304
Copy link
Author

PTAL!

Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler wouldn't move a store across a system call

How do you know for sure this store won't be moved? I am pretty sure we have seen the compiler do nearly identical things in other cases. A developer's intuition on what the compiler might do is not enough, independent of any coding standards. Can you point to something in the language specification? Even if there were some guarantee in some situation, it would require a comment and probably still not be worth leaving out the synch that all other cases require.

core/unix/os.c Show resolved Hide resolved
core/unix/os.c Outdated Show resolved Hide resolved
core/unix/os.c Show resolved Hide resolved
core/unix/os.c Outdated Show resolved Hide resolved
core/unix/signal.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Outdated Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Show resolved Hide resolved
suite/tests/linux/syscall_pwait.c Show resolved Hide resolved
core/unix/signal.c Outdated Show resolved Hide resolved
core/unix/signal.c Outdated Show resolved Hide resolved
core/unix/signal.c Outdated Show resolved Hide resolved
core/unix/signal_linux.c Outdated Show resolved Hide resolved
suite/tests/CMakeLists.txt Show resolved Hide resolved
Removed handle_sigreturn non-rt path restoring pre-syscall mask.
@hgreving2304
Copy link
Author

s/Simply/Simplify/

@hgreving2304
Copy link
Author

run armtests

core/unix/signal.c Outdated Show resolved Hide resolved
core/unix/signal.c Outdated Show resolved Hide resolved
core/unix/signal.c Outdated Show resolved Hide resolved
core/unix/signal_linux.c Outdated Show resolved Hide resolved
Hendrik Greving added 2 commits January 16, 2019 12:53
@hgreving2304 hgreving2304 merged commit aedafbf into master Jan 16, 2019
@hgreving2304 hgreving2304 deleted the i3336-syscall_pwait-robus-process-dir branch January 16, 2019 22:09
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

Successfully merging this pull request may close these issues.

2 participants