Skip to content

Reapply "[sanitizer_common] AND signals in BlockSignals instead of deleting (#113443)" for non-Android Linux only #115790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 14, 2024
Merged

Reapply "[sanitizer_common] AND signals in BlockSignals instead of deleting (#113443)" for non-Android Linux only #115790

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3f043e1
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
thurstond Nov 11, 2024
e88b5fc
Revert to pre-#98200 behavior for Android
thurstond Nov 13, 2024
5789cca
Add FIXME
thurstond Nov 13, 2024
7558d41
Add https://github.com/google/sanitizers/issues/1816 to FIXMEs
thurstond Nov 13, 2024
8ec1550
Add SANITIZER_LINUX guard
thurstond Nov 13, 2024
625f812
Refactor
thurstond Nov 14, 2024
a6988aa
Fix logic error pointed out by Vitaly
thurstond Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,6 @@ void SetSigProcMask(__sanitizer_sigset_t *set, __sanitizer_sigset_t *oldset) {
// Equivalently: newset[signum] = newset[signum] & oldset[signum]
static void KeepUnblocked(__sanitizer_sigset_t &newset,
__sanitizer_sigset_t &oldset, int signum) {
// FIXME: Figure out why Android tests fail with internal_sigismember
// See also: comment in BlockSignals().
// In the meantime, we prefer to sometimes incorrectly unblock signals.
if (SANITIZER_ANDROID || !internal_sigismember(&oldset, signum))
internal_sigdelset(&newset, signum);
}
Expand All @@ -181,9 +178,7 @@ static void KeepUnblocked(__sanitizer_sigset_t &newset,
void BlockSignals(__sanitizer_sigset_t *oldset) {
__sanitizer_sigset_t currentset;
Copy link
Collaborator

@vitalybuka vitalybuka Nov 13, 2024

Choose a reason for hiding this comment

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

unused current set warning is possible on non-linux.

Copy link
Contributor Author

@thurstond thurstond Nov 13, 2024

Choose a reason for hiding this comment

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

Thanks, will fix

Copy link
Collaborator

@vitalybuka vitalybuka Nov 14, 2024

Choose a reason for hiding this comment

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

we still need

__sanitizer_sigset_t newset;
internal_sigfillset(&newset);

# if SANITIZER_LINUX
...
#endif

SetSigProcMask(&newset, oldset);

Copy link
Contributor Author

@thurstond thurstond Nov 14, 2024

Choose a reason for hiding this comment

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

Oops, thank you!

# if !SANITIZER_ANDROID
// FIXME: SetSigProcMask and pthread_sigmask cause mysterious failures
// See also: comment in KeepUnblocked().
// In the meantime, we prefer to sometimes incorrectly unblock signals.
// FIXME: SetSigProcMask cause mysterious failures on Android
Copy link
Contributor

@fmayer fmayer Nov 13, 2024

Choose a reason for hiding this comment

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

can we create some reference? a bug in llvm or google/sanitizers repo that has the logs of the failing buildbot run?

Copy link
Contributor Author

@thurstond thurstond Nov 13, 2024

Choose a reason for hiding this comment

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

Filed google/sanitizers#1816 and updated the FIXMEs

SetSigProcMask(NULL, &currentset);
# endif

Expand All @@ -200,9 +195,12 @@ void BlockSignals(__sanitizer_sigset_t *oldset) {
// If this signal is blocked, such calls cannot be handled and the process may
// hang.
KeepUnblocked(newset, currentset, 31);
# endif

# if SANITIZER_LINUX && !SANITIZER_ANDROID
// Don't block synchronous signals
// but also don't unblock signals that the user had deliberately blocked.
// FIXME: this causes mysterious failures on Android
KeepUnblocked(newset, currentset, SIGSEGV);
KeepUnblocked(newset, currentset, SIGBUS);
KeepUnblocked(newset, currentset, SIGILL);
Expand Down
Loading