[llvm-bugs] [Bug 40966] New: Memory Leak in CreateSigAltStack
via llvm-bugs
llvm-bugs at lists.llvm.org
Tue Mar 5 09:53:34 PST 2019
https://bugs.llvm.org/show_bug.cgi?id=40966
Bug ID: 40966
Summary: Memory Leak in CreateSigAltStack
Product: libraries
Version: 7.0
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P
Component: Support Libraries
Assignee: unassignedbugs at nondot.org
Reporter: pietro at rev.ng
CC: llvm-bugs at lists.llvm.org
In lib/Support/Unix/Signals.inc there is an occasional memory leak, whenever
the function CreateSigAltStack() is called multiple times with certain
conditions.
To reproduce the bug, just run
valgrind --leak-check=full opt -o /dev/null /dev/null
OS: Ubuntu 18.04
Valgrind version: 3.13.0
llvm version: 7.0.0 (opt-7 system installation in ubuntu, but for what I saw in
the sources it affects llvm 8.0.0 as well)
You should be on a Unix system with sigaltstack available.
This is the leak report I get from Valgrind
$ valgrind --leak-check=full opt-7 -o /dev/null /dev/null
==27771== Memcheck, a memory error detector
==27771== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27771== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==27771== Command: opt-7 -o /dev/null /dev/null
==27771==
==27771==
==27771== HEAP SUMMARY:
==27771== in use at exit: 67,640 bytes in 3 blocks
==27771== total heap usage: 3,488 allocs, 3,485 frees, 1,069,258 bytes
allocated
==27771==
==27771== 67,584 bytes in 1 blocks are definitely lost in loss record 3 of 3
==27771== at 0x4C2FB0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==27771== by 0x59622D3: ??? (in /usr/lib/x86_64-linux-gnu/libLLVM-7.so.1)
==27771== by 0x5909CED: llvm::EnablePrettyStackTrace() (in
/usr/lib/x86_64-linux-gnu/libLLVM-7.so.1)
==27771== by 0x58F2C21: llvm::InitLLVM::InitLLVM(int&, char const**&) (in
/usr/lib/x86_64-linux-gnu/libLLVM-7.so.1)
==27771== by 0x1B43F7: main (in /usr/lib/llvm-7/bin/opt)
==27771==
==27771== LEAK SUMMARY:
==27771== definitely lost: 67,584 bytes in 1 blocks
==27771== indirectly lost: 0 bytes in 0 blocks
==27771== possibly lost: 0 bytes in 0 blocks
==27771== still reachable: 56 bytes in 2 blocks
==27771== suppressed: 0 bytes in 0 blocks
==27771== Reachable blocks (those to which a pointer was found) are not shown.
==27771== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==27771==
==27771== For counts of detected and suppressed errors, rerun with: -v
==27771== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
I actually tracked it down in a version compiled from source with all the debug
symbols. The leak report is very similar, except that I get the exact info on
the leaking allocation:
==20704== HEAP SUMMARY:
==20704== in use at exit: 1,020,675 bytes in 35 blocks
==20704== total heap usage: 292,536 allocs, 292,501 frees, 49,368,073 bytes
allocated
==20704==
==20704== 67,584 bytes in 1 blocks are definitely lost in loss record 14 of 27
==20704== at 0x4C2FB0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20704== by 0x893D63C: safe_malloc (MemAlloc.h:27)
==20704== by 0x893D63C: CreateSigAltStack (Signals.inc:254)
==20704== by 0x893D63C: RegisterHandlers() (Signals.inc:277)
==20704== by 0x893DF9C: llvm::sys::AddSignalHandler(void (*)(void*), void*)
(Signals.inc:386)
==20704== by 0x88E538D: RegisterCrashPrinter (PrettyStackTrace.cpp:180)
==20704== by 0x88E538D: llvm::EnablePrettyStackTrace()
(PrettyStackTrace.cpp:188)
==20704== by 0x88CFFED: PrettyStackTraceProgram (PrettyStackTrace.h:77)
==20704== by 0x88CFFED: llvm::InitLLVM::InitLLVM(int&, char const**&)
(InitLLVM.cpp:25)
==20704== by 0x41DF07: InitLLVM (InitLLVM.h:35)
==20704== by 0x41DF07: main (opt.cpp:426)
I found out the cause. Here's the code with a little bit of context.
#if defined(HAVE_SIGALTSTACK)
// Hold onto both the old and new alternate signal stack so that it's not
// reported as a leak. We don't make any attempt to remove our alt signal
// stack if we remove our signal handlers; that can't be done reliably if
// someone else is also trying to do the same thing.
static stack_t OldAltStack;
static void* NewAltStackPointer;
static void CreateSigAltStack() {
const size_t AltStackSize = MINSIGSTKSZ + 64 * 1024;
// If we're executing on the alternate stack, or we already have an alternate
// signal stack that we're happy with, there's nothing for us to do. Don't
// reduce the size, some other part of the process might need a larger stack
// than we do.
if (sigaltstack(nullptr, &OldAltStack) != 0 ||
OldAltStack.ss_flags & SS_ONSTACK ||
(OldAltStack.ss_sp && OldAltStack.ss_size >= AltStackSize))
return;
stack_t AltStack = {};
AltStack.ss_sp = static_cast<char *>(safe_malloc(AltStackSize));
NewAltStackPointer = AltStack.ss_sp; // Save to avoid reporting a leak.
AltStack.ss_size = AltStackSize;
if (sigaltstack(&AltStack, &OldAltStack) != 0)
free(AltStack.ss_sp);
}
#else
The leak shows up whenever CreateSigAltStack is called multiple times, in some
specific conditions.
The first time CreateSigAltStack() is called, a new alternate signal stack
isallocated with
AltStack.ss_sp = static_cast<char *>(safe_malloc(AltStackSize));
Then the value of AltStack.ss_sp is saved into NewAltStackPointer. According to
the comments, this is a workaround to stop valgrind or asan complaining about a
leak, but it's not enough. Here's why. Whenever CreateSigAltStack is called a
second time and it reaches past the first if, a new larger alternate stack is
allocated again with
AltStack.ss_sp = static_cast<char *>(safe_malloc(AltStackSize));
But at this point, the NewAltStackPointer is overwritten, and the old alternate
signal stack is never free'd, and we hold a pointer to it anymore, causing the
leak.
I also figured out a workaround:
diff --git a/lib/Support/Unix/Signals.inc b/lib/Support/Unix/Signals.inc
index de26695d64e..813ddea2833 100644
--- a/lib/Support/Unix/Signals.inc
+++ b/lib/Support/Unix/Signals.inc
@@ -252,10 +252,13 @@ static void CreateSigAltStack() {
stack_t AltStack = {};
AltStack.ss_sp = static_cast<char *>(safe_malloc(AltStackSize));
- NewAltStackPointer = AltStack.ss_sp; // Save to avoid reporting a leak.
AltStack.ss_size = AltStackSize;
- if (sigaltstack(&AltStack, &OldAltStack) != 0)
+ if (sigaltstack(&AltStack, &OldAltStack) != 0) {
free(AltStack.ss_sp);
+ } else {
+ free(NewAltStackPointer);
+ NewAltStackPointer = AltStack.ss_sp;
+ }
}
#else
static void CreateSigAltStack() {}
With this fix the leak should go away. Valgrind leak report report goes away as
well.
It's the first time I file a bug report to LLVM, so I'm not sure what to do.
Is this the correct place for the report of it's better to file the patch for
review on Phabricator?
Thanks,
Pietro
--
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190305/6790b983/attachment.html>
More information about the llvm-bugs
mailing list