Skip to content
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

Jump to bottom

Sanitizers unblock user blocked signals #113385

Closed
vitalybuka opened this issue Oct 22, 2024 · 8 comments · Fixed by #113443 or #115790
Closed

Sanitizers unblock user blocked signals #113385

vitalybuka opened this issue Oct 22, 2024 · 8 comments · Fixed by #113443 or #115790
Assignees
@thurstond
Labels
compiler-rt regression:19 Regression in 19 release release:backport
Milestone
LLVM 19.X Release

Comments

@vitalybuka
Copy link
Collaborator

Regression after #98200
@vitalybuka vitalybuka added compiler-rt regression:19 Regression in 19 release labels Oct 22, 2024
@vitalybuka vitalybuka added this to the LLVM 19.X Release milestone Oct 22, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Oct 22, 2024
@vitalybuka
Copy link
Collaborator Author

We need to get the gurrent procmask and check sigismember before each sigdelset.

@thurstond thurstond mentioned this issue Oct 23, 2024
Merged
@tru tru moved this from Needs Triage to Needs Merge in LLVM Release Status Oct 28, 2024
@tru tru moved this from Needs Merge to Needs Fix in LLVM Release Status Oct 29, 2024
@github-project-automation github-project-automation bot moved this from Needs Fix to Done in LLVM Release Status Oct 31, 2024
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
@thurstond @vitalybuka
[sanitizer_common] AND signals in BlockSignals instead of deleting (l…
1e547fc 
…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 <[email protected]>
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
@thurstond @vitalybuka
[sanitizer_common] AND signals in BlockSignals instead of deleting (l…
beb171a 
…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 <[email protected]>
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
@thurstond @vitalybuka @NoumanAmir657
[sanitizer_common] AND signals in BlockSignals instead of deleting (l…
1e1dc6d 
…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 <[email protected]>
@thevar1able
Copy link
Contributor

thevar1able commented Nov 4, 2024 •
edited
Loading

@vitalybuka @thurstond Could you please point me to a "revert revert" commit, or reopen this issue?

@thevar1able thevar1able mentioned this issue Nov 4, 2024
Merged
20 tasks
@thurstond thurstond reopened this Nov 4, 2024
@github-project-automation github-project-automation bot moved this from Done to Needs Triage in LLVM Release Status Nov 4, 2024
@thurstond
Copy link
Contributor

@vitalybuka @thurstond Could you please point me to a "revert revert" commit, or reopen this issue?

Sorry, reopened. I didn't realize the revert didn't do that automatically.

thurstond added a commit to thurstond/llvm-project that referenced this issue Nov 11, 2024
@thurstond
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
3f043e1 
…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 thurstond mentioned this issue Nov 11, 2024
Merged
@tru tru moved this from Needs Triage to Done in LLVM Release Status Nov 12, 2024
@thurstond thurstond mentioned this issue Nov 13, 2024
Open
thurstond added a commit that referenced this issue Nov 14, 2024
@thurstond
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
531acf9 
…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
@thurstond
Copy link
Contributor

Landed #115790, hopefully the change will stick this time 🤞

@thevar1able
Copy link
Contributor

thevar1able commented Nov 15, 2024 •
edited
Loading

@thurstond Thank you! Sorry I'm not familiar with the release process, does it require a backport to be included in 19.1.4?

@thurstond
Copy link
Contributor

@thurstond Thank you! Sorry I'm not familiar with the release process, does it require a backport to be included in 19.1.4?

Yes. I'll kick off the backport in a few days, to give time for the buildbots to surface any issues with the patch in head.

@thurstond thurstond reopened this Nov 15, 2024
@github-project-automation github-project-automation bot moved this from Done to Needs Triage in LLVM Release Status Nov 15, 2024
@thurstond
Copy link
Contributor

/cherry-pick 531acf9

@llvmbot llvmbot closed this as completed Nov 18, 2024
@github-project-automation github-project-automation bot moved this from Needs Triage to Done in LLVM Release Status Nov 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 18, 2024

/pull-request #116670

akshayrdeodhar pushed a commit to akshayrdeodhar/llvm-project that referenced this issue Nov 18, 2024
@thurstond @akshayrdeodhar
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
6003a5e 
…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 issue Nov 19, 2024
@thurstond @llvmbot
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
6925f3c 
…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 issue Nov 20, 2024
@thurstond @nikic
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
10465f9 
…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 issue Dec 4, 2024
@thurstond @adrian-prantl
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
d3241df 
…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 issue Dec 5, 2024
@thurstond @adrian-prantl
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
9ec2a39 
…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 issue Dec 5, 2024
@thurstond @adrian-prantl
Reapply "[sanitizer_common] AND signals in BlockSignals instead of de…
f30e722 
…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
Assignees

@thurstond thurstond

Labels
compiler-rt regression:19 Regression in 19 release release:backport
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 19.X Release
5 participants
@thevar1able @vitalybuka @thurstond @EugeneZelenko @llvmbot