[sanitizer_common] Block asynchronous signals only #98200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This does not change the existing behavior of sanitizers, since BlockSignal will still default to blocking (nearly) all signals. This extension is intended to be used in a future fix for MSan (block async signals during MsanThread::Destroy).
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) ChangesThis does not change the existing behavior of sanitizers, since BlockSignal will still default to blocking (nearly) all signals. This extension is intended to be used in a future fix for MSan (block async signals during MsanThread::Destroy). Full diff: https://github.com/llvm/llvm-project/pull/98200.diff 2 Files Affected:
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
index 12df3ef73da4b..fa32ab998bbd9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
@@ -155,7 +155,9 @@ void SetSigProcMask(__sanitizer_sigset_t *set, __sanitizer_sigset_t *oldset) {
CHECK_EQ(0, internal_sigprocmask(SIG_SETMASK, set, oldset));
}
-void BlockSignals(__sanitizer_sigset_t *oldset) {
+// If block_async_only is true: blocks only asynchronous signals; otherwise,
+// blocks (nearly) all signals.
+void BlockSignals(__sanitizer_sigset_t *oldset, bool block_async_only) {
__sanitizer_sigset_t set;
internal_sigfillset(&set);
# if SANITIZER_LINUX && !SANITIZER_ANDROID
@@ -170,11 +172,21 @@ void BlockSignals(__sanitizer_sigset_t *oldset) {
// hang.
internal_sigdelset(&set, 31);
# endif
+ if (block_async_only) {
+ internal_sigdelset(&set, SIGSEGV);
+ internal_sigdelset(&set, SIGBUS);
+ internal_sigdelset(&set, SIGILL);
+ internal_sigdelset(&set, SIGTRAP);
+ internal_sigdelset(&set, SIGABRT);
+ internal_sigdelset(&set, SIGFPE);
+ internal_sigdelset(&set, SIGPIPE);
+ }
SetSigProcMask(&set, oldset);
}
-ScopedBlockSignals::ScopedBlockSignals(__sanitizer_sigset_t *copy) {
- BlockSignals(&saved_);
+ScopedBlockSignals::ScopedBlockSignals(__sanitizer_sigset_t *copy,
+ bool block_async_only) {
+ BlockSignals(&saved_, block_async_only);
if (copy)
internal_memcpy(copy, &saved_, sizeof(saved_));
}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.h b/compiler-rt/lib/sanitizer_common/sanitizer_linux.h
index c30f0326793d5..7f3da79a2c03b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.h
@@ -51,9 +51,11 @@ uptr internal_sigprocmask(int how, __sanitizer_sigset_t *set,
__sanitizer_sigset_t *oldset);
void SetSigProcMask(__sanitizer_sigset_t *set, __sanitizer_sigset_t *oldset);
-void BlockSignals(__sanitizer_sigset_t *oldset = nullptr);
+void BlockSignals(__sanitizer_sigset_t *oldset = nullptr,
+ bool block_async_only = false);
struct ScopedBlockSignals {
- explicit ScopedBlockSignals(__sanitizer_sigset_t *copy);
+ explicit ScopedBlockSignals(__sanitizer_sigset_t *copy,
+ bool block_async_only = false);
~ScopedBlockSignals();
ScopedBlockSignals &operator=(const ScopedBlockSignals &) = delete;
|
vitalybuka
reviewed
Jul 10, 2024
Done |
vitalybuka
approved these changes
Jul 10, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/1697 Here is the relevant piece of the build log for the reference: |
|
This patch broke the Solaris/amd64 buildbot. Solaris currently lacks |
thurstond
added a commit
that referenced
this pull request
Jul 10, 2024
My patch broke Solaris: #98200 (comment) Fixing forward by limiting the change to Linux only.
Thanks for the report and sorry for the inconvenience! I've fixed-forward in 6789e1b to limit the change to Linux only. |
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
This changes the behavior of `BlockSignals` and `ScopedBlockSignals` to block only asynchronous signals. This extension is intended to be used in a future fix for MSan (block async signals during `MsanThread::Destroy`).
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
My patch broke Solaris: llvm#98200 (comment) Fixing forward by limiting the change to Linux only.
20 tasks
|
With |
|
I see what you mean. Your program is blocking signal, and we unblock it? |
|
@vitalybuka Right. I verified that removing this line fixes the issue. |
We should just update sigmask more carefully, we just somehow missed this. |
thurstond
added a commit
that referenced
this pull request
Oct 31, 2024
…113443) My earlier patch #98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes #113385 --------- Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…lvm#113443) My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 --------- Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…lvm#113443) My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 --------- Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…lvm#113443) My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 --------- Co-authored-by: Vitaly Buka <vitalybuka@gmail.com>
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Nov 11, 2024
…leting (llvm#113443)" for non-Android Linux only The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - No-op the change for Android: this had mysterious (but reproducible) test failures. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigismember. - Two other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Nov 13, 2024
thurstond
added a commit
that referenced
this pull request
Nov 14, 2024
…leting (#113443)" for non-Android Linux only (#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch #98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes #113385
akshayrdeodhar
pushed a commit
to akshayrdeodhar/llvm-project
that referenced
this pull request
Nov 18, 2024
…leting (llvm#113443)" for non-Android Linux only (llvm#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-llvm#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385
tru
pushed a commit
that referenced
this pull request
Nov 19, 2024
…leting (#113443)" for non-Android Linux only (#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch #98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes #113385 (cherry picked from commit 531acf9)
nikic
pushed a commit
to rust-lang/llvm-project
that referenced
this pull request
Nov 20, 2024
…leting (llvm#113443)" for non-Android Linux only (llvm#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-llvm#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 (cherry picked from commit 531acf9)
adrian-prantl
pushed a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Dec 4, 2024
…leting (llvm#113443)" for non-Android Linux only (llvm#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-llvm#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 (cherry picked from commit 531acf9) (cherry picked from commit 6925f3c)
adrian-prantl
pushed a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Dec 5, 2024
…leting (llvm#113443)" for non-Android Linux only (llvm#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-llvm#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 (cherry picked from commit 531acf9) (cherry picked from commit 6925f3c)
adrian-prantl
pushed a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Dec 5, 2024
…leting (llvm#113443)" for non-Android Linux only (llvm#115790) The original patch (25fd366) was reverted in 083a5cd because it broke some buildbots. This revised patch makes two changes: - Reverts to *pre-llvm#98200* behavior for Android. This avoids a build breakage on Android. - Only define KeepUnblocked if SANITIZER_LINUX: this avoids a build breakage on solaris, which does not support internal_sigdelset. N.B. Other buildbot failures were non-sanitizer tests and are therefore unrelated. Original commit message: My earlier patch llvm#98200 caused a regression because it unconditionally unblocked synchronous signals, even if the user program had deliberately blocked them. This patch fixes the issue by checking the current signal mask, as suggested by Vitaly. It also adds tests. Fixes llvm#113385 (cherry picked from commit 531acf9) (cherry picked from commit 6925f3c)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the behavior of
BlockSignalsandScopedBlockSignalsto block only asynchronous signals.This extension is intended to be used in a future fix for MSan (block async signals during
MsanThread::Destroy).