[PATCH] D59005: Fix memory leak in CreateSigAltStack

Wed Mar 6 14:59:32 PST 2019

jfb added inline comments.

================
Comment at: llvm/lib/Support/Unix/Signals.inc:239
 static stack_t OldAltStack;
 static void* NewAltStackPointer;

----------------
This comment leads me to believe that the leak is quite intentional. What happens if you free the alt stack while it's being used by another signal handler?

================
Comment at: llvm/lib/Support/Unix/Signals.inc:255
   AltStack.ss_sp = static_cast<char *>(safe_malloc(AltStackSize));
-  NewAltStackPointer = AltStack.ss_sp; // Save to avoid reporting a leak.
   AltStack.ss_size = AltStackSize;
----------------
fez wrote:
> The leak shows up here, whenever the execution reaches this point two times in a single execution. The first time is fine, but the second time the first alternate signal stack leaks, because `free` is not called before reassigning `NewAltStackPointer`.
IIUC you need to trigger two signals? How does this happen? What are the signals that trigger the leak? Do they happen concurrently?

Maybe the answer is to hold on to the alt stacks, and free them only when there's no pending signal (or not free them, ever). That seems error-prone, and the point of my refactoring was to reduce the number of existing errors in the signal handler code (which was pretty high before).

Before we do something I think we need to understand exactly what triggers this.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59005/new/

https://reviews.llvm.org/D59005