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.

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

alternative signal handler stack should last #169

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kiani45
Copy link

@kiani45 kiani45 commented Apr 29, 2020

When SignalHandling instance destroyed, signal handler alternative stack is freed.
However, the signal handler could still be called after that which causes use-after-free error.

When SignalHandling instance destroyed, signal handler alternative stack is freed.
However, the signal handler could still be called after that which causes use-after-free error.
@kiani45
Copy link
Author

kiani45 commented May 4, 2020

CPU spins when I build a memory leak program with ASAN enabled and export ASAN_OPTIONS="abort_on_error=1" in my .bashrc.

This is where this patch comes from.

Copy link
Owner

@bombela bombela left a comment

Choose a reason for hiding this comment

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

What about doing this where you instantiate it: new SignalHandling();. And just like that... its leaked.

@@ -3847,6 +3847,9 @@ class Printer {

#if defined(BACKWARD_SYSTEM_LINUX) || defined(BACKWARD_SYSTEM_DARWIN)

const size_t stack_size = 8 * 1024 * 1024;
static char *alt_stack = static_cast<char *>(malloc(stack_size));
Copy link
Owner

Choose a reason for hiding this comment

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

yes but... this allocates RAM out of the control of the user. The intend of the SignalHandling class is to be used only if you want it. If you don't you shouldn't be paying anything for it.

@bombela
Copy link
Owner

bombela commented Apr 8, 2021

So in the example backward.cpp add a new with a little comment expressing the fact that we intentionally leak it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants