From 0497e502e284a6989727d9250a18486f25f781f0 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 15:51:33 -0500 Subject: [PATCH 01/37] i#5256 sigprocmask: add tests for MacOS. Enables the linux.sigaction test on MacOS. Also enables tmate debugging for MacOS. Issue: #5256 --- .github/workflows/ci-osx.yml | 4 ++++ suite/tests/CMakeLists.txt | 1 + suite/tests/linux/sigaction.c | 44 +++++++++++++++++------------------ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index 2bea967b8f1..bb0579eee2b 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,6 +81,10 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} + - name: Setup tmate session + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index b6ab9ef827c..cc68c7b0c2a 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4789,6 +4789,7 @@ if (APPLE) code_api|api.dis code_api|api.ir-static code_api|api.drdecode + code_api|linux.sigaction PROPERTIES LABELS OSX) if (DEBUG) # FIXME i#1806: fails in release set_tests_properties(code_api|client.flush PROPERTIES LABELS OSX) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 7b783f464ff..658dea3a56d 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -115,56 +115,54 @@ set_sigaction_handler(int sig, void *action) assert(rc == 0); } -#if !defined(MACOS) static void -test_rt_sigprocmask() +test_sigprocmask(long sysnum) { uint64 new = 0xf00d, old, original; /* Save original sigprocmask. Both return the current sigprocmask. */ - assert(syscall(SYS_rt_sigprocmask, SIG_SETMASK, NULL, &original, + assert(syscall(sysnum, SIG_SETMASK, NULL, &original, /*sizeof(kernel_sigset_t)*/ 8) == 0); - assert(syscall(SYS_rt_sigprocmask, ~0, NULL, &original, 8) == 0); + assert(syscall(sysnum, ~0, NULL, &original, 8) == 0); /* EFAULT cases. */ - assert(syscall(SYS_rt_sigprocmask, ~0, NULL, 0x123, 8) == -1); + assert(syscall(sysnum, ~0, NULL, 0x123, 8) == -1); assert(errno == EFAULT); - assert(syscall(SYS_rt_sigprocmask, SIG_BLOCK, 0x123, NULL, 8) == -1); + assert(syscall(sysnum, SIG_BLOCK, 0x123, NULL, 8) == -1); assert(errno == EFAULT); - assert(syscall(SYS_rt_sigprocmask, SIG_BLOCK, NULL, 0x123, 8) == -1); + assert(syscall(sysnum, SIG_BLOCK, NULL, 0x123, 8) == -1); assert(errno == EFAULT); /* Bad new sigmask EFAULT gets reported before bad 'how' EINVAL. */ - assert(syscall(SYS_rt_sigprocmask, ~0, 0x123, NULL, 8) == -1); + assert(syscall(sysnum, ~0, 0x123, NULL, 8) == -1); assert(errno == EFAULT); /* EFAULT due to unwritable address. */ - assert(syscall(SYS_rt_sigprocmask, SIG_BLOCK, NULL, test_rt_sigprocmask, 8) == -1); + assert(syscall(sysnum, SIG_BLOCK, NULL, test_sigprocmask, 8) == -1); assert(errno == EFAULT); /* EINVAL cases. */ /* Bad size. */ - assert(syscall(SYS_rt_sigprocmask, SIG_SETMASK, &new, NULL, 7) == -1); + assert(syscall(sysnum, SIG_SETMASK, &new, NULL, 7) == -1); assert(errno == EINVAL); /* Bad size EINVAL gets reported before bad new sigmask EFAULT. */ - assert(syscall(SYS_rt_sigprocmask, SIG_SETMASK, 0x123, NULL, 7) == -1); + assert(syscall(sysnum, SIG_SETMASK, 0x123, NULL, 7) == -1); assert(errno == EINVAL); /* Bad 'how' arg. */ - assert(syscall(SYS_rt_sigprocmask, ~0, &new, NULL, 8) == -1); + assert(syscall(sysnum, ~0, &new, NULL, 8) == -1); assert(errno == EINVAL); - assert(syscall(SYS_rt_sigprocmask, SIG_SETMASK + 1, &new, NULL, 8) == -1); + assert(syscall(sysnum, SIG_SETMASK + 1, &new, NULL, 8) == -1); assert(errno == EINVAL); /* Bad 'how' EINVAL gets reported before bad old sigset EFAULT. */ - assert(syscall(SYS_rt_sigprocmask, ~0, &new, 0x123, 8) == -1); + assert(syscall(sysnum, ~0, &new, 0x123, 8) == -1); assert(errno == EINVAL); /* Success. */ - assert(syscall(SYS_rt_sigprocmask, ~0, NULL, NULL, 8) == 0); - assert(syscall(SYS_rt_sigprocmask, SIG_SETMASK, &new, NULL, 8) == 0); - assert(syscall(SYS_rt_sigprocmask, ~0, NULL, &old, 8) == 0); + assert(syscall(sysnum, ~0, NULL, NULL, 8) == 0); + assert(syscall(sysnum, SIG_SETMASK, &new, NULL, 8) == 0); + assert(syscall(sysnum, ~0, NULL, &old, 8) == 0); assert(new == old); /* Restore original sigprocmask. */ - assert(syscall(SYS_rt_sigprocmask, SIG_SETMASK, &original, NULL, 8) == 0); + assert(syscall(sysnum, SIG_SETMASK, &original, NULL, 8) == 0); } -#endif #if !defined(MACOS) && !defined(X64) static void @@ -214,8 +212,10 @@ int main(int argc, char **argv) { test_query(SIGTERM); -#if !defined(MACOS) - test_rt_sigprocmask(); +#ifdef MACOS + test_sigprocmask(SYS_sigprocmask); +#else + test_sigprocmask(SYS_rt_sigprocmask); #endif #if !defined(MACOS) && !defined(X64) test_non_rt_sigaction(SIGPIPE); @@ -227,4 +227,4 @@ main(int argc, char **argv) print("Sending SIGTERM second time\n"); kill(getpid(), SIGTERM); print("Should not be reached\n"); -} +} \ No newline at end of file From 268e105f508b3f072f53330f492b2becdc5cc86f Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 16:41:45 -0500 Subject: [PATCH 02/37] Debug stuff. --- suite/tests/linux/sigaction.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 658dea3a56d..7f3008c3811 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -120,8 +120,15 @@ test_sigprocmask(long sysnum) { uint64 new = 0xf00d, old, original; /* Save original sigprocmask. Both return the current sigprocmask. */ + asm volatile("nop\n\t" + "nop\n\t" + "nop\n\t" + "nop\n\t" + "mov %[constant], %%rdx\n\t" + ::[constant] "i"(0xdeadbeef):"rdx"); assert(syscall(sysnum, SIG_SETMASK, NULL, &original, /*sizeof(kernel_sigset_t)*/ 8) == 0); +#ifdef REAL assert(syscall(sysnum, ~0, NULL, &original, 8) == 0); /* EFAULT cases. */ @@ -162,6 +169,7 @@ test_sigprocmask(long sysnum) /* Restore original sigprocmask. */ assert(syscall(sysnum, SIG_SETMASK, &original, NULL, 8) == 0); +#endif } #if !defined(MACOS) && !defined(X64) @@ -211,6 +219,9 @@ test_non_rt_sigaction(int sig) int main(int argc, char **argv) { +#ifndef REAL + test_sigprocmask(SYS_sigprocmask); +#else test_query(SIGTERM); #ifdef MACOS test_sigprocmask(SYS_sigprocmask); @@ -227,4 +238,5 @@ main(int argc, char **argv) print("Sending SIGTERM second time\n"); kill(getpid(), SIGTERM); print("Should not be reached\n"); -} \ No newline at end of file +#endif +} From 40ff89050f1b072a8889351f6591922ff4c7bd7c Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 20:05:21 -0500 Subject: [PATCH 03/37] Fix sigaction test for Mac. --- suite/tests/linux/sigaction.c | 94 ++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 39 deletions(-) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 7f3008c3811..df09d195cc6 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -115,61 +115,77 @@ set_sigaction_handler(int sig, void *action) assert(rc == 0); } +static int +make_sigprocmask(int how, void *sigset, void *oldsigset, int size) +{ +#if defined(MACOS) + /* The raw syscall does not work on Mac. */ + return sigprocmask(how, sigset, oldsigset); +#else + return syscall(SYS_rt_sigprocmask, how, sigset, oldsigset, size); +#endif +} + static void -test_sigprocmask(long sysnum) +test_sigprocmask() { +#if defined(MACOS) + sigset_t new = 0xf00d, old, original; +#else uint64 new = 0xf00d, old, original; +#endif + /* Save original sigprocmask. Both return the current sigprocmask. */ - asm volatile("nop\n\t" - "nop\n\t" - "nop\n\t" - "nop\n\t" - "mov %[constant], %%rdx\n\t" - ::[constant] "i"(0xdeadbeef):"rdx"); - assert(syscall(sysnum, SIG_SETMASK, NULL, &original, - /*sizeof(kernel_sigset_t)*/ 8) == 0); -#ifdef REAL - assert(syscall(sysnum, ~0, NULL, &original, 8) == 0); + assert(make_sigprocmask(SIG_SETMASK, NULL, &original, + /*sizeof(kernel_sigset_t)*/ 8) == 0); + assert(make_sigprocmask(~0, NULL, &original, 8) == 0); /* EFAULT cases. */ - assert(syscall(sysnum, ~0, NULL, 0x123, 8) == -1); + /* TODO: bad old sigset does not cause EFAULT on Mac. Fix before + * submitting. + */ + assert(make_sigprocmask(~0, NULL, (void *)0x123, 8) == -1); assert(errno == EFAULT); - assert(syscall(sysnum, SIG_BLOCK, 0x123, NULL, 8) == -1); + assert(make_sigprocmask(SIG_BLOCK, (void *)0x123, NULL, 8) == -1); assert(errno == EFAULT); - assert(syscall(sysnum, SIG_BLOCK, NULL, 0x123, 8) == -1); + assert(make_sigprocmask(SIG_BLOCK, NULL, (void *)0x123, 8) == -1); assert(errno == EFAULT); /* Bad new sigmask EFAULT gets reported before bad 'how' EINVAL. */ - assert(syscall(sysnum, ~0, 0x123, NULL, 8) == -1); + assert(make_sigprocmask(~0, (void *)0x123, NULL, 8) == -1); assert(errno == EFAULT); /* EFAULT due to unwritable address. */ - assert(syscall(sysnum, SIG_BLOCK, NULL, test_sigprocmask, 8) == -1); + assert(make_sigprocmask(SIG_BLOCK, NULL, test_sigprocmask, 8) == -1); assert(errno == EFAULT); /* EINVAL cases. */ +#if defined(MACOS) + /* Size arg is ignored on MacOS. */ +#else /* Bad size. */ - assert(syscall(sysnum, SIG_SETMASK, &new, NULL, 7) == -1); + assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 7) == -1); assert(errno == EINVAL); /* Bad size EINVAL gets reported before bad new sigmask EFAULT. */ - assert(syscall(sysnum, SIG_SETMASK, 0x123, NULL, 7) == -1); + assert(make_sigprocmask(SIG_SETMASK, (void *)0x123, NULL, 7) == -1); assert(errno == EINVAL); + +#endif /* Bad 'how' arg. */ - assert(syscall(sysnum, ~0, &new, NULL, 8) == -1); + assert(make_sigprocmask(~0, &new, NULL, 8) == -1); assert(errno == EINVAL); - assert(syscall(sysnum, SIG_SETMASK + 1, &new, NULL, 8) == -1); + assert(make_sigprocmask(SIG_SETMASK + 1, &new, NULL, 8) == -1); assert(errno == EINVAL); /* Bad 'how' EINVAL gets reported before bad old sigset EFAULT. */ - assert(syscall(sysnum, ~0, &new, 0x123, 8) == -1); + assert(make_sigprocmask(~0, &new, (void *)0x123, 8) == -1); assert(errno == EINVAL); /* Success. */ - assert(syscall(sysnum, ~0, NULL, NULL, 8) == 0); - assert(syscall(sysnum, SIG_SETMASK, &new, NULL, 8) == 0); - assert(syscall(sysnum, ~0, NULL, &old, 8) == 0); + assert(make_sigprocmask(~0, NULL, NULL, 8) == 0); + assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 8) == 0); + assert(make_sigprocmask(~0, NULL, &old, 8) == 0); assert(new == old); /* Restore original sigprocmask. */ - assert(syscall(sysnum, SIG_SETMASK, &original, NULL, 8) == 0); -#endif + assert(make_sigprocmask(SIG_SETMASK, &original, NULL, 8) == 0); } #if !defined(MACOS) && !defined(X64) @@ -195,9 +211,13 @@ test_non_rt_sigaction(int sig) memset((void *)&old_act, 0xff, sizeof(old_act)); rc = dynamorio_syscall(SYS_sigaction, 3, sig, NULL, &old_act); assert(rc == 0 && old_act.handler == first_act.handler && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ +# if defined(MACOS) + *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); +# else *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); +# endif /* Test with a new action. */ memset((void *)&old_act, 0xff, sizeof(old_act)); @@ -205,9 +225,13 @@ test_non_rt_sigaction(int sig) new_act.handler = (void (*)(int, siginfo_t *, void *))SIG_IGN; rc = dynamorio_syscall(SYS_sigaction, 3, sig, &new_act, &old_act); assert(rc == 0 && old_act.handler == first_act.handler && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ +# if defined(MACOS) + *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); +# else *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); +# endif /* Clear handler */ memset((void *)&new_act, 0, sizeof(new_act)); @@ -219,15 +243,8 @@ test_non_rt_sigaction(int sig) int main(int argc, char **argv) { -#ifndef REAL - test_sigprocmask(SYS_sigprocmask); -#else test_query(SIGTERM); -#ifdef MACOS - test_sigprocmask(SYS_sigprocmask); -#else - test_sigprocmask(SYS_rt_sigprocmask); -#endif + test_sigprocmask(); #if !defined(MACOS) && !defined(X64) test_non_rt_sigaction(SIGPIPE); #endif @@ -238,5 +255,4 @@ main(int argc, char **argv) print("Sending SIGTERM second time\n"); kill(getpid(), SIGTERM); print("Should not be reached\n"); -#endif } From db2f04f32f8caa35c3f4df0ba86aa0686f30653b Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 20:47:32 -0500 Subject: [PATCH 04/37] Fix mac test bug. Added the ifdef in the wrong routine. --- core/unix/os.c | 1 - suite/tests/linux/sigaction.c | 32 ++++++++++++++++---------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/core/unix/os.c b/core/unix/os.c index db89d26cb3f..1bd87317b78 100644 --- a/core/unix/os.c +++ b/core/unix/os.c @@ -7331,7 +7331,6 @@ pre_system_call(dcontext_t *dcontext) break; } case IF_MACOS_ELSE(SYS_sigprocmask, SYS_rt_sigprocmask): { /* 175 */ - /* TODO i#5256: Fx this path and enable linux.sigaction on MacOS. */ /* in /usr/src/linux/kernel/signal.c: asmlinkage long sys_rt_sigprocmask(int how, sigset_t *set, sigset_t *oset, diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index df09d195cc6..5c61dfac806 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -78,9 +78,13 @@ test_query(int sig) memset((void *)&old_act, 0xff, sizeof(old_act)); rc = sigaction(sig, NULL, &old_act); assert(rc == 0 && old_act.sa_sigaction == first_act.sa_sigaction && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ +#if defined(MACOS) + *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); +#else *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); +#endif /* Test with a new action. */ memset((void *)&old_act, 0xff, sizeof(old_act)); @@ -89,9 +93,13 @@ test_query(int sig) sigemptyset(&new_act.sa_mask); rc = sigaction(sig, &new_act, &old_act); assert(rc == 0 && old_act.sa_sigaction == first_act.sa_sigaction && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ +#if defined(MACOS) + *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); +#else *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); +#endif /* Test pattern from i#1984 issue and ensure no assert. */ memset(&new_act, 0, sizeof(new_act)); @@ -211,13 +219,9 @@ test_non_rt_sigaction(int sig) memset((void *)&old_act, 0xff, sizeof(old_act)); rc = dynamorio_syscall(SYS_sigaction, 3, sig, NULL, &old_act); assert(rc == 0 && old_act.handler == first_act.handler && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ -# if defined(MACOS) - *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); -# else + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); -# endif /* Test with a new action. */ memset((void *)&old_act, 0xff, sizeof(old_act)); @@ -225,13 +229,9 @@ test_non_rt_sigaction(int sig) new_act.handler = (void (*)(int, siginfo_t *, void *))SIG_IGN; rc = dynamorio_syscall(SYS_sigaction, 3, sig, &new_act, &old_act); assert(rc == 0 && old_act.handler == first_act.handler && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ -# if defined(MACOS) - *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); -# else + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); -# endif /* Clear handler */ memset((void *)&new_act, 0, sizeof(new_act)); From 27ef6e655a3b4ddaf93e95801e2ab5d4e0e63a0d Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 20:59:54 -0500 Subject: [PATCH 05/37] Add a native run for linux.sigaction --- suite/tests/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index cc68c7b0c2a..79327eb641c 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4142,6 +4142,9 @@ if (UNIX) tobuild(linux.sigplain110 linux/sigplain110.c) tobuild(linux.sigplain111 linux/sigplain111.c) tobuild(linux.sigaction linux/sigaction.c) + # We want a native run to verify our assumptions of kernel behaviour. + torunonly_native(linux.sigaction.native linux.sigaction + "" linux/sigaction.c "") if (LINUX) tobuild(linux.syscall_pwait linux/syscall_pwait.cpp) link_with_pthread(linux.syscall_pwait) From 5e6c372efb62ba6b7fbd20ee534fe81fa7c416b3 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 21:09:44 -0500 Subject: [PATCH 06/37] Enable linux.sigaction.native on MacOS. --- suite/tests/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 79327eb641c..6b8339a412f 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4793,6 +4793,7 @@ if (APPLE) code_api|api.ir-static code_api|api.drdecode code_api|linux.sigaction + code_api|linux.sigaction.native PROPERTIES LABELS OSX) if (DEBUG) # FIXME i#1806: fails in release set_tests_properties(code_api|client.flush PROPERTIES LABELS OSX) From 4723ba737c8a92e17d6f4303e9a5c8606dbd9492 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 21:46:50 -0500 Subject: [PATCH 07/37] Fix test names --- suite/tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 6b8339a412f..6822630d061 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4143,7 +4143,7 @@ if (UNIX) tobuild(linux.sigplain111 linux/sigplain111.c) tobuild(linux.sigaction linux/sigaction.c) # We want a native run to verify our assumptions of kernel behaviour. - torunonly_native(linux.sigaction.native linux.sigaction + torunonly_native(code_api|linux.sigaction.native linux.sigaction "" linux/sigaction.c "") if (LINUX) tobuild(linux.syscall_pwait linux/syscall_pwait.cpp) From f7573e225a2d8126a848a93815c84ba86b94cbf4 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 22:03:53 -0500 Subject: [PATCH 08/37] Fix handling of bad old sigset on Mac. --- core/unix/signal.c | 8 ++++++-- suite/tests/linux/sigaction.c | 19 ++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index b8311991995..5640ddd6af4 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2297,7 +2297,7 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, { thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field; int i; - kernel_sigset_t safe_set, safe_old_set; + kernel_sigset_t safe_set; /* Some code uses this syscall to check whether the given address is * readable. E.g. * github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/ @@ -2323,7 +2323,11 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, *error_code = EINVAL; return false; } - if (oset != NULL) { + /* On MacOS, sigprocmask does not fail if the old sigset is not readable + * or not writeable. + */ + if (IF_MACOS_ELSE(false, oset != NULL)) { + kernel_sigset_t safe_old_set; /* Old sigset should be writable too. */ if (!d_r_safe_read(oset, sizeof(safe_old_set), &safe_old_set) || !safe_write_ex(oset, sizeof(safe_old_set), &safe_old_set, NULL)) { diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 5c61dfac806..4e993ae2a09 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -152,18 +152,31 @@ test_sigprocmask() /* TODO: bad old sigset does not cause EFAULT on Mac. Fix before * submitting. */ - assert(make_sigprocmask(~0, NULL, (void *)0x123, 8) == -1); + /* sigprocmask on MacOS does not fail when the old sigset is not + * readable or not writeable. + */ + int expected_result_bad_old_sigset = IF_MACOS_ELSE(0, -1); + assert(make_sigprocmask(~0, NULL, (void *)0x123, 8) == + expected_result_bad_old_sigset); +#if !defined(MACOS) assert(errno == EFAULT); +#endif assert(make_sigprocmask(SIG_BLOCK, (void *)0x123, NULL, 8) == -1); assert(errno == EFAULT); - assert(make_sigprocmask(SIG_BLOCK, NULL, (void *)0x123, 8) == -1); + assert(make_sigprocmask(SIG_BLOCK, NULL, (void *)0x123, 8) == + expected_result_bad_old_sigset); +#if !defined(MACOS) assert(errno == EFAULT); +#endif /* Bad new sigmask EFAULT gets reported before bad 'how' EINVAL. */ assert(make_sigprocmask(~0, (void *)0x123, NULL, 8) == -1); assert(errno == EFAULT); /* EFAULT due to unwritable address. */ - assert(make_sigprocmask(SIG_BLOCK, NULL, test_sigprocmask, 8) == -1); + assert(make_sigprocmask(SIG_BLOCK, NULL, test_sigprocmask, 8) == + expected_result_bad_old_sigset); +#if !defined(MACOS) assert(errno == EFAULT); +#endif /* EINVAL cases. */ #if defined(MACOS) From 5d37fd5e62f054b67800adab382322a690bcf44d Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 22:32:36 -0500 Subject: [PATCH 09/37] Remove tmate debug statements. --- .github/workflows/ci-osx.yml | 4 ---- suite/tests/linux/sigaction.c | 3 --- 2 files changed, 7 deletions(-) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index bb0579eee2b..2bea967b8f1 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,10 +81,6 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} - - name: Setup tmate session - if: ${{ failure() }} - uses: mxschmitt/action-tmate@v3 - - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 4e993ae2a09..8c1bffb2d27 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -149,9 +149,6 @@ test_sigprocmask() assert(make_sigprocmask(~0, NULL, &original, 8) == 0); /* EFAULT cases. */ - /* TODO: bad old sigset does not cause EFAULT on Mac. Fix before - * submitting. - */ /* sigprocmask on MacOS does not fail when the old sigset is not * readable or not writeable. */ From 085257aea3fe1544082c9e7de0f9a01b9f91e885 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 17 Dec 2021 22:56:44 -0500 Subject: [PATCH 10/37] Fix issue comments. --- core/unix/signal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index 5640ddd6af4..472fbca0320 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2304,7 +2304,7 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, * address_is_readable.cc#L85 * Those uses as well as our checks below don't guarantee that the given * address will _remain_ readable or writable, even if they succeed now. - * TODO i#5255: DR can at least pass the safe_set to the actual syscall + * TODO i#5254: DR can at least pass the safe_set to the actual syscall * (in cases where it is not skipped) to avoid a second read of app_set, * by the kernel. */ @@ -2421,7 +2421,7 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, { thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field; int i; - /* TODO i#5255: Handle the case where the old sigset was writable in + /* TODO i#5254: Handle the case where the old sigset was writable in * handle_sigprocmask but not now. Also, for the case where app_set is only * readable but not writable. */ From b2f748a186d85d77cef62fa790320f0d475ff2d9 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sat, 18 Dec 2021 13:35:55 -0500 Subject: [PATCH 11/37] Add tmate debug statements. --- .github/workflows/ci-osx.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index 2bea967b8f1..a11e85c121d 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,6 +81,13 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} + - name: Setup tmate session on failure + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + + - name: Setup tmate session on success + uses: mxschmitt/action-tmate@v3 + - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 From e85f09cb8fe59b982cd252a4d04288ae7a3e873c Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sat, 18 Dec 2021 14:44:53 -0500 Subject: [PATCH 12/37] Block some signals to make other tests better. --- core/unix/signal.c | 2 ++ suite/tests/CMakeLists.txt | 2 ++ suite/tests/linux/sigaction.c | 22 ++++++++++++++-------- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index 472fbca0320..2e5e55552a8 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2434,6 +2434,7 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, if (DYNAMO_OPTION(intercept_all_signals)) safe_write_ex(oset, sizeof(*oset), &info->pre_syscall_app_sigblocked, NULL); else { + dr_printf("AAA !intercept_all_signals\n"); /* the syscall wrote to oset already, so just add any additional */ for (i = 1; i <= MAX_SIGNUM; i++) { if (EMULATE_SIGMASK(info, i) && @@ -2441,6 +2442,7 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, * from this syscall itself! (PR 523394) */ kernel_sigismember(&info->pre_syscall_app_sigblocked, i)) { + dr_printf("AAA writing to oset %llx\n",oset); kernel_sigaddset(oset, i); } } diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 6822630d061..eb0867c6bc2 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4142,6 +4142,8 @@ if (UNIX) tobuild(linux.sigplain110 linux/sigplain110.c) tobuild(linux.sigplain111 linux/sigplain111.c) tobuild(linux.sigaction linux/sigaction.c) + tobuild_ops(linux.sigaction.no_code_api linux/sigaction.c + "-no_code_api -no_intercept_all_signals" "") # We want a native run to verify our assumptions of kernel behaviour. torunonly_native(code_api|linux.sigaction.native linux.sigaction "" linux/sigaction.c "") diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 8c1bffb2d27..f0d9949b7c4 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -138,9 +138,10 @@ static void test_sigprocmask() { #if defined(MACOS) - sigset_t new = 0xf00d, old, original; + sigset_t new, old, original; + sigfillset(&new); #else - uint64 new = 0xf00d, old, original; + uint64 new = 0xffffffff, old, original; #endif /* Save original sigprocmask. Both return the current sigprocmask. */ @@ -148,6 +149,17 @@ test_sigprocmask() /*sizeof(kernel_sigset_t)*/ 8) == 0); assert(make_sigprocmask(~0, NULL, &original, 8) == 0); + /* Success cases. + * The following should come before other tests so that some + * signals that DR intercepts are blocked, which would require + * DR to fix the old sigset manually before returning to the user + * in -no_intercept_all_signals. + */ + assert(make_sigprocmask(~0, NULL, NULL, 8) == 0); + assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 8) == 0); + assert(make_sigprocmask(~0, NULL, &old, 8) == 0); + assert(new == old); + /* EFAULT cases. */ /* sigprocmask on MacOS does not fail when the old sigset is not * readable or not writeable. @@ -196,12 +208,6 @@ test_sigprocmask() assert(make_sigprocmask(~0, &new, (void *)0x123, 8) == -1); assert(errno == EINVAL); - /* Success. */ - assert(make_sigprocmask(~0, NULL, NULL, 8) == 0); - assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 8) == 0); - assert(make_sigprocmask(~0, NULL, &old, 8) == 0); - assert(new == old); - /* Restore original sigprocmask. */ assert(make_sigprocmask(SIG_SETMASK, &original, NULL, 8) == 0); } From 95f3f3afcea9fb70b2fb17eb31c62122a4e9c5e5 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sat, 18 Dec 2021 15:15:24 -0500 Subject: [PATCH 13/37] Add test for no_intercept_all_signals --- core/unix/signal.c | 2 -- suite/tests/CMakeLists.txt | 8 ++++---- suite/tests/linux/sigaction.c | 9 ++++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index 2e5e55552a8..472fbca0320 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2434,7 +2434,6 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, if (DYNAMO_OPTION(intercept_all_signals)) safe_write_ex(oset, sizeof(*oset), &info->pre_syscall_app_sigblocked, NULL); else { - dr_printf("AAA !intercept_all_signals\n"); /* the syscall wrote to oset already, so just add any additional */ for (i = 1; i <= MAX_SIGNUM; i++) { if (EMULATE_SIGMASK(info, i) && @@ -2442,7 +2441,6 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, * from this syscall itself! (PR 523394) */ kernel_sigismember(&info->pre_syscall_app_sigblocked, i)) { - dr_printf("AAA writing to oset %llx\n",oset); kernel_sigaddset(oset, i); } } diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index eb0867c6bc2..6528b2be9d7 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -99,6 +99,8 @@ endif (UNIX) set(main_run_list # our main configuration "SHORT::-code_api" + # no_intercept_all_signals tests. + "SHORT::ONLY::signal_intercept$::-no_code_api -no_intercept_all_signals" # sanity check: run single app to make sure these options aren't totally broken # i#1575: ARM doesn't yet support -coarse_units "SHORT::X86::ONLY::client.events$::-code_api -opt_memory" @@ -4141,11 +4143,9 @@ if (UNIX) tobuild(linux.sigplain101 linux/sigplain101.c) tobuild(linux.sigplain110 linux/sigplain110.c) tobuild(linux.sigplain111 linux/sigplain111.c) - tobuild(linux.sigaction linux/sigaction.c) - tobuild_ops(linux.sigaction.no_code_api linux/sigaction.c - "-no_code_api -no_intercept_all_signals" "") + tobuild(linux.sigaction.signal_intercept linux/sigaction.c) # We want a native run to verify our assumptions of kernel behaviour. - torunonly_native(code_api|linux.sigaction.native linux.sigaction + torunonly_native(code_api|linux.sigaction.native linux.sigaction.signal_intercept "" linux/sigaction.c "") if (LINUX) tobuild(linux.syscall_pwait linux/syscall_pwait.cpp) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index f0d9949b7c4..df276484f6d 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -138,10 +138,13 @@ static void test_sigprocmask() { #if defined(MACOS) - sigset_t new, old, original; - sigfillset(&new); + sigset_t new = 0xf00d, old, original; + /* We explicitly add SIGSEGV to the blocked set as it is one of the + * signals that DR intercepts. + */ + sigaddset(&new, SIGSEGV); #else - uint64 new = 0xffffffff, old, original; + uint64 new = 0xf00d | SIGSEGV, old, original; #endif /* Save original sigprocmask. Both return the current sigprocmask. */ From 3615f96cbf2fb8b841408406a3a92a7b916bd773 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sat, 18 Dec 2021 15:17:04 -0500 Subject: [PATCH 14/37] Enable new tests on MacOS. --- suite/tests/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 6528b2be9d7..d0004614aa4 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4794,7 +4794,8 @@ if (APPLE) code_api|api.dis code_api|api.ir-static code_api|api.drdecode - code_api|linux.sigaction + code_api|linux.sigaction.signal_intercept + no_code_api,no_intercept_all_signals|linux.sigaction.signal_intercept code_api|linux.sigaction.native PROPERTIES LABELS OSX) if (DEBUG) # FIXME i#1806: fails in release From 45110affc360f3815ab0840cea8fddbfd9f150e1 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sat, 18 Dec 2021 15:33:24 -0500 Subject: [PATCH 15/37] Try fixing new sigmask. --- suite/tests/CMakeLists.txt | 2 +- suite/tests/linux/sigaction.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index d0004614aa4..051ad7027d4 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4829,7 +4829,7 @@ if (NOT ANDROID AND AARCHXX) code_api|linux.infinite code_api|linux.longjmp code_api|linux.readlink - code_api|linux.sigaction + code_api|linux.sigaction.signal_intercept code_api|linux.signalfd code_api|linux.signal_racesys code_api|security-common.codemod diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index df276484f6d..5484ee0f150 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -142,11 +142,11 @@ test_sigprocmask() /* We explicitly add SIGSEGV to the blocked set as it is one of the * signals that DR intercepts. */ + sigaddset(&new, SIGBUS); sigaddset(&new, SIGSEGV); #else - uint64 new = 0xf00d | SIGSEGV, old, original; + uint64 new = (0xf00d | (1< Date: Sat, 18 Dec 2021 15:34:46 -0500 Subject: [PATCH 16/37] Try fixing new sigmask. --- suite/tests/linux/sigaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 5484ee0f150..eb68ebd4b99 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -143,7 +143,7 @@ test_sigprocmask() * signals that DR intercepts. */ sigaddset(&new, SIGBUS); - sigaddset(&new, SIGSEGV); + sigdelset(&new, SIGSEGV); #else uint64 new = (0xf00d | (1< Date: Sat, 18 Dec 2021 15:54:11 -0500 Subject: [PATCH 17/37] Block SIGBUS only. --- suite/tests/linux/sigaction.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index eb68ebd4b99..9c362b34ced 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -138,14 +138,14 @@ static void test_sigprocmask() { #if defined(MACOS) - sigset_t new = 0xf00d, old, original; + sigset_t new, old, original; /* We explicitly add SIGSEGV to the blocked set as it is one of the * signals that DR intercepts. */ + sigemptyset(&new); sigaddset(&new, SIGBUS); - sigdelset(&new, SIGSEGV); #else - uint64 new = (0xf00d | (1< Date: Sat, 18 Dec 2021 16:12:35 -0500 Subject: [PATCH 18/37] Add fix for non-writable oset on mac. --- core/unix/os.c | 5 ++- core/unix/os_private.h | 2 +- core/unix/signal.c | 65 +++++++++++++++++++++++++---------- suite/tests/linux/sigaction.c | 7 ++-- 4 files changed, 54 insertions(+), 25 deletions(-) diff --git a/core/unix/os.c b/core/unix/os.c index 1bd87317b78..78edfab33e2 100644 --- a/core/unix/os.c +++ b/core/unix/os.c @@ -8926,9 +8926,12 @@ post_system_call(dcontext_t *dcontext) size_t sigsetsize) */ /* FIXME i#148: Handle syscall failure. */ - handle_post_sigprocmask( + int status = handle_post_sigprocmask( dcontext, (int)dcontext->sys_param0, (kernel_sigset_t *)dcontext->sys_param1, (kernel_sigset_t *)dcontext->sys_param2, (size_t)dcontext->sys_param3); + if (status != 0) { + set_failure_return_val(dcontext, status); + } break; } #if defined(LINUX) && !defined(X64) diff --git a/core/unix/os_private.h b/core/unix/os_private.h index a9e76492c57..4eedc909cba 100644 --- a/core/unix/os_private.h +++ b/core/unix/os_private.h @@ -351,7 +351,7 @@ handle_sigaltstack(dcontext_t *dcontext, const stack_t *stack, stack_t *old_stac bool handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *set, kernel_sigset_t *oset, size_t sigsetsize, uint *error_code); -void +int handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *set, kernel_sigset_t *oset, size_t sigsetsize); void diff --git a/core/unix/signal.c b/core/unix/signal.c index 472fbca0320..daa80564386 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2415,37 +2415,64 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, } /* need to add in our signals that the app thinks are blocked */ -void +int handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, kernel_sigset_t *oset, size_t sigsetsize) { thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field; int i; - /* TODO i#5254: Handle the case where the old sigset was writable in - * handle_sigprocmask but not now. Also, for the case where app_set is only - * readable but not writable. - */ if (!DYNAMO_OPTION(intercept_all_signals)) { - /* Restore app memory */ + /* Restore app memory. + * XXX i#1187: See comment in handle_sigprocmask about read-only + * app_set. Ideally, we would replace app_set with our own copy + * pre-syscall. + * In case of a write failure below, we do not return an EFAULT as it would + * be incorrect. We try continuing and return a cloberred app_set to the app. + * This is low priority as -intercept_all_signals is true by default. + */ safe_write_ex(app_set, sizeof(*app_set), &info->pre_syscall_app_sigprocmask, NULL); } if (oset != NULL) { - if (DYNAMO_OPTION(intercept_all_signals)) - safe_write_ex(oset, sizeof(*oset), &info->pre_syscall_app_sigblocked, NULL); - else { - /* the syscall wrote to oset already, so just add any additional */ - for (i = 1; i <= MAX_SIGNUM; i++) { - if (EMULATE_SIGMASK(info, i) && - /* use the pre-syscall value: do not take into account changes - * from this syscall itself! (PR 523394) - */ - kernel_sigismember(&info->pre_syscall_app_sigblocked, i)) { - kernel_sigaddset(oset, i); - } - } + /* We want to handle the following cases where we are unable to fix oset + * because it is not writeable (anymore). Note that handle_sigprocmask + * does the writeability check only for non-MacOS. + * - On non-Mac systems, if oset was initially writable in our + * handle_sigprocmask check, but is unwriteable now, we want to + * return EFAULT. + * - On Mac systems, where a bad oset does not cause EFAULT, + * we want to fail the write silently. + */ + if (DYNAMO_OPTION(intercept_all_signals)) { + if (!safe_write_ex(oset, sizeof(*oset), &info->pre_syscall_app_sigblocked, + NULL)) + return IF_MACOS_ELSE(0, EFAULT); + } else { + bool write_fault = true; + TRY_EXCEPT( + dcontext, + { + /* the syscall wrote to oset already, so just add any additional */ + for (i = 1; i <= MAX_SIGNUM; i++) { + if (EMULATE_SIGMASK(info, i) && + /* use the pre-syscall value: do not take into account changes + * from this syscall itself! (PR 523394) + */ + kernel_sigismember(&info->pre_syscall_app_sigblocked, i)) { + kernel_sigaddset(oset, i); + } + } + write_fault = false; + }, + { + /* EXCEPT */ + /* nothing: write_fault is already true */ + }); + if (write_fault) + return IF_MACOS_ELSE(0, EFAULT); } } + return 0; } void diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 9c362b34ced..4b1934ab354 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -138,14 +138,13 @@ static void test_sigprocmask() { #if defined(MACOS) - sigset_t new, old, original; - /* We explicitly add SIGSEGV to the blocked set as it is one of the + sigset_t new = 0xf00d, old, original; + /* We explicitly add SIGBUS to the blocked set as it is one of the * signals that DR intercepts. */ - sigemptyset(&new); sigaddset(&new, SIGBUS); #else - uint64 new = (1 << SIGBUS), old, original; + uint64 new = 0xf00d | (1 << SIGBUS), old, original; #endif /* Save original sigprocmask. Both return the current sigprocmask. */ assert(make_sigprocmask(SIG_SETMASK, NULL, &original, From b89c421d75b741a17f47e85b7a7b4afe5c8802df Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sat, 18 Dec 2021 19:18:36 -0500 Subject: [PATCH 19/37] Fix sys_kill detection for MacOS. https://www.unix.com/man-page/osx/3HEAD/siginfo.h/ si_info <= 0 denotes user-sent signals on Mac as well. --- core/unix/signal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index daa80564386..d64fada2798 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -4874,7 +4874,7 @@ record_pending_signal(dcontext_t *dcontext, int sig, kernel_ucontext_t *ucxt, static bool is_sys_kill(dcontext_t *dcontext, byte *pc, byte *xsp, kernel_siginfo_t *info) { -#if !defined(VMX86_SERVER) && !defined(MACOS) /* does not use SI_KERNEL */ +#if !defined(VMX86_SERVER) /* does not use SI_KERNEL */ /* i#133: use si_code to distinguish user-sent signals. * Even 2.2 Linux kernel supports <=0 meaning user-sent (except * SIGIO) so we assume we can rely on it. From 04150be2d3817a493219dd468826a4a5c526b4f0 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 03:13:12 -0500 Subject: [PATCH 20/37] Fix MacOS sig kill issues. --- core/unix/signal.c | 2 +- i5256_debug_diff | 343 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 344 insertions(+), 1 deletion(-) create mode 100644 i5256_debug_diff diff --git a/core/unix/signal.c b/core/unix/signal.c index d64fada2798..2b48d89090d 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -6203,7 +6203,7 @@ terminate_via_kill(dcontext_t *dcontext) * this may not exit all threads in the address space */ block_cleanup_and_terminate( - dcontext, SYS_kill, + dcontext, IF_MACOS(0x2000000 +) SYS_kill, /* Pass -pid in case main thread has exited * in which case will get -ESRCH */ diff --git a/i5256_debug_diff b/i5256_debug_diff new file mode 100644 index 00000000000..b9a4fa516a9 --- /dev/null +++ b/i5256_debug_diff @@ -0,0 +1,343 @@ +diff --git a/core/arch/arch.c b/core/arch/arch.c +index e132a289..68a4101d 100644 +--- a/core/arch/arch.c ++++ b/core/arch/arch.c +@@ -2819,6 +2819,7 @@ get_global_do_syscall_entry() + return (byte *)global_do_syscall_sygate_int; + else + #endif ++ dr_printf("AAA returning global_do_syscall_int\n"); + return (byte *)global_do_syscall_int; + } else if (method == SYSCALL_METHOD_SYSENTER) { + #ifdef WINDOWS +@@ -2827,6 +2828,7 @@ get_global_do_syscall_entry() + else + return (byte *)global_do_syscall_sysenter; + #else ++ dr_printf("AAA returning global_do_syscall_int\n"); + return (byte *)global_do_syscall_int; + #endif + } +@@ -2836,11 +2838,13 @@ get_global_do_syscall_entry() + #endif + else if (method == SYSCALL_METHOD_SYSCALL) { + #if defined(X86) && defined(X64) ++ dr_printf("AAA returning global_do_syscall_syscall\n"); + return (byte *)global_do_syscall_syscall; + #else + # ifdef WINDOWS + ASSERT_NOT_IMPLEMENTED(false && "PR 205898: 32-bit syscall on Windows NYI"); + # else ++ dr_printf("AAA returning global_do_syscall_int\n"); + return (byte *)global_do_syscall_int; + # endif + #endif +@@ -2849,6 +2853,7 @@ get_global_do_syscall_entry() + /* PR 205310: we sometimes have to execute syscalls before we + * see an app syscall: for a signal default action, e.g. + */ ++ dr_printf("AAA returning global_do_syscall_int\n"); + return (byte *)IF_X86_64_ELSE(global_do_syscall_syscall, global_do_syscall_int); + #else + ASSERT_NOT_REACHED(); +@@ -2862,14 +2867,17 @@ get_global_do_syscall_entry() + byte * + get_cleanup_and_terminate_global_do_syscall_entry() + { ++ dr_printf("AAA in get_cleanup_and_terminate_global_do_syscall_entry\n"); + /* see note above: for 32-bit linux apps we use int. + * xref PR 332427 as well where sysenter causes a crash + * if called from cleanup_and_terminate() where ebp is + * left pointing to the old freed stack. + */ + #if defined(WINDOWS) || (defined(X86) && defined(X64)) +- if (get_syscall_method() == SYSCALL_METHOD_SYSENTER) ++ if (get_syscall_method() == SYSCALL_METHOD_SYSENTER) { ++ dr_printf("AAA returning global_do_syscall_sysenter\n"); + return (byte *)global_do_syscall_sysenter; ++ } + else + #endif + #ifdef WINDOWS +@@ -2877,8 +2885,11 @@ get_cleanup_and_terminate_global_do_syscall_entry() + return (byte *)global_do_syscall_wow64_index0; + else + #endif ++{ ++ dr_printf("AAA calling get_global_do_syscall_entry\n"); + return get_global_do_syscall_entry(); + } ++} + + #ifdef MACOS + /* There is no single resumption point from sysenter: each sysenter stores +diff --git a/core/arch/x86/x86.asm b/core/arch/x86/x86.asm +index 249f3b72..c756d664 100644 +--- a/core/arch/x86/x86.asm ++++ b/core/arch/x86/x86.asm +@@ -155,6 +155,7 @@ DECL_EXTERN(dr_app_start_helper) + DECL_EXTERN(dynamo_process_exit) + DECL_EXTERN(dynamo_thread_exit) + DECL_EXTERN(dynamo_thread_stack_free_and_exit) ++DECL_EXTERN(dynamo_debug) + DECL_EXTERN(dynamorio_app_take_over_helper) + DECL_EXTERN(found_modified_code) + DECL_EXTERN(get_cleanup_and_terminate_global_do_syscall_entry) +@@ -690,6 +691,7 @@ cat_no_thread2: + # ifdef UNIX + pop REG_XDI /* sys_arg1 */ + pop REG_XSI /* sys_arg2 */ ++ //CALLC4(GLOBAL_REF(dynamo_debug), REG_XAX, r10, REG_XDI, REG_XSI) + # else + pop REG_XCX /* sys_arg1 */ + pop REG_XDX /* sys_arg2 */ +@@ -837,6 +839,7 @@ GLOBAL_LABEL(global_do_syscall_sygate_sysenter:) + DECLARE_FUNC(global_do_syscall_syscall) + GLOBAL_LABEL(global_do_syscall_syscall:) + mov r10, REG_XCX ++ CALLC4(GLOBAL_REF(dynamo_debug), REG_XAX, REG_XCX, REG_XDI, REG_XSI) + syscall + # ifdef DEBUG + jmp GLOBAL_REF(debug_infinite_loop) +diff --git a/core/dynamo.c b/core/dynamo.c +index 80bb4ca7..0ef14abb 100644 +--- a/core/dynamo.c ++++ b/core/dynamo.c +@@ -1422,6 +1422,7 @@ dynamo_nullcalls_exit(void) + int + dynamo_process_exit(void) + { ++ dr_printf("AAA in dynamo_process_exit\n"); + #ifndef DEBUG + bool each_thread; + #endif +@@ -2698,17 +2699,25 @@ dynamo_other_thread_exit(thread_record_t *tr _IF_WINDOWS(bool detach_stacked_cal + IF_WINDOWS_(detach_stacked_callbacks) true); + } + ++void ++dynamo_debug(uint64 a, uint64 b, uint64 c, uint64 d) { ++ ++dr_printf("AAA dynamo_debug %lld %lld %lld %lld\n",a,b,c,d); ++} ++ + /* Called from another stack to finish cleaning up a thread. + * The final steps are to free the stack and perform the exit hook. + */ + void + dynamo_thread_stack_free_and_exit(byte *stack) + { ++ dr_printf("AAA in dynamo_thread_stack_free_and_exit\n"); + if (stack != NULL) { + stack_free(stack, DYNAMORIO_STACK_SIZE); + /* ASSUMPTION: if stack is NULL here, the exit was done earlier + * (fixes case 6967) + */ ++ dr_printf("AAA calling exiting_dr\n"); + EXITING_DR(); + } + } +diff --git a/core/globals.h b/core/globals.h +index 0aadd6cd..c83789fa 100644 +--- a/core/globals.h ++++ b/core/globals.h +@@ -524,6 +524,8 @@ dynamo_thread_init(byte *dstack_in, priv_mcontext_t *mc, void *os_data, + int + dynamo_thread_exit(void); + void ++dynamo_debug(uint64 a, uint64 b, uint64 c, uint64 d); ++void + dynamo_thread_stack_free_and_exit(byte *stack); + int + dynamo_other_thread_exit(thread_record_t *tr _IF_WINDOWS(bool detach_stacked_callbacks)); +diff --git a/core/unix/os.c b/core/unix/os.c +index 78edfab3..12e6bd54 100644 +--- a/core/unix/os.c ++++ b/core/unix/os.c +@@ -1381,15 +1381,20 @@ block_cleanup_and_terminate(dcontext_t *dcontext, int sysnum, ptr_uint_t sys_arg + /* these 2 args are only used for Mac thread exit */ + ptr_uint_t sys_arg3, ptr_uint_t sys_arg4) + { ++ dr_printf("AAA in block_cleanup_and_terminate, sysnum %d %d\n", sysnum, SYS_kill); + /* This thread is on its way to exit. We are blocking all signals since any + * signal that reaches us now can be delayed until after the exit is complete. + * We may still receive a suspend signal for synchronization that we may need + * to reply to (i#2921). + */ +- if (sysnum == SYS_kill) +- block_all_noncrash_signals_except(NULL, 2, dcontext->sys_param0, SUSPEND_SIGNAL); ++ if (sysnum == SYS_kill) { ++ block_all_noncrash_signals_except(NULL, 3, dcontext->sys_param0, SUSPEND_SIGNAL, ++ SIGINT); ++ } + else + block_all_noncrash_signals_except(NULL, 1, SUSPEND_SIGNAL); ++ dr_printf("AAA calling cleanup_and_terminate %d %lld %lld\n", sysnum, sys_arg1, ++ sys_arg2); + cleanup_and_terminate(dcontext, sysnum, sys_arg1, sys_arg2, exitproc, sys_arg3, + sys_arg4); + } +@@ -5984,28 +5989,28 @@ handle_execve(dcontext_t *dcontext) + /* i#909: change the target image to libdynamorio.so */ + const char *drpath = IF_X64_ELSE(x64, !x64) ? dynamorio_library_filepath + : dynamorio_alt_arch_filepath; +- TRY_EXCEPT(dcontext, /* try */ +- { +- if (symlink_is_self_exe(argv[0])) { +- /* we're out of sys_param entries so we assume argv[0] == fname +- */ +- dcontext->sys_param3 = (reg_t)argv; +- argv[0] = fname; /* XXX: handle readable but not writable! */ +- } else +- dcontext->sys_param3 = 0; /* no restore in post */ +- dcontext->sys_param4 = +- (reg_t)fname; /* store for restore in post */ +- *sys_param_addr(dcontext, 0) = (reg_t)drpath; +- LOG(THREAD, LOG_SYSCALLS, 2, "actual execve on: %s\n", +- (char *)sys_param(dcontext, 0)); +- }, +- /* except */ +- { +- dcontext->sys_param3 = 0; /* no restore in post */ +- dcontext->sys_param4 = 0; /* no restore in post */ +- LOG(THREAD, LOG_SYSCALLS, 2, +- "argv is unreadable, expect execve to fail\n"); +- }); ++ TRY_EXCEPT( ++ dcontext, /* try */ ++ { ++ if (symlink_is_self_exe(argv[0])) { ++ /* we're out of sys_param entries so we assume argv[0] == fname ++ */ ++ dcontext->sys_param3 = (reg_t)argv; ++ argv[0] = fname; /* XXX: handle readable but not writable! */ ++ } else ++ dcontext->sys_param3 = 0; /* no restore in post */ ++ dcontext->sys_param4 = (reg_t)fname; /* store for restore in post */ ++ *sys_param_addr(dcontext, 0) = (reg_t)drpath; ++ LOG(THREAD, LOG_SYSCALLS, 2, "actual execve on: %s\n", ++ (char *)sys_param(dcontext, 0)); ++ }, ++ /* except */ ++ { ++ dcontext->sys_param3 = 0; /* no restore in post */ ++ dcontext->sys_param4 = 0; /* no restore in post */ ++ LOG(THREAD, LOG_SYSCALLS, 2, ++ "argv is unreadable, expect execve to fail\n"); ++ }); + } else { + dcontext->sys_param3 = 0; /* no restore in post */ + dcontext->sys_param4 = 0; /* no restore in post */ +@@ -6839,11 +6844,12 @@ pre_system_call(dcontext_t *dcontext) + /* not using SAFE_READ due to performance concerns (we do this for + * every single system call on systems where we can't hook vsyscall!) + */ +- TRY_EXCEPT(dcontext, /* try */ { mc->xbp = *(reg_t *)mc->xsp; }, /* except */ +- { +- ASSERT_NOT_REACHED(); +- mc->xbp = 0; +- }); ++ TRY_EXCEPT( ++ dcontext, /* try */ { mc->xbp = *(reg_t *)mc->xsp; }, /* except */ ++ { ++ ASSERT_NOT_REACHED(); ++ mc->xbp = 0; ++ }); + } + #endif + +diff --git a/core/unix/signal.c b/core/unix/signal.c +index d64fada2..8a3ddd2d 100644 +--- a/core/unix/signal.c ++++ b/core/unix/signal.c +@@ -1011,9 +1011,9 @@ signal_thread_inherit(dcontext_t *dcontext, void *clone_record) + ? "vfork" + : + #endif +- (IF_LINUX(record->clone_sysnum == SYS_clone +- ? "clone" +- : record->clone_sysnum == SYS_clone3 ? "clone3" :) ++ (IF_LINUX(record->clone_sysnum == SYS_clone ? "clone" ++ : record->clone_sysnum == SYS_clone3 ? "clone3" ++ :) + IF_MACOS(record->clone_sysnum == SYS_bsdthread_create + ? "bsdthread_create" + :) "unexpected"), +@@ -3553,15 +3553,16 @@ copy_frame_to_stack(dcontext_t *dcontext, thread_sig_info_t *info, int sig, + IF_NOT_X64(IF_LINUX(else convert_frame_to_nonrt(dcontext, sig, frame, + (sigframe_plain_t *)sp, size);)); + } else { +- TRY_EXCEPT(dcontext, /* try */ +- { +- if (rtframe) +- memcpy_rt_frame(frame, sp, from_pending); +- IF_NOT_X64(IF_LINUX( +- else convert_frame_to_nonrt(dcontext, sig, frame, +- (sigframe_plain_t *)sp, size);)); +- }, +- /* except */ { stack_unwritable = true; }); ++ TRY_EXCEPT( ++ dcontext, /* try */ ++ { ++ if (rtframe) ++ memcpy_rt_frame(frame, sp, from_pending); ++ IF_NOT_X64( ++ IF_LINUX(else convert_frame_to_nonrt(dcontext, sig, frame, ++ (sigframe_plain_t *)sp, size);)); ++ }, ++ /* except */ { stack_unwritable = true; }); + } + if (stack_unwritable) { + /* Override the no-nested check in record_pending_signal(): it's ok b/c +@@ -4926,10 +4927,11 @@ compute_memory_target(dcontext_t *dcontext, cache_pc instr_cache_pc, + /* Be sure to use the interrupted mode and not the last-dispatch mode */ + dr_set_isa_mode(dcontext, get_pc_mode_from_cpsr(sc), &old_mode); + }); +- TRY_EXCEPT(dcontext, { decode(dcontext, instr_cache_pc, &instr); }, +- { +- return NULL; /* instr_cache_pc was unreadable */ +- }); ++ TRY_EXCEPT( ++ dcontext, { decode(dcontext, instr_cache_pc, &instr); }, ++ { ++ return NULL; /* instr_cache_pc was unreadable */ ++ }); + IF_ARM(dr_set_isa_mode(dcontext, old_mode, NULL)); + + if (!instr_valid(&instr)) { +@@ -6194,11 +6196,16 @@ execute_native_handler(dcontext_t *dcontext, int sig, sigframe_rt_t *our_frame) + static void + terminate_via_kill(dcontext_t *dcontext) + { ++ dr_printf("AAA in terminate_viakill\n"); + thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field; + ASSERT(dcontext == get_thread_private_dcontext()); + /* Enure signal_thread_exit() will not re-block */ + memset(&info->app_sigblocked, 0, sizeof(info->app_sigblocked)); + ++ dr_printf("AAA calling block_cleanup_and_terminate %d %d %d\n", SYS_kill, ++ IF_VMX86(os_in_vmkernel_userworld() ? -(int)get_process_id() :) ++ get_process_id(), ++ dcontext->sys_param0); + /* FIXME PR 541760: there can be multiple thread groups and thus + * this may not exit all threads in the address space + */ +@@ -6225,8 +6232,10 @@ is_currently_on_sigaltstack(dcontext_t *dcontext) + static void + terminate_via_kill_from_anywhere(dcontext_t *dcontext, int sig) + { ++ dr_printf("AAA in terminate_via_kill_from_anywhere sig %d\n", sig); + dcontext->sys_param0 = sig; /* store arg to SYS_kill */ + if (is_currently_on_sigaltstack(dcontext)) { ++ dr_printf("AAA currently on sigaltstac\n"); + /* We can't clean up our sigstack properly when we're on it + * (i#1160) so we terminate on the dstack. + */ +@@ -6234,6 +6243,7 @@ terminate_via_kill_from_anywhere(dcontext_t *dcontext, int sig) + (void (*)(void *))terminate_via_kill, NULL /*!d_r_initstack */, + false /*no return */); + } else { ++ dr_printf("AAA terminate_via_kill call\n"); + terminate_via_kill(dcontext); + } + ASSERT_NOT_REACHED(); From 8a2bec208f45586a0328ddb75e28b8ae4f5ca20a Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 03:14:47 -0500 Subject: [PATCH 21/37] Remove stray code. --- i5256_debug_diff | 343 ----------------------------------------------- 1 file changed, 343 deletions(-) delete mode 100644 i5256_debug_diff diff --git a/i5256_debug_diff b/i5256_debug_diff deleted file mode 100644 index b9a4fa516a9..00000000000 --- a/i5256_debug_diff +++ /dev/null @@ -1,343 +0,0 @@ -diff --git a/core/arch/arch.c b/core/arch/arch.c -index e132a289..68a4101d 100644 ---- a/core/arch/arch.c -+++ b/core/arch/arch.c -@@ -2819,6 +2819,7 @@ get_global_do_syscall_entry() - return (byte *)global_do_syscall_sygate_int; - else - #endif -+ dr_printf("AAA returning global_do_syscall_int\n"); - return (byte *)global_do_syscall_int; - } else if (method == SYSCALL_METHOD_SYSENTER) { - #ifdef WINDOWS -@@ -2827,6 +2828,7 @@ get_global_do_syscall_entry() - else - return (byte *)global_do_syscall_sysenter; - #else -+ dr_printf("AAA returning global_do_syscall_int\n"); - return (byte *)global_do_syscall_int; - #endif - } -@@ -2836,11 +2838,13 @@ get_global_do_syscall_entry() - #endif - else if (method == SYSCALL_METHOD_SYSCALL) { - #if defined(X86) && defined(X64) -+ dr_printf("AAA returning global_do_syscall_syscall\n"); - return (byte *)global_do_syscall_syscall; - #else - # ifdef WINDOWS - ASSERT_NOT_IMPLEMENTED(false && "PR 205898: 32-bit syscall on Windows NYI"); - # else -+ dr_printf("AAA returning global_do_syscall_int\n"); - return (byte *)global_do_syscall_int; - # endif - #endif -@@ -2849,6 +2853,7 @@ get_global_do_syscall_entry() - /* PR 205310: we sometimes have to execute syscalls before we - * see an app syscall: for a signal default action, e.g. - */ -+ dr_printf("AAA returning global_do_syscall_int\n"); - return (byte *)IF_X86_64_ELSE(global_do_syscall_syscall, global_do_syscall_int); - #else - ASSERT_NOT_REACHED(); -@@ -2862,14 +2867,17 @@ get_global_do_syscall_entry() - byte * - get_cleanup_and_terminate_global_do_syscall_entry() - { -+ dr_printf("AAA in get_cleanup_and_terminate_global_do_syscall_entry\n"); - /* see note above: for 32-bit linux apps we use int. - * xref PR 332427 as well where sysenter causes a crash - * if called from cleanup_and_terminate() where ebp is - * left pointing to the old freed stack. - */ - #if defined(WINDOWS) || (defined(X86) && defined(X64)) -- if (get_syscall_method() == SYSCALL_METHOD_SYSENTER) -+ if (get_syscall_method() == SYSCALL_METHOD_SYSENTER) { -+ dr_printf("AAA returning global_do_syscall_sysenter\n"); - return (byte *)global_do_syscall_sysenter; -+ } - else - #endif - #ifdef WINDOWS -@@ -2877,8 +2885,11 @@ get_cleanup_and_terminate_global_do_syscall_entry() - return (byte *)global_do_syscall_wow64_index0; - else - #endif -+{ -+ dr_printf("AAA calling get_global_do_syscall_entry\n"); - return get_global_do_syscall_entry(); - } -+} - - #ifdef MACOS - /* There is no single resumption point from sysenter: each sysenter stores -diff --git a/core/arch/x86/x86.asm b/core/arch/x86/x86.asm -index 249f3b72..c756d664 100644 ---- a/core/arch/x86/x86.asm -+++ b/core/arch/x86/x86.asm -@@ -155,6 +155,7 @@ DECL_EXTERN(dr_app_start_helper) - DECL_EXTERN(dynamo_process_exit) - DECL_EXTERN(dynamo_thread_exit) - DECL_EXTERN(dynamo_thread_stack_free_and_exit) -+DECL_EXTERN(dynamo_debug) - DECL_EXTERN(dynamorio_app_take_over_helper) - DECL_EXTERN(found_modified_code) - DECL_EXTERN(get_cleanup_and_terminate_global_do_syscall_entry) -@@ -690,6 +691,7 @@ cat_no_thread2: - # ifdef UNIX - pop REG_XDI /* sys_arg1 */ - pop REG_XSI /* sys_arg2 */ -+ //CALLC4(GLOBAL_REF(dynamo_debug), REG_XAX, r10, REG_XDI, REG_XSI) - # else - pop REG_XCX /* sys_arg1 */ - pop REG_XDX /* sys_arg2 */ -@@ -837,6 +839,7 @@ GLOBAL_LABEL(global_do_syscall_sygate_sysenter:) - DECLARE_FUNC(global_do_syscall_syscall) - GLOBAL_LABEL(global_do_syscall_syscall:) - mov r10, REG_XCX -+ CALLC4(GLOBAL_REF(dynamo_debug), REG_XAX, REG_XCX, REG_XDI, REG_XSI) - syscall - # ifdef DEBUG - jmp GLOBAL_REF(debug_infinite_loop) -diff --git a/core/dynamo.c b/core/dynamo.c -index 80bb4ca7..0ef14abb 100644 ---- a/core/dynamo.c -+++ b/core/dynamo.c -@@ -1422,6 +1422,7 @@ dynamo_nullcalls_exit(void) - int - dynamo_process_exit(void) - { -+ dr_printf("AAA in dynamo_process_exit\n"); - #ifndef DEBUG - bool each_thread; - #endif -@@ -2698,17 +2699,25 @@ dynamo_other_thread_exit(thread_record_t *tr _IF_WINDOWS(bool detach_stacked_cal - IF_WINDOWS_(detach_stacked_callbacks) true); - } - -+void -+dynamo_debug(uint64 a, uint64 b, uint64 c, uint64 d) { -+ -+dr_printf("AAA dynamo_debug %lld %lld %lld %lld\n",a,b,c,d); -+} -+ - /* Called from another stack to finish cleaning up a thread. - * The final steps are to free the stack and perform the exit hook. - */ - void - dynamo_thread_stack_free_and_exit(byte *stack) - { -+ dr_printf("AAA in dynamo_thread_stack_free_and_exit\n"); - if (stack != NULL) { - stack_free(stack, DYNAMORIO_STACK_SIZE); - /* ASSUMPTION: if stack is NULL here, the exit was done earlier - * (fixes case 6967) - */ -+ dr_printf("AAA calling exiting_dr\n"); - EXITING_DR(); - } - } -diff --git a/core/globals.h b/core/globals.h -index 0aadd6cd..c83789fa 100644 ---- a/core/globals.h -+++ b/core/globals.h -@@ -524,6 +524,8 @@ dynamo_thread_init(byte *dstack_in, priv_mcontext_t *mc, void *os_data, - int - dynamo_thread_exit(void); - void -+dynamo_debug(uint64 a, uint64 b, uint64 c, uint64 d); -+void - dynamo_thread_stack_free_and_exit(byte *stack); - int - dynamo_other_thread_exit(thread_record_t *tr _IF_WINDOWS(bool detach_stacked_callbacks)); -diff --git a/core/unix/os.c b/core/unix/os.c -index 78edfab3..12e6bd54 100644 ---- a/core/unix/os.c -+++ b/core/unix/os.c -@@ -1381,15 +1381,20 @@ block_cleanup_and_terminate(dcontext_t *dcontext, int sysnum, ptr_uint_t sys_arg - /* these 2 args are only used for Mac thread exit */ - ptr_uint_t sys_arg3, ptr_uint_t sys_arg4) - { -+ dr_printf("AAA in block_cleanup_and_terminate, sysnum %d %d\n", sysnum, SYS_kill); - /* This thread is on its way to exit. We are blocking all signals since any - * signal that reaches us now can be delayed until after the exit is complete. - * We may still receive a suspend signal for synchronization that we may need - * to reply to (i#2921). - */ -- if (sysnum == SYS_kill) -- block_all_noncrash_signals_except(NULL, 2, dcontext->sys_param0, SUSPEND_SIGNAL); -+ if (sysnum == SYS_kill) { -+ block_all_noncrash_signals_except(NULL, 3, dcontext->sys_param0, SUSPEND_SIGNAL, -+ SIGINT); -+ } - else - block_all_noncrash_signals_except(NULL, 1, SUSPEND_SIGNAL); -+ dr_printf("AAA calling cleanup_and_terminate %d %lld %lld\n", sysnum, sys_arg1, -+ sys_arg2); - cleanup_and_terminate(dcontext, sysnum, sys_arg1, sys_arg2, exitproc, sys_arg3, - sys_arg4); - } -@@ -5984,28 +5989,28 @@ handle_execve(dcontext_t *dcontext) - /* i#909: change the target image to libdynamorio.so */ - const char *drpath = IF_X64_ELSE(x64, !x64) ? dynamorio_library_filepath - : dynamorio_alt_arch_filepath; -- TRY_EXCEPT(dcontext, /* try */ -- { -- if (symlink_is_self_exe(argv[0])) { -- /* we're out of sys_param entries so we assume argv[0] == fname -- */ -- dcontext->sys_param3 = (reg_t)argv; -- argv[0] = fname; /* XXX: handle readable but not writable! */ -- } else -- dcontext->sys_param3 = 0; /* no restore in post */ -- dcontext->sys_param4 = -- (reg_t)fname; /* store for restore in post */ -- *sys_param_addr(dcontext, 0) = (reg_t)drpath; -- LOG(THREAD, LOG_SYSCALLS, 2, "actual execve on: %s\n", -- (char *)sys_param(dcontext, 0)); -- }, -- /* except */ -- { -- dcontext->sys_param3 = 0; /* no restore in post */ -- dcontext->sys_param4 = 0; /* no restore in post */ -- LOG(THREAD, LOG_SYSCALLS, 2, -- "argv is unreadable, expect execve to fail\n"); -- }); -+ TRY_EXCEPT( -+ dcontext, /* try */ -+ { -+ if (symlink_is_self_exe(argv[0])) { -+ /* we're out of sys_param entries so we assume argv[0] == fname -+ */ -+ dcontext->sys_param3 = (reg_t)argv; -+ argv[0] = fname; /* XXX: handle readable but not writable! */ -+ } else -+ dcontext->sys_param3 = 0; /* no restore in post */ -+ dcontext->sys_param4 = (reg_t)fname; /* store for restore in post */ -+ *sys_param_addr(dcontext, 0) = (reg_t)drpath; -+ LOG(THREAD, LOG_SYSCALLS, 2, "actual execve on: %s\n", -+ (char *)sys_param(dcontext, 0)); -+ }, -+ /* except */ -+ { -+ dcontext->sys_param3 = 0; /* no restore in post */ -+ dcontext->sys_param4 = 0; /* no restore in post */ -+ LOG(THREAD, LOG_SYSCALLS, 2, -+ "argv is unreadable, expect execve to fail\n"); -+ }); - } else { - dcontext->sys_param3 = 0; /* no restore in post */ - dcontext->sys_param4 = 0; /* no restore in post */ -@@ -6839,11 +6844,12 @@ pre_system_call(dcontext_t *dcontext) - /* not using SAFE_READ due to performance concerns (we do this for - * every single system call on systems where we can't hook vsyscall!) - */ -- TRY_EXCEPT(dcontext, /* try */ { mc->xbp = *(reg_t *)mc->xsp; }, /* except */ -- { -- ASSERT_NOT_REACHED(); -- mc->xbp = 0; -- }); -+ TRY_EXCEPT( -+ dcontext, /* try */ { mc->xbp = *(reg_t *)mc->xsp; }, /* except */ -+ { -+ ASSERT_NOT_REACHED(); -+ mc->xbp = 0; -+ }); - } - #endif - -diff --git a/core/unix/signal.c b/core/unix/signal.c -index d64fada2..8a3ddd2d 100644 ---- a/core/unix/signal.c -+++ b/core/unix/signal.c -@@ -1011,9 +1011,9 @@ signal_thread_inherit(dcontext_t *dcontext, void *clone_record) - ? "vfork" - : - #endif -- (IF_LINUX(record->clone_sysnum == SYS_clone -- ? "clone" -- : record->clone_sysnum == SYS_clone3 ? "clone3" :) -+ (IF_LINUX(record->clone_sysnum == SYS_clone ? "clone" -+ : record->clone_sysnum == SYS_clone3 ? "clone3" -+ :) - IF_MACOS(record->clone_sysnum == SYS_bsdthread_create - ? "bsdthread_create" - :) "unexpected"), -@@ -3553,15 +3553,16 @@ copy_frame_to_stack(dcontext_t *dcontext, thread_sig_info_t *info, int sig, - IF_NOT_X64(IF_LINUX(else convert_frame_to_nonrt(dcontext, sig, frame, - (sigframe_plain_t *)sp, size);)); - } else { -- TRY_EXCEPT(dcontext, /* try */ -- { -- if (rtframe) -- memcpy_rt_frame(frame, sp, from_pending); -- IF_NOT_X64(IF_LINUX( -- else convert_frame_to_nonrt(dcontext, sig, frame, -- (sigframe_plain_t *)sp, size);)); -- }, -- /* except */ { stack_unwritable = true; }); -+ TRY_EXCEPT( -+ dcontext, /* try */ -+ { -+ if (rtframe) -+ memcpy_rt_frame(frame, sp, from_pending); -+ IF_NOT_X64( -+ IF_LINUX(else convert_frame_to_nonrt(dcontext, sig, frame, -+ (sigframe_plain_t *)sp, size);)); -+ }, -+ /* except */ { stack_unwritable = true; }); - } - if (stack_unwritable) { - /* Override the no-nested check in record_pending_signal(): it's ok b/c -@@ -4926,10 +4927,11 @@ compute_memory_target(dcontext_t *dcontext, cache_pc instr_cache_pc, - /* Be sure to use the interrupted mode and not the last-dispatch mode */ - dr_set_isa_mode(dcontext, get_pc_mode_from_cpsr(sc), &old_mode); - }); -- TRY_EXCEPT(dcontext, { decode(dcontext, instr_cache_pc, &instr); }, -- { -- return NULL; /* instr_cache_pc was unreadable */ -- }); -+ TRY_EXCEPT( -+ dcontext, { decode(dcontext, instr_cache_pc, &instr); }, -+ { -+ return NULL; /* instr_cache_pc was unreadable */ -+ }); - IF_ARM(dr_set_isa_mode(dcontext, old_mode, NULL)); - - if (!instr_valid(&instr)) { -@@ -6194,11 +6196,16 @@ execute_native_handler(dcontext_t *dcontext, int sig, sigframe_rt_t *our_frame) - static void - terminate_via_kill(dcontext_t *dcontext) - { -+ dr_printf("AAA in terminate_viakill\n"); - thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field; - ASSERT(dcontext == get_thread_private_dcontext()); - /* Enure signal_thread_exit() will not re-block */ - memset(&info->app_sigblocked, 0, sizeof(info->app_sigblocked)); - -+ dr_printf("AAA calling block_cleanup_and_terminate %d %d %d\n", SYS_kill, -+ IF_VMX86(os_in_vmkernel_userworld() ? -(int)get_process_id() :) -+ get_process_id(), -+ dcontext->sys_param0); - /* FIXME PR 541760: there can be multiple thread groups and thus - * this may not exit all threads in the address space - */ -@@ -6225,8 +6232,10 @@ is_currently_on_sigaltstack(dcontext_t *dcontext) - static void - terminate_via_kill_from_anywhere(dcontext_t *dcontext, int sig) - { -+ dr_printf("AAA in terminate_via_kill_from_anywhere sig %d\n", sig); - dcontext->sys_param0 = sig; /* store arg to SYS_kill */ - if (is_currently_on_sigaltstack(dcontext)) { -+ dr_printf("AAA currently on sigaltstac\n"); - /* We can't clean up our sigstack properly when we're on it - * (i#1160) so we terminate on the dstack. - */ -@@ -6234,6 +6243,7 @@ terminate_via_kill_from_anywhere(dcontext_t *dcontext, int sig) - (void (*)(void *))terminate_via_kill, NULL /*!d_r_initstack */, - false /*no return */); - } else { -+ dr_printf("AAA terminate_via_kill call\n"); - terminate_via_kill(dcontext); - } - ASSERT_NOT_REACHED(); From d655145fca42e67bbdf6cab069ea7b24f229749d Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 03:23:49 -0500 Subject: [PATCH 22/37] Add constant for unix class syscalls on Mac. --- core/unix/include/syscall_mach.h | 1 + core/unix/signal.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/unix/include/syscall_mach.h b/core/unix/include/syscall_mach.h index f7bc9997413..d82ee7ebd8c 100644 --- a/core/unix/include/syscall_mach.h +++ b/core/unix/include/syscall_mach.h @@ -38,6 +38,7 @@ #define _SYSCALL_MACH_H_ 1 #define SYSCALL_NUM_MARKER_MACH 0x1000000 +#define SYSCALL_NUM_MARKER_UNIX 0x2000000 #define SYSCALL_NUM_MARKER_MACHDEP 0x3000000 #define MACH__kernelrpc_mach_vm_allocate_trap 10 diff --git a/core/unix/signal.c b/core/unix/signal.c index 2b48d89090d..af80c5ff3eb 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -6203,7 +6203,7 @@ terminate_via_kill(dcontext_t *dcontext) * this may not exit all threads in the address space */ block_cleanup_and_terminate( - dcontext, IF_MACOS(0x2000000 +) SYS_kill, + dcontext, IF_MACOS(SYSCALL_NUM_MARKER_UNIX +) SYS_kill, /* Pass -pid in case main thread has exited * in which case will get -ESRCH */ From 99c194cf0e2a7b2a4610792fa229f50efcb811ee Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 03:24:27 -0500 Subject: [PATCH 23/37] Revert tmate --- .github/workflows/ci-osx.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index a11e85c121d..2bea967b8f1 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,13 +81,6 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} - - name: Setup tmate session on failure - if: ${{ failure() }} - uses: mxschmitt/action-tmate@v3 - - - name: Setup tmate session on success - uses: mxschmitt/action-tmate@v3 - - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 From dd64447233f5cf2769eb50f06eb4ba0fe6e391fa Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 03:39:16 -0500 Subject: [PATCH 24/37] Fix MacOS build. --- core/unix/signal.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/unix/signal.c b/core/unix/signal.c index af80c5ff3eb..21ef20d639f 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -87,6 +87,9 @@ # include "include/syscall.h" #else # include +# ifdef MACOS +# include "include/syscall_mach.h" +# endif #endif #include "instrument.h" From adb3c46e90a057948fa7dcaefff96684fb5d2485 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 03:39:22 -0500 Subject: [PATCH 25/37] Add tmate back --- .github/workflows/ci-osx.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index 2bea967b8f1..a11e85c121d 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,6 +81,13 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} + - name: Setup tmate session on failure + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + + - name: Setup tmate session on success + uses: mxschmitt/action-tmate@v3 + - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 From 8104fa76306002b2303c737338f46f7bd362198d Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 04:13:27 -0500 Subject: [PATCH 26/37] Revert tmate. --- .github/workflows/ci-osx.yml | 7 ------- suite/tests/linux/sigaction.c | 4 ++-- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index a11e85c121d..2bea967b8f1 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,13 +81,6 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} - - name: Setup tmate session on failure - if: ${{ failure() }} - uses: mxschmitt/action-tmate@v3 - - - name: Setup tmate session on success - uses: mxschmitt/action-tmate@v3 - - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 4b1934ab354..9567df32b29 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -31,7 +31,7 @@ */ /* - * test of sigaction + * Tests for sigaction and sigprocmask. */ #include "tools.h" @@ -127,7 +127,7 @@ static int make_sigprocmask(int how, void *sigset, void *oldsigset, int size) { #if defined(MACOS) - /* The raw syscall does not work on Mac. */ + /* XXX: Couldn't get the raw syscall to work on Mac. */ return sigprocmask(how, sigset, oldsigset); #else return syscall(SYS_rt_sigprocmask, how, sigset, oldsigset, size); From a225c7ea30e2ed130b8b99e2a0a26ce7159fbee9 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 19 Dec 2021 13:10:39 -0500 Subject: [PATCH 27/37] Comments fix. --- core/unix/signal.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index 21ef20d639f..025d7c395ab 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2305,11 +2305,6 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, * readable. E.g. * github.com/abseil/abseil-cpp/blob/master/absl/debugging/internal/ * address_is_readable.cc#L85 - * Those uses as well as our checks below don't guarantee that the given - * address will _remain_ readable or writable, even if they succeed now. - * TODO i#5254: DR can at least pass the safe_set to the actual syscall - * (in cases where it is not skipped) to avoid a second read of app_set, - * by the kernel. */ if (sigsetsize != sizeof(kernel_sigset_t)) { if (error_code != NULL) @@ -2352,7 +2347,9 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, * XXX i#1187: we could crash here touching app memory -- could * use TRY, but the app could pass read-only memory and it * would work natively! Better to swap in our own - * allocated data struct. There's a transparency issue w/ + * allocated data struct. This would prevent a second read (by + * the kernel) too, which might fail if it does not _remain_ + * readable anymore. There's a transparency issue w/ * races too if another thread looks at this memory. This * won't happen by default b/c -intercept_all_signals is * on by default so we don't try to solve all these @@ -2428,10 +2425,12 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, /* Restore app memory. * XXX i#1187: See comment in handle_sigprocmask about read-only * app_set. Ideally, we would replace app_set with our own copy - * pre-syscall. + * pre-syscall so that we only need to restore a pointer to the original + * app copy post-syscall. * In case of a write failure below, we do not return an EFAULT as it would - * be incorrect. We try continuing and return a cloberred app_set to the app. - * This is low priority as -intercept_all_signals is true by default. + * be incorrect to expect app_set to be writeable. We try continuing and + * return a cloberred app_set to the app. This is low priority as + * -intercept_all_signals is true by default. */ safe_write_ex(app_set, sizeof(*app_set), &info->pre_syscall_app_sigprocmask, NULL); From 61feb3f16c0afb10898f719beb7c7737866501a3 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Mon, 20 Dec 2021 14:51:18 -0500 Subject: [PATCH 28/37] Reviewer suggested edits. --- core/arch/x86/x86.asm | 5 ++++- core/drlibc/drlibc_x86.asm | 4 ++-- core/unix/include/syscall_mach.h | 2 +- core/unix/os.c | 2 +- core/unix/signal.c | 2 +- suite/tests/CMakeLists.txt | 1 + suite/tests/linux/sigaction.c | 32 +++++++++++++++----------------- 7 files changed, 25 insertions(+), 23 deletions(-) diff --git a/core/arch/x86/x86.asm b/core/arch/x86/x86.asm index 249f3b72507..c5507fe91ac 100644 --- a/core/arch/x86/x86.asm +++ b/core/arch/x86/x86.asm @@ -641,6 +641,9 @@ cat_have_lock: mov REG_XDI, REG_XAX /* esp to use */ #endif mov REG_XSI, [2*ARG_SZ + REG_XBP] /* sysnum */ +#ifdef MACOS64 + or REG_XSI, SYSCALL_NUM_MARKER_BSD +#endif pop REG_XAX /* syscall */ pop REG_XCX /* dstack */ #if defined(UNIX) && !defined(X64) @@ -1261,7 +1264,7 @@ dynamorio_sys_exit_next: mov ARG2, REG_XAX /* kernel port, which we just acquired */ mov ARG1, 0 /* join semaphore: SEMAPHORE_NULL */ mov eax, SYS_bsdthread_terminate - or eax, HEX(2000000) /* 2<<24 for BSD syscall */ + or eax, SYSCALL_NUM_MARKER_BSD mov r10, rcx syscall # else diff --git a/core/drlibc/drlibc_x86.asm b/core/drlibc/drlibc_x86.asm index 20972d39864..c32f435b35a 100644 --- a/core/drlibc/drlibc_x86.asm +++ b/core/drlibc/drlibc_x86.asm @@ -43,7 +43,7 @@ #include "../arch/asm_defines.asm" #include "../arch/x86/x86_asm_defines.asm" /* PUSHGPR, POPGPR, etc. */ #ifdef MACOS -# include "include/syscall_mach.h" /* SYSCALL_NUM_MARKER_MACH */ +# include "include/syscall_mach.h" /* SYSCALL_NUM_MARKER_* */ #endif START_FILE @@ -68,7 +68,7 @@ GLOBAL_LABEL(dynamorio_syscall:) mov rax, ARG1 /* sysnum: only need eax, but need rax for ARG1 (or movzx) */ # ifdef MACOS /* for now we assume a BSD syscall */ - or rax, 0x2000000 + or rax, SYSCALL_NUM_MARKER_BSD # endif cmp REG_XBX, 0 je syscall_ready diff --git a/core/unix/include/syscall_mach.h b/core/unix/include/syscall_mach.h index d82ee7ebd8c..d6b23807ddd 100644 --- a/core/unix/include/syscall_mach.h +++ b/core/unix/include/syscall_mach.h @@ -38,7 +38,7 @@ #define _SYSCALL_MACH_H_ 1 #define SYSCALL_NUM_MARKER_MACH 0x1000000 -#define SYSCALL_NUM_MARKER_UNIX 0x2000000 +#define SYSCALL_NUM_MARKER_BSD 0x2000000 #define SYSCALL_NUM_MARKER_MACHDEP 0x3000000 #define MACH__kernelrpc_mach_vm_allocate_trap 10 diff --git a/core/unix/os.c b/core/unix/os.c index 78edfab33e2..b3b5692329b 100644 --- a/core/unix/os.c +++ b/core/unix/os.c @@ -4880,7 +4880,7 @@ os_normalized_sysnum(int num_raw, instr_t *gateway, dcontext_t *dcontext) } } # ifdef X64 - if (num_raw >> 24 == 0x2) + if (num_raw & SYSCALL_NUM_MARKER_BSD) return (int)(num_raw & 0xffffff); /* Drop BSD bit */ else num = (int)num_raw; /* Keep Mach and Machdep bits */ diff --git a/core/unix/signal.c b/core/unix/signal.c index 025d7c395ab..a3b4f8757e3 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -6205,7 +6205,7 @@ terminate_via_kill(dcontext_t *dcontext) * this may not exit all threads in the address space */ block_cleanup_and_terminate( - dcontext, IF_MACOS(SYSCALL_NUM_MARKER_UNIX +) SYS_kill, + dcontext, SYS_kill, /* Pass -pid in case main thread has exited * in which case will get -ESRCH */ diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index 051ad7027d4..b627ef351c4 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -100,6 +100,7 @@ set(main_run_list # our main configuration "SHORT::-code_api" # no_intercept_all_signals tests. + # TODO i#5261: Remove this suite when -intercept_all_signals is deprecated. "SHORT::ONLY::signal_intercept$::-no_code_api -no_intercept_all_signals" # sanity check: run single app to make sure these options aren't totally broken # i#1575: ARM doesn't yet support -coarse_units diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 9567df32b29..f9ba44defc3 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -45,10 +45,16 @@ #define SENTINEL 0x12345678UL +#ifdef MACOS +typedef int si_mask_t; +#else +typedef unsigned long si_mask_t; +#endif + #if !defined(MACOS) && !defined(X64) typedef struct old_sigaction_t { void (*handler)(int, siginfo_t *, void *); - unsigned int sa_mask; + si_mask_t sa_mask; unsigned long sa_flags; void (*sa_restorer)(void); } old_sigaction_t; @@ -78,13 +84,9 @@ test_query(int sig) memset((void *)&old_act, 0xff, sizeof(old_act)); rc = sigaction(sig, NULL, &old_act); assert(rc == 0 && old_act.sa_sigaction == first_act.sa_sigaction && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ -#if defined(MACOS) - *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); -#else - *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); -#endif + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ + *(si_mask_t *)&old_act.sa_mask == *(si_mask_t *)&first_act.sa_mask); /* Test with a new action. */ memset((void *)&old_act, 0xff, sizeof(old_act)); @@ -93,13 +95,9 @@ test_query(int sig) sigemptyset(&new_act.sa_mask); rc = sigaction(sig, &new_act, &old_act); assert(rc == 0 && old_act.sa_sigaction == first_act.sa_sigaction && - /* The flags do not match due to SA_RESTORER. */ - /* The rest of mask is uninit stack values from the libc wrapper. */ -#if defined(MACOS) - *(int *)&old_act.sa_mask == *(int *)&first_act.sa_mask); -#else - *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); -#endif + /* The flags do not match due to SA_RESTORER. */ + /* The rest of mask is uninit stack values from the libc wrapper. */ + *(si_mask_t *)&old_act.sa_mask == *(si_mask_t *)&first_act.sa_mask); /* Test pattern from i#1984 issue and ensure no assert. */ memset(&new_act, 0, sizeof(new_act)); @@ -239,7 +237,7 @@ test_non_rt_sigaction(int sig) assert(rc == 0 && old_act.handler == first_act.handler && /* The flags do not match due to SA_RESTORER. */ /* The rest of mask is uninit stack values from the libc wrapper. */ - *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); + *(si_mask_t *)&old_act.sa_mask == *(si_mask_t *)&first_act.sa_mask); /* Test with a new action. */ memset((void *)&old_act, 0xff, sizeof(old_act)); @@ -249,7 +247,7 @@ test_non_rt_sigaction(int sig) assert(rc == 0 && old_act.handler == first_act.handler && /* The flags do not match due to SA_RESTORER. */ /* The rest of mask is uninit stack values from the libc wrapper. */ - *(long *)&old_act.sa_mask == *(long *)&first_act.sa_mask); + *(si_mask_t *)&old_act.sa_mask == *(si_mask_t *)&first_act.sa_mask); /* Clear handler */ memset((void *)&new_act, 0, sizeof(new_act)); From 43091ed68086cd6a9eea1541ca37ee4a86d882f3 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Mon, 20 Dec 2021 16:25:45 -0500 Subject: [PATCH 29/37] Handle EFAULT due to old sigset still setting the new mask. --- core/arch/x86/x86.asm | 1 + core/drlibc/drlibc_x86.asm | 2 +- core/unix/signal.c | 18 ++++-------------- suite/tests/linux/sigaction.c | 11 +++++++++++ 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/core/arch/x86/x86.asm b/core/arch/x86/x86.asm index c5507fe91ac..444557282d0 100644 --- a/core/arch/x86/x86.asm +++ b/core/arch/x86/x86.asm @@ -642,6 +642,7 @@ cat_have_lock: #endif mov REG_XSI, [2*ARG_SZ + REG_XBP] /* sysnum */ #ifdef MACOS64 + /* For now we assume a BSD syscall */ or REG_XSI, SYSCALL_NUM_MARKER_BSD #endif pop REG_XAX /* syscall */ diff --git a/core/drlibc/drlibc_x86.asm b/core/drlibc/drlibc_x86.asm index c32f435b35a..430126c319b 100644 --- a/core/drlibc/drlibc_x86.asm +++ b/core/drlibc/drlibc_x86.asm @@ -67,7 +67,7 @@ GLOBAL_LABEL(dynamorio_syscall:) mov REG_XBX, ARG2 /* put num_args where we can reference it longer */ mov rax, ARG1 /* sysnum: only need eax, but need rax for ARG1 (or movzx) */ # ifdef MACOS - /* for now we assume a BSD syscall */ + /* For now we assume a BSD syscall */ or rax, SYSCALL_NUM_MARKER_BSD # endif cmp REG_XBX, 0 diff --git a/core/unix/signal.c b/core/unix/signal.c index a3b4f8757e3..2ff45ddfb72 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2321,19 +2321,6 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, *error_code = EINVAL; return false; } - /* On MacOS, sigprocmask does not fail if the old sigset is not readable - * or not writeable. - */ - if (IF_MACOS_ELSE(false, oset != NULL)) { - kernel_sigset_t safe_old_set; - /* Old sigset should be writable too. */ - if (!d_r_safe_read(oset, sizeof(safe_old_set), &safe_old_set) || - !safe_write_ex(oset, sizeof(safe_old_set), &safe_old_set, NULL)) { - if (error_code != NULL) - *error_code = EFAULT; - return false; - } - } /* If we're intercepting all, we emulate the whole thing */ bool execute_syscall = !DYNAMO_OPTION(intercept_all_signals); LOG(THREAD, LOG_ASYNCH, 2, "handle_sigprocmask\n"); @@ -2408,7 +2395,10 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, check_signals_pending(dcontext, info); } if (!execute_syscall) { - handle_post_sigprocmask(dcontext, how, app_set, oset, sigsetsize); + int status = handle_post_sigprocmask(dcontext, how, app_set, oset, sigsetsize); + if (status != 0 && error_code != NULL) { + *error_code = status; + } return false; /* skip syscall */ } else return true; diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index f9ba44defc3..373a4c7cccd 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -208,6 +208,17 @@ test_sigprocmask() assert(make_sigprocmask(~0, &new, (void *)0x123, 8) == -1); assert(errno == EINVAL); + /* EFAULT due to bad old sigset still sets the new mask. */ + assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 8) == 0); + new = 0x1234; + assert(make_sigprocmask(SIG_SETMASK, &new, (void *)0x123, 8) == + expected_result_bad_old_sigset); +#if !defined(MACOS) + assert(errno == EFAULT); +#endif + assert(make_sigprocmask(~0, NULL, &old, 8) == 0); + assert(new == old); + /* Restore original sigprocmask. */ assert(make_sigprocmask(SIG_SETMASK, &original, NULL, 8) == 0); } From 90c415ea905967c9fcbfdb1bc738c85c8d8feb2c Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Mon, 20 Dec 2021 16:41:39 -0500 Subject: [PATCH 30/37] Test readability improvements. --- suite/tests/linux/sigaction.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/suite/tests/linux/sigaction.c b/suite/tests/linux/sigaction.c index 373a4c7cccd..1d743021f97 100644 --- a/suite/tests/linux/sigaction.c +++ b/suite/tests/linux/sigaction.c @@ -136,14 +136,17 @@ static void test_sigprocmask() { #if defined(MACOS) - sigset_t new = 0xf00d, old, original; + sigset_t new = 0xf00d, new2 = 0x1234, old, original; /* We explicitly add SIGBUS to the blocked set as it is one of the * signals that DR intercepts. */ sigaddset(&new, SIGBUS); #else - uint64 new = 0xf00d | (1 << SIGBUS), old, original; + uint64 new = 0xf00d | (1 << SIGBUS), new2 = 0x1234, old, original; #endif + void *fault_addr = (void *)0x123; + void *read_only_addr = test_sigprocmask; + /* Save original sigprocmask. Both return the current sigprocmask. */ assert(make_sigprocmask(SIG_SETMASK, NULL, &original, /*sizeof(kernel_sigset_t)*/ 8) == 0); @@ -165,23 +168,22 @@ test_sigprocmask() * readable or not writeable. */ int expected_result_bad_old_sigset = IF_MACOS_ELSE(0, -1); - assert(make_sigprocmask(~0, NULL, (void *)0x123, 8) == - expected_result_bad_old_sigset); + assert(make_sigprocmask(~0, NULL, fault_addr, 8) == expected_result_bad_old_sigset); #if !defined(MACOS) assert(errno == EFAULT); #endif - assert(make_sigprocmask(SIG_BLOCK, (void *)0x123, NULL, 8) == -1); + assert(make_sigprocmask(SIG_BLOCK, fault_addr, NULL, 8) == -1); assert(errno == EFAULT); - assert(make_sigprocmask(SIG_BLOCK, NULL, (void *)0x123, 8) == + assert(make_sigprocmask(SIG_BLOCK, NULL, fault_addr, 8) == expected_result_bad_old_sigset); #if !defined(MACOS) assert(errno == EFAULT); #endif /* Bad new sigmask EFAULT gets reported before bad 'how' EINVAL. */ - assert(make_sigprocmask(~0, (void *)0x123, NULL, 8) == -1); + assert(make_sigprocmask(~0, fault_addr, NULL, 8) == -1); assert(errno == EFAULT); /* EFAULT due to unwritable address. */ - assert(make_sigprocmask(SIG_BLOCK, NULL, test_sigprocmask, 8) == + assert(make_sigprocmask(SIG_BLOCK, NULL, read_only_addr, 8) == expected_result_bad_old_sigset); #if !defined(MACOS) assert(errno == EFAULT); @@ -195,7 +197,7 @@ test_sigprocmask() assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 7) == -1); assert(errno == EINVAL); /* Bad size EINVAL gets reported before bad new sigmask EFAULT. */ - assert(make_sigprocmask(SIG_SETMASK, (void *)0x123, NULL, 7) == -1); + assert(make_sigprocmask(SIG_SETMASK, fault_addr, NULL, 7) == -1); assert(errno == EINVAL); #endif @@ -205,19 +207,18 @@ test_sigprocmask() assert(make_sigprocmask(SIG_SETMASK + 1, &new, NULL, 8) == -1); assert(errno == EINVAL); /* Bad 'how' EINVAL gets reported before bad old sigset EFAULT. */ - assert(make_sigprocmask(~0, &new, (void *)0x123, 8) == -1); + assert(make_sigprocmask(~0, &new, fault_addr, 8) == -1); assert(errno == EINVAL); /* EFAULT due to bad old sigset still sets the new mask. */ assert(make_sigprocmask(SIG_SETMASK, &new, NULL, 8) == 0); - new = 0x1234; - assert(make_sigprocmask(SIG_SETMASK, &new, (void *)0x123, 8) == + assert(make_sigprocmask(SIG_SETMASK, &new2, fault_addr, 8) == expected_result_bad_old_sigset); #if !defined(MACOS) assert(errno == EFAULT); #endif assert(make_sigprocmask(~0, NULL, &old, 8) == 0); - assert(new == old); + assert(new2 == old); /* Restore original sigprocmask. */ assert(make_sigprocmask(SIG_SETMASK, &original, NULL, 8) == 0); From 4abbf12f6c5419701dca09c08253db027a711119 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Mon, 20 Dec 2021 19:22:19 -0500 Subject: [PATCH 31/37] Remove signal_intercept prefix for CMake tests --- suite/tests/CMakeLists.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index b627ef351c4..e1c27a1b08b 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -101,7 +101,7 @@ set(main_run_list "SHORT::-code_api" # no_intercept_all_signals tests. # TODO i#5261: Remove this suite when -intercept_all_signals is deprecated. - "SHORT::ONLY::signal_intercept$::-no_code_api -no_intercept_all_signals" + "SHORT::LIN::ONLY::sigaction$::-no_code_api -no_intercept_all_signals" # sanity check: run single app to make sure these options aren't totally broken # i#1575: ARM doesn't yet support -coarse_units "SHORT::X86::ONLY::client.events$::-code_api -opt_memory" @@ -4144,9 +4144,9 @@ if (UNIX) tobuild(linux.sigplain101 linux/sigplain101.c) tobuild(linux.sigplain110 linux/sigplain110.c) tobuild(linux.sigplain111 linux/sigplain111.c) - tobuild(linux.sigaction.signal_intercept linux/sigaction.c) + tobuild(linux.sigaction linux/sigaction.c) # We want a native run to verify our assumptions of kernel behaviour. - torunonly_native(code_api|linux.sigaction.native linux.sigaction.signal_intercept + torunonly_native(code_api|linux.sigaction.native linux.sigaction "" linux/sigaction.c "") if (LINUX) tobuild(linux.syscall_pwait linux/syscall_pwait.cpp) @@ -4795,8 +4795,8 @@ if (APPLE) code_api|api.dis code_api|api.ir-static code_api|api.drdecode - code_api|linux.sigaction.signal_intercept - no_code_api,no_intercept_all_signals|linux.sigaction.signal_intercept + code_api|linux.sigaction + no_code_api,no_intercept_all_signals|linux.sigaction code_api|linux.sigaction.native PROPERTIES LABELS OSX) if (DEBUG) # FIXME i#1806: fails in release @@ -4830,7 +4830,7 @@ if (NOT ANDROID AND AARCHXX) code_api|linux.infinite code_api|linux.longjmp code_api|linux.readlink - code_api|linux.sigaction.signal_intercept + code_api|linux.sigaction code_api|linux.signalfd code_api|linux.signal_racesys code_api|security-common.codemod From c660d4a8a8688e9a0ff29e557cf7f39f51f2079b Mon Sep 17 00:00:00 2001 From: Abhinav Sharma Date: Tue, 21 Dec 2021 12:38:07 -0500 Subject: [PATCH 32/37] Add clarifying comment. --- core/unix/os.c | 2 +- core/unix/signal.c | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/core/unix/os.c b/core/unix/os.c index b3b5692329b..971cf7dfc8e 100644 --- a/core/unix/os.c +++ b/core/unix/os.c @@ -4880,7 +4880,7 @@ os_normalized_sysnum(int num_raw, instr_t *gateway, dcontext_t *dcontext) } } # ifdef X64 - if (num_raw & SYSCALL_NUM_MARKER_BSD) + if (TEST(SYSCALL_NUM_MARKER_BSD, num_raw)) return (int)(num_raw & 0xffffff); /* Drop BSD bit */ else num = (int)num_raw; /* Keep Mach and Machdep bits */ diff --git a/core/unix/signal.c b/core/unix/signal.c index 2ff45ddfb72..b8981c0c019 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2321,6 +2321,15 @@ handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, *error_code = EINVAL; return false; } + /* We intentionally do not check validity of oset here. This is because, + * for an unwriteable oset, the native sigprocmask syscall shows a weird + * non-atomic behavior: it returns EFAULT, but also updates the app signal + * mask. To replicate the same behavior, we allow the following code to + * update the app signal masks, which happens either by making the actual + * syscall (for -no_intercept_all_signals) or by updating app_sigblocked + * (for -intercept_all_signals). oset is checked in handle_post_sigprocmask + * and any failures are returned to the app. + */ /* If we're intercepting all, we emulate the whole thing */ bool execute_syscall = !DYNAMO_OPTION(intercept_all_signals); LOG(THREAD, LOG_ASYNCH, 2, "handle_sigprocmask\n"); From 978e09142db3d590335dad0388c9cbf1f9f165d6 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 21 Dec 2021 12:42:59 -0500 Subject: [PATCH 33/37] Add tmate --- .github/workflows/ci-osx.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index 2bea967b8f1..a11e85c121d 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,6 +81,13 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} + - name: Setup tmate session on failure + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + + - name: Setup tmate session on success + uses: mxschmitt/action-tmate@v3 + - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 From 42b4ffb4a25f06d99deca895f3a8b5b34d6f782f Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 21 Dec 2021 15:33:40 -0500 Subject: [PATCH 34/37] Revert tmate --- .github/workflows/ci-osx.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index a11e85c121d..2bea967b8f1 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,13 +81,6 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} - - name: Setup tmate session on failure - if: ${{ failure() }} - uses: mxschmitt/action-tmate@v3 - - - name: Setup tmate session on success - uses: mxschmitt/action-tmate@v3 - - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 From 4de1c96379623e5e22809e313ee657e3aa94067b Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 21 Dec 2021 19:40:36 -0500 Subject: [PATCH 35/37] Add more comments. --- core/unix/signal.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index b8981c0c019..c1336ba2d2a 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -2435,14 +2435,15 @@ handle_post_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *app_set, NULL); } if (oset != NULL) { - /* We want to handle the following cases where we are unable to fix oset - * because it is not writeable (anymore). Note that handle_sigprocmask - * does the writeability check only for non-MacOS. - * - On non-Mac systems, if oset was initially writable in our - * handle_sigprocmask check, but is unwriteable now, we want to - * return EFAULT. - * - On Mac systems, where a bad oset does not cause EFAULT, - * we want to fail the write silently. + /* Note that we check validity of oset post-syscall only, after the app's + * blocked signal mask has been updated. If there's a fault writing oset, + * the signal mask is not reverted. This is same as the native syscall + * behavior, which is also non-atomic: for a bad oset, it returns EFAULT + * but still updates the blocked signal mask. + * + * If we are unable to fix oset because it is not writeable, we return + * EFAULT. However, on Mac systems, a bad oset does not cause EFAULT + * natively, so we fail the write silently. */ if (DYNAMO_OPTION(intercept_all_signals)) { if (!safe_write_ex(oset, sizeof(*oset), &info->pre_syscall_app_sigblocked, From 55a43f20dc7383f72a629b8b80b0956c88d02ef1 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 21 Dec 2021 19:47:41 -0500 Subject: [PATCH 36/37] Add tmate. --- .github/workflows/ci-osx.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index 2bea967b8f1..a11e85c121d 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,6 +81,13 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} + - name: Setup tmate session on failure + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + + - name: Setup tmate session on success + uses: mxschmitt/action-tmate@v3 + - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2 From 0387c708e6c4f15ce2ae2b1d53b460cf13cfbfa6 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Tue, 21 Dec 2021 20:19:46 -0500 Subject: [PATCH 37/37] Revert "Add tmate." This reverts commit 55a43f20dc7383f72a629b8b80b0956c88d02ef1. --- .github/workflows/ci-osx.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/ci-osx.yml b/.github/workflows/ci-osx.yml index a11e85c121d..2bea967b8f1 100644 --- a/.github/workflows/ci-osx.yml +++ b/.github/workflows/ci-osx.yml @@ -81,13 +81,6 @@ jobs: CI_TRIGGER: ${{ github.event_name }} CI_BRANCH: ${{ github.ref }} - - name: Setup tmate session on failure - if: ${{ failure() }} - uses: mxschmitt/action-tmate@v3 - - - name: Setup tmate session on success - uses: mxschmitt/action-tmate@v3 - - name: Send failure mail to dynamorio-devs if: failure() && github.ref == 'refs/heads/master' uses: dawidd6/action-send-mail@v2