diff mbox series

[v1] userfaultfd: selftests: infinite loop in faulting_process

Message ID 20220613233321.431282-1-edliaw@google.com
State Accepted
Commit 50b0f797cab6cb7dff418777e1acf82dfd3568f8
Headers show
Series [v1] userfaultfd: selftests: infinite loop in faulting_process | expand

Commit Message

Edward Liaw June 13, 2022, 11:33 p.m. UTC
On Android this test is getting stuck in an infinite loop due to
indeterminate behavior:

The local variables steps and signalled were being reset to 1 and 0
respectively after every jump back to sigsetjmp by siglongjmp in the
signal handler. The test was incrementing them and expecting them to
retain their incremented values. The documentation for siglongjmp says:

All accessible objects have values as of the time sigsetjmp() was
called, except that the values of objects of automatic storage duration
which are local to the function containing the invocation of the
corresponding sigsetjmp() which do not have volatile-qualified type and
which are changed between the sigsetjmp() invocation and siglongjmp()
call are indeterminate.

Tagging steps and signalled with volatile enabled the test to pass.

Signed-off-by: Edward Liaw <edliaw@google.com>
---
 tools/testing/selftests/vm/userfaultfd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Axel Rasmussen June 14, 2022, 6:31 p.m. UTC | #1
Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>

On Mon, Jun 13, 2022 at 4:33 PM Edward Liaw <edliaw@google.com> wrote:
>
> On Android this test is getting stuck in an infinite loop due to
> indeterminate behavior:
>
> The local variables steps and signalled were being reset to 1 and 0
> respectively after every jump back to sigsetjmp by siglongjmp in the
> signal handler. The test was incrementing them and expecting them to
> retain their incremented values. The documentation for siglongjmp says:
>
> All accessible objects have values as of the time sigsetjmp() was
> called, except that the values of objects of automatic storage duration
> which are local to the function containing the invocation of the
> corresponding sigsetjmp() which do not have volatile-qualified type and
> which are changed between the sigsetjmp() invocation and siglongjmp()
> call are indeterminate.
>
> Tagging steps and signalled with volatile enabled the test to pass.
>
> Signed-off-by: Edward Liaw <edliaw@google.com>
> ---
>  tools/testing/selftests/vm/userfaultfd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
> index 0bdfc1955229..2b2a68722ae1 100644
> --- a/tools/testing/selftests/vm/userfaultfd.c
> +++ b/tools/testing/selftests/vm/userfaultfd.c
> @@ -931,7 +931,7 @@ static int faulting_process(int signal_test)
>         unsigned long split_nr_pages;
>         unsigned long lastnr;
>         struct sigaction act;
> -       unsigned long signalled = 0;
> +       volatile unsigned long signalled = 0;
>
>         split_nr_pages = (nr_pages + 1) / 2;
>
> @@ -946,7 +946,7 @@ static int faulting_process(int signal_test)
>         }
>
>         for (nr = 0; nr < split_nr_pages; nr++) {
> -               int steps = 1;
> +               volatile int steps = 1;
>                 unsigned long offset = nr * page_size;
>
>                 if (signal_test) {
> --
> 2.36.1.476.g0c4daa206d-goog
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 0bdfc1955229..2b2a68722ae1 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -931,7 +931,7 @@  static int faulting_process(int signal_test)
 	unsigned long split_nr_pages;
 	unsigned long lastnr;
 	struct sigaction act;
-	unsigned long signalled = 0;
+	volatile unsigned long signalled = 0;
 
 	split_nr_pages = (nr_pages + 1) / 2;
 
@@ -946,7 +946,7 @@  static int faulting_process(int signal_test)
 	}
 
 	for (nr = 0; nr < split_nr_pages; nr++) {
-		int steps = 1;
+		volatile int steps = 1;
 		unsigned long offset = nr * page_size;
 
 		if (signal_test) {