Merge pull request #4206 from hercules-ci/fix-coroutine-gc

Fix memory corruption caused by GC-invisible coroutine stacks
This commit is contained in:
Eelco Dolstra 2020-11-05 10:18:31 +01:00 committed by GitHub
commit 387f824cab
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 80 additions and 2 deletions

View file

@ -27,6 +27,10 @@
#include <gc/gc.h> #include <gc/gc.h>
#include <gc/gc_cpp.h> #include <gc/gc_cpp.h>
#include <boost/coroutine2/coroutine.hpp>
#include <boost/coroutine2/protected_fixedsize_stack.hpp>
#include <boost/context/stack_context.hpp>
#endif #endif
namespace nix { namespace nix {
@ -220,6 +224,31 @@ static void * oomHandler(size_t requested)
/* Convert this to a proper C++ exception. */ /* Convert this to a proper C++ exception. */
throw std::bad_alloc(); throw std::bad_alloc();
} }
class BoehmGCStackAllocator : public StackAllocator {
boost::coroutines2::protected_fixedsize_stack stack {
// We allocate 8 MB, the default max stack size on NixOS.
// A smaller stack might be quicker to allocate but reduces the stack
// depth available for source filter expressions etc.
std::max(boost::context::stack_traits::default_size(), static_cast<std::size_t>(8 * 1024 * 1024))
};
public:
boost::context::stack_context allocate() override {
auto sctx = stack.allocate();
GC_add_roots(static_cast<char *>(sctx.sp) - sctx.size, sctx.sp);
return sctx;
}
void deallocate(boost::context::stack_context sctx) override {
GC_remove_roots(static_cast<char *>(sctx.sp) - sctx.size, sctx.sp);
stack.deallocate(sctx);
}
};
static BoehmGCStackAllocator boehmGCStackAllocator;
#endif #endif
@ -257,6 +286,8 @@ void initGC()
GC_set_oom_fn(oomHandler); GC_set_oom_fn(oomHandler);
StackAllocator::defaultAllocator = &boehmGCStackAllocator;
/* Set the initial heap size to something fairly big (25% of /* Set the initial heap size to something fairly big (25% of
physical RAM, up to a maximum of 384 MiB) so that in most cases physical RAM, up to a maximum of 384 MiB) so that in most cases
we don't need to garbage collect at all. (Collection has a we don't need to garbage collect at all. (Collection has a

View file

@ -15,7 +15,7 @@ libexpr_CXXFLAGS += -I src/libutil -I src/libstore -I src/libfetchers -I src/lib
libexpr_LIBS = libutil libstore libfetchers libexpr_LIBS = libutil libstore libfetchers
libexpr_LDFLAGS = libexpr_LDFLAGS = -lboost_context
ifneq ($(OS), FreeBSD) ifneq ($(OS), FreeBSD)
libexpr_LDFLAGS += -ldl libexpr_LDFLAGS += -ldl
endif endif

View file

@ -171,6 +171,39 @@ size_t StringSource::read(unsigned char * data, size_t len)
#error Coroutines are broken in this version of Boost! #error Coroutines are broken in this version of Boost!
#endif #endif
/* A concrete datatype allow virtual dispatch of stack allocation methods. */
struct VirtualStackAllocator {
StackAllocator *allocator = StackAllocator::defaultAllocator;
boost::context::stack_context allocate() {
return allocator->allocate();
}
void deallocate(boost::context::stack_context sctx) {
allocator->deallocate(sctx);
}
};
/* This class reifies the default boost coroutine stack allocation strategy with
a virtual interface. */
class DefaultStackAllocator : public StackAllocator {
boost::coroutines2::default_stack stack;
boost::context::stack_context allocate() {
return stack.allocate();
}
void deallocate(boost::context::stack_context sctx) {
deallocate(sctx);
}
};
static DefaultStackAllocator defaultAllocatorSingleton;
StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton;
std::unique_ptr<Source> sinkToSource( std::unique_ptr<Source> sinkToSource(
std::function<void(Sink &)> fun, std::function<void(Sink &)> fun,
std::function<void()> eof) std::function<void()> eof)
@ -195,7 +228,7 @@ std::unique_ptr<Source> sinkToSource(
size_t read(unsigned char * data, size_t len) override size_t read(unsigned char * data, size_t len) override
{ {
if (!coro) if (!coro)
coro = coro_t::pull_type([&](coro_t::push_type & yield) { coro = coro_t::pull_type(VirtualStackAllocator{}, [&](coro_t::push_type & yield) {
LambdaSink sink([&](const unsigned char * data, size_t len) { LambdaSink sink([&](const unsigned char * data, size_t len) {
if (len) yield(std::string((const char *) data, len)); if (len) yield(std::string((const char *) data, len));
}); });

View file

@ -5,6 +5,7 @@
#include "types.hh" #include "types.hh"
#include "util.hh" #include "util.hh"
namespace boost::context { struct stack_context; }
namespace nix { namespace nix {
@ -497,5 +498,18 @@ struct FramedSink : nix::BufferedSink
}; };
}; };
/* Stack allocation strategy for sinkToSource.
Mutable to avoid a boehm gc dependency in libutil.
boost::context doesn't provide a virtual class, so we define our own.
*/
struct StackAllocator {
virtual boost::context::stack_context allocate() = 0;
virtual void deallocate(boost::context::stack_context sctx) = 0;
/* The stack allocator to use in sinkToSource and potentially elsewhere.
It is reassigned by the initGC() method in libexpr. */
static StackAllocator *defaultAllocator;
};
} }