Skip to content

Commit 50e81ef

Browse files
CheSemazvonand
authored andcommitted
Merge pull request ClickHouse#79147 from ClickHouse/chesema-reduce-stack-protection
stack allocation in Fiber and alternative stack for signals
1 parent e59958f commit 50e81ef

File tree

2 files changed

+79
-33
lines changed

2 files changed

+79
-33
lines changed

src/Common/FiberStack.cpp

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,17 @@
1818
#define MAP_ANONYMOUS MAP_ANON
1919
#endif
2020

21+
namespace
22+
{
23+
constexpr bool guardPagesEnabled()
24+
{
25+
#ifdef DEBUG_OR_SANITIZER_BUILD
26+
return true;
27+
#else
28+
return false;
29+
#endif
30+
}
31+
}
2132

2233
namespace DB::ErrorCodes
2334
{
@@ -34,33 +45,41 @@ FiberStack::FiberStack(size_t stack_size_)
3445
boost::context::stack_context FiberStack::allocate() const
3546
{
3647
size_t num_pages = 1 + (stack_size - 1) / page_size;
37-
size_t num_bytes = (num_pages + 1) * page_size; /// Add one page at bottom that will be used as guard-page
3848

39-
void * vp = ::mmap(nullptr, num_bytes, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
40-
if (MAP_FAILED == vp)
41-
throw DB::ErrnoException(DB::ErrorCodes::CANNOT_ALLOCATE_MEMORY, "FiberStack: Cannot mmap {}.", ReadableSize(num_bytes));
49+
if constexpr (guardPagesEnabled())
50+
/// Add one page at bottom that will be used as guard-page
51+
num_pages += 1;
4252

43-
/// TODO: make reports on illegal guard page access more clear.
44-
/// Currently we will see segfault and almost random stacktrace.
45-
try
46-
{
47-
memoryGuardInstall(vp, page_size);
48-
}
49-
catch (...)
53+
size_t num_bytes = num_pages * page_size;
54+
55+
void * data = aligned_alloc(page_size, num_bytes);
56+
57+
if (!data)
58+
throw DB::ErrnoException(DB::ErrorCodes::CANNOT_ALLOCATE_MEMORY, "Cannot allocate FiberStack");
59+
60+
if constexpr (guardPagesEnabled())
5061
{
51-
::munmap(vp, num_bytes);
52-
throw;
62+
/// TODO: make reports on illegal guard page access more clear.
63+
/// Currently we will see segfault and almost random stacktrace.
64+
try
65+
{
66+
memoryGuardInstall(data, page_size);
67+
}
68+
catch (...)
69+
{
70+
free(data);
71+
throw;
72+
}
5373
}
5474

55-
/// Do not count guard page in memory usage.
56-
auto trace = CurrentMemoryTracker::alloc(num_pages * page_size);
57-
trace.onAlloc(vp, num_pages * page_size);
75+
auto trace = CurrentMemoryTracker::alloc(num_bytes);
76+
trace.onAlloc(data, num_bytes);
5877

5978
boost::context::stack_context sctx;
6079
sctx.size = num_bytes;
61-
sctx.sp = static_cast< char * >(vp) + sctx.size;
80+
sctx.sp = static_cast< char * >(data) + sctx.size;
6281
#if defined(BOOST_USE_VALGRIND)
63-
sctx.valgrind_stack_id = VALGRIND_STACK_REGISTER(sctx.sp, vp);
82+
sctx.valgrind_stack_id = VALGRIND_STACK_REGISTER(sctx.sp, data);
6483
#endif
6584
return sctx;
6685
}
@@ -70,10 +89,14 @@ void FiberStack::deallocate(boost::context::stack_context & sctx) const
7089
#if defined(BOOST_USE_VALGRIND)
7190
VALGRIND_STACK_DEREGISTER(sctx.valgrind_stack_id);
7291
#endif
73-
void * vp = static_cast< char * >(sctx.sp) - sctx.size;
74-
::munmap(vp, sctx.size);
92+
void * data = static_cast< char * >(sctx.sp) - sctx.size;
93+
94+
if constexpr (guardPagesEnabled())
95+
memoryGuardRemove(data, page_size);
96+
97+
free(data);
7598

7699
/// Do not count guard page in memory usage.
77-
auto trace = CurrentMemoryTracker::free(sctx.size - page_size);
78-
trace.onFree(vp, sctx.size - page_size);
100+
auto trace = CurrentMemoryTracker::free(sctx.size);
101+
trace.onFree(data, sctx.size - page_size);
79102
}

src/Common/ThreadStatus.cpp

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ namespace ErrorCodes
2929
namespace
3030
{
3131

32+
constexpr bool guardPagesEnabled()
33+
{
34+
#ifdef DEBUG_OR_SANITIZER_BUILD
35+
return true;
36+
#else
37+
return false;
38+
#endif
39+
}
40+
3241
/// For aarch64 16K is not enough (likely due to tons of registers)
3342
constexpr size_t UNWIND_MINSIGSTKSZ = 32 << 10;
3443

@@ -48,28 +57,42 @@ struct ThreadStack
4857
{
4958
ThreadStack()
5059
{
51-
data = aligned_alloc(getPageSize(), getSize());
60+
auto page_size = getPageSize();
61+
data = aligned_alloc(page_size, getSize());
5262
if (!data)
5363
throw ErrnoException(ErrorCodes::CANNOT_ALLOCATE_MEMORY, "Cannot allocate ThreadStack");
5464

55-
try
56-
{
57-
/// Since the stack grows downward, we need to protect the first page
58-
memoryGuardInstall(data, getPageSize());
59-
}
60-
catch (...)
65+
if constexpr (guardPagesEnabled())
6166
{
62-
free(data);
63-
throw;
67+
try
68+
{
69+
/// Since the stack grows downward, we need to protect the first page
70+
memoryGuardInstall(data, page_size);
71+
}
72+
catch (...)
73+
{
74+
free(data);
75+
throw;
76+
}
6477
}
6578
}
6679
~ThreadStack()
6780
{
68-
memoryGuardRemove(data, getPageSize());
81+
if constexpr (guardPagesEnabled())
82+
memoryGuardRemove(data, getPageSize());
83+
6984
free(data);
7085
}
7186

72-
static size_t getSize() { return std::max<size_t>(UNWIND_MINSIGSTKSZ, MINSIGSTKSZ); }
87+
static size_t getSize()
88+
{
89+
auto size = std::max<size_t>(UNWIND_MINSIGSTKSZ, MINSIGSTKSZ);
90+
91+
if constexpr (guardPagesEnabled())
92+
size += 1;
93+
94+
return size;
95+
}
7396
void * getData() const { return data; }
7497

7598
private:

0 commit comments

Comments
 (0)