[m-rev.] for review: Don't use __builtin_setjmp, __builtin_longjmp on AArch64.

[Peter Wang]

There is an issue on Linux/aarch64/glibc where a commit performed on a
thread using the gcc __builtin_longjmp function causes an assertion
failure in pthread_create() if the Mercury runtime is dynamically
linked:

    pthread_create.c:430: start_thread: Assertion `freesize < pd->stackblock_size' failed.

or a crash if the Mercury runtime is statically linked:

    *** Mercury runtime: caught segmentation violation ***
    cause: address not mapped to object

Tested using gcc version 6.3.0 20170516 (Debian 6.3.0-18)

I have not reproduced the issue on Linux/aarch64/musl.

runtime/mercury.h:
    Define MR_USE_GCC_BUILTIN_SETJMP_LONGJMP if we decide to use gcc's
    __builtin_setjmp and __builtin_longjmp functions instead of the
    standard functions.

    Use standard setjmp/longjmp on aarch64.

    Update the comment for MR_builtin_jmp_buf to refer to the GCC manual
    instead of the GCC source code. Use an array of 'intptr_t's instead
    of 'void *'s, to match the manual.
---
 runtime/mercury.h                   | 33 ++++++----
 tests/hard_coded/Mmakefile          |  1 +
 tests/hard_coded/thread_commit.exp  |  7 ++
 tests/hard_coded/thread_commit.exp2 |  1 +
 tests/hard_coded/thread_commit.m    | 99 +++++++++++++++++++++++++++++
 5 files changed, 127 insertions(+), 14 deletions(-)
 create mode 100644 tests/hard_coded/thread_commit.exp
 create mode 100644 tests/hard_coded/thread_commit.exp2
 create mode 100644 tests/hard_coded/thread_commit.m

diff --git a/runtime/mercury.h b/runtime/mercury.h
index 699dc9712..748e38906 100644
--- a/runtime/mercury.h
+++ b/runtime/mercury.h
@@ -1,7 +1,7 @@
 // vim: ts=4 sw=4 expandtab ft=c
 
 // Copyright (C) 1999-2006, 2011 The University of Melbourne.
-// Copyright (C) 2014, 2015-2016, 2018 The Mercury team.
+// Copyright (C) 2014, 2015-2016, 2018, 2022 The Mercury team.
 // This file is distributed under the terms specified in COPYING.LIB.
 
 // mercury.h - This file defines the macros, types, etc. that
@@ -75,12 +75,20 @@
 // The jmp_buf type used by MR_builtin_setjmp()
 // to save the stack context when implementing commits.
 
-#ifdef MR_GNUC
-  // For GCC, we use `__builtin_setjmp' and `__builtin_longjmp'.
-  // These are documented (in gcc/builtins.c in the GCC source code)
-  // as taking for their parameter a pointer to an array of five words.
+#if (MR_GNUC > 2 || (MR_GNUC == 2 && __GNUC_MINOR__ >= 8))
+  // There is some problem with using __builtin_setjmp and __builtin_longjmp on
+  // Linux/glibc/aarch64. When a thread commits using __builtin_longjmp, either
+  // we get an assertion failure in pthread_create or a segmentation fault.
+  #if !defined(__aarch64__) && !defined(MR_DARWIN_SETJMP_WORKAROUND)
+    #define MR_USE_GCC_BUILTIN_SETJMP_LONGJMP
+  #endif
+#endif
 
-  typedef void *MR_builtin_jmp_buf[5];
+#ifdef MR_USE_GCC_BUILTIN_SETJMP_LONGJMP
+  // The jump buffer for GCC `__builtin_setjmp' and `__builtin_longjmp'
+  // functions is documented to be an array of five intptr_t's
+  // (GCC manual section 6.5 Nonlocal gotos).
+  typedef intptr_t MR_builtin_jmp_buf[5];
 #else
   // Otherwise we use the standard jmp_buf type.
   typedef jmp_buf MR_builtin_jmp_buf;
@@ -141,14 +149,11 @@ extern  MR_Word mercury__private_builtin__dummy_var;
 //  2. The call to __builtin_longjmp() must not be in the same
 //      function as the call to __builtin_setjmp().
 
-#if (MR_GNUC > 2 || (MR_GNUC == 2 && __GNUC_MINOR__ >= 8))
-  #ifndef MR_DARWIN_SETJMP_WORKAROUND
-    #define MR_builtin_setjmp(buf)  __builtin_setjmp((buf))
-    #define MR_builtin_longjmp(buf, val)    __builtin_longjmp((buf), (val))
-  #endif
-#endif
-#ifndef MR_builtin_setjmp
-  #define MR_builtin_setjmp(buf)    setjmp((buf))
+#ifdef MR_USE_GCC_BUILTIN_SETJMP_LONGJMP
+  #define MR_builtin_setjmp(buf)        __builtin_setjmp((buf))
+  #define MR_builtin_longjmp(buf, val)  __builtin_longjmp((buf), (val))
+#else
+  #define MR_builtin_setjmp(buf)        setjmp((buf))
   #define MR_builtin_longjmp(buf, val)  longjmp((buf), (val))
 #endif
 
diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile
index 13577f2fe..a67ae2fc3 100644
--- a/tests/hard_coded/Mmakefile
+++ b/tests/hard_coded/Mmakefile
@@ -442,6 +442,7 @@ ORDINARY_PROGS = \
 	test_semaphore \
 	test_tree_bitset \
 	test_yield \
+	thread_commit \
 	thread_sbrk \
 	tim_qual1 \
 	time_test \
diff --git a/tests/hard_coded/thread_commit.exp b/tests/hard_coded/thread_commit.exp
new file mode 100644
index 000000000..c23938f32
--- /dev/null
+++ b/tests/hard_coded/thread_commit.exp
@@ -0,0 +1,7 @@
+Spawning thread...
+Running test on thread...
+Not all ints are odd.
+thread done.
+Running test on main thread...
+Not all ints are odd.
+done.
diff --git a/tests/hard_coded/thread_commit.exp2 b/tests/hard_coded/thread_commit.exp2
new file mode 100644
index 000000000..ff161d491
--- /dev/null
+++ b/tests/hard_coded/thread_commit.exp2
@@ -0,0 +1 @@
+spawn_native not supported.
diff --git a/tests/hard_coded/thread_commit.m b/tests/hard_coded/thread_commit.m
new file mode 100644
index 000000000..86e46bbc9
--- /dev/null
+++ b/tests/hard_coded/thread_commit.m
@@ -0,0 +1,99 @@
+%---------------------------------------------------------------------------%
+% vim: ts=4 sw=4 et ft=mercury
+%---------------------------------------------------------------------------%
+
+% This program checks if performing a commit on a thread causes any issues.
+
+:- module thread_commit.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is cc_multi.
+
+%---------------------------------------------------------------------------%
+%---------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module int.
+:- import_module maybe.
+:- import_module thread.
+:- import_module thread.semaphore.
+
+:- type tree
+    --->    leaf(int)
+    ;       branch(tree, tree).
+
+main(!IO) :-
+    ( if thread.can_spawn_native then
+        semaphore.init(JoinSem, !IO),
+        io.write_string("Spawning thread...\n", !IO),
+        thread.spawn_native(thread_proc(JoinSem), SpawnRes, !IO),
+        (
+            SpawnRes = ok(_Thread),
+            semaphore.wait(JoinSem, !IO),
+            io.write_string("thread done.\n", !IO),
+            % Repeat test on main thread so program doesn't exit immediately.
+            io.write_string("Running test on main thread...\n", !IO),
+            tree_test(!IO),
+            io.write_string("done.\n", !IO)
+        ;
+            SpawnRes = error(Error),
+            io.print_line(Error, !IO)
+        )
+    else
+        io.write_string("spawn_native not supported.\n", !IO)
+    ).
+
+:- pred thread_proc(semaphore::in, thread::in, io::di, io::uo) is cc_multi.
+
+thread_proc(JoinSem, _Thread, !IO) :-
+    cc_multi_equal(!IO),
+    io.write_string("Running test on thread...\n", !IO),
+    tree_test(!IO),
+    semaphore.signal(JoinSem, !IO).
+
+:- pred tree_test(io::di, io::uo) is det.
+
+tree_test(!IO) :-
+    build_tree(5, Tree, 0, _Acc),
+    ( if all_odd(Tree) then
+        io.print_line("All ints are odd.", !IO)
+    else
+        io.print_line("Not all ints are odd.", !IO)
+    ).
+
+:- pred build_tree(int::in, tree::out, int::in, int::out) is det.
+
+build_tree(Height, Tree, !Acc) :-
+    ( if Height =< 0 then
+        Tree = leaf(!.Acc),
+        !:Acc = !.Acc + 2
+    else
+        build_tree(Height - 1, TreeA, !Acc),
+        build_tree(Height - 2, TreeB, !Acc),
+        Tree = branch(TreeA, TreeB)
+    ).
+
+:- pred all_odd(tree::in) is semidet.
+
+all_odd(Tree) :-
+    % This may perform a commit.
+    all [X] (
+        leaf(Leaf, Tree)
+    =>
+        not(int.even(Leaf))
+    ).
+
+:- pred leaf(int::out, tree::in) is nondet.
+
+leaf(Leaf, Tree) :-
+    (
+        Tree = leaf(Leaf)
+    ;
+        Tree = branch(TreeA, TreeB),
+        ( leaf(Leaf, TreeA)
+        ; leaf(Leaf, TreeB)
+        )
+    ).
-- 
2.35.1