Skip to content

Commit

Permalink
Fix main stack top detection on musl-libc (#15047)
Browse files Browse the repository at this point in the history
Stack overflow detection relies on the current thread's stack bounds. On Linux this is obtained using `LibC.pthread_getattr_np` and `LibC.pthread_attr_getstack`.

However, on musl-libc the internal stack size is always [hardcoded](https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_create.c?id=dd1e63c3638d5f9afb857fccf6ce1415ca5f1b8b#n267) to a very small [default](https://git.musl-libc.org/cgit/musl/tree/src/internal/pthread_impl.h?id=dd1e63c3638d5f9afb857fccf6ce1415ca5f1b8b#n197) of [128 KiB](https://git.musl-libc.org/cgit/musl/tree/src/thread/default_attr.c?id=dd1e63c3638d5f9afb857fccf6ce1415ca5f1b8b#n3), and [`pthread_getattr_np` returns this value unmodified](https://git.musl-libc.org/cgit/musl/tree/src/thread/pthread_getattr_np.c?id=dd1e63c3638d5f9afb857fccf6ce1415ca5f1b8b#n13), presumably for the main thread as well. The result is that the main thread has an entirely incorrect `@stack`, as it respects `ulimit -s`, and that non-main threads are really that small, as we don't pass any `attr` to `GC.pthread_create` on thread creation. [This is also mentioned on Ruby's bug tracker](https://bugs.ruby-lang.org/issues/14387#change-70914).
  • Loading branch information
HertzDevil authored Sep 30, 2024
1 parent 2821fbb commit 46ca8fb
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 17 deletions.
23 changes: 9 additions & 14 deletions spec/std/kernel_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -254,27 +254,22 @@ describe "hardware exception" do
error.should_not contain("Stack overflow")
end

{% if flag?(:musl) %}
# FIXME: Pending as mitigation for https://github.com/crystal-lang/crystal/issues/7482
pending "detects stack overflow on the main stack"
{% else %}
it "detects stack overflow on the main stack", tags: %w[slow] do
# This spec can take some time under FreeBSD where
# the default stack size is 0.5G. Setting a
# smaller stack size with `ulimit -s 8192`
# will address this.
status, _, error = compile_and_run_source <<-'CRYSTAL'
it "detects stack overflow on the main stack", tags: %w[slow] do
# This spec can take some time under FreeBSD where
# the default stack size is 0.5G. Setting a
# smaller stack size with `ulimit -s 8192`
# will address this.
status, _, error = compile_and_run_source <<-'CRYSTAL'
def foo
y = StaticArray(Int8, 512).new(0)
foo
end
foo
CRYSTAL

status.success?.should be_false
error.should contain("Stack overflow")
end
{% end %}
status.success?.should be_false
error.should contain("Stack overflow")
end

it "detects stack overflow on a fiber stack", tags: %w[slow] do
status, _, error = compile_and_run_source <<-'CRYSTAL'
Expand Down
29 changes: 26 additions & 3 deletions src/crystal/system/unix/pthread.cr
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,26 @@ module Crystal::System::Thread
ret = LibC.pthread_attr_destroy(pointerof(attr))
raise RuntimeError.from_os_error("pthread_attr_destroy", Errno.new(ret)) unless ret == 0
{% elsif flag?(:linux) %}
if LibC.pthread_getattr_np(@system_handle, out attr) == 0
LibC.pthread_attr_getstack(pointerof(attr), pointerof(address), out _)
end
ret = LibC.pthread_getattr_np(@system_handle, out attr)
raise RuntimeError.from_os_error("pthread_getattr_np", Errno.new(ret)) unless ret == 0

LibC.pthread_attr_getstack(pointerof(attr), pointerof(address), out stack_size)

ret = LibC.pthread_attr_destroy(pointerof(attr))
raise RuntimeError.from_os_error("pthread_attr_destroy", Errno.new(ret)) unless ret == 0

# with musl-libc, the main thread does not respect `rlimit -Ss` and
# instead returns the same default stack size as non-default threads, so
# we obtain the rlimit to correct the stack address manually
{% if flag?(:musl) %}
if Thread.current_is_main?
if LibC.getrlimit(LibC::RLIMIT_STACK, out rlim) == 0
address = address + stack_size - rlim.rlim_cur
else
raise RuntimeError.from_errno("getrlimit")
end
end
{% end %}
{% elsif flag?(:openbsd) %}
ret = LibC.pthread_stackseg_np(@system_handle, out stack)
raise RuntimeError.from_os_error("pthread_stackseg_np", Errno.new(ret)) unless ret == 0
Expand All @@ -153,6 +168,14 @@ module Crystal::System::Thread
address
end

{% if flag?(:musl) %}
@@main_handle : Handle = current_handle

def self.current_is_main?
current_handle == @@main_handle
end
{% end %}

# Warning: must be called from the current thread itself, because Darwin
# doesn't allow to set the name of any thread but the current one!
private def system_name=(name : String) : String
Expand Down
11 changes: 11 additions & 0 deletions src/lib_c/aarch64-linux-musl/c/sys/resource.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
lib LibC
alias RlimT = ULongLong

struct Rlimit
rlim_cur : RlimT
rlim_max : RlimT
end

fun getrlimit(Int, Rlimit*) : Int

RLIMIT_STACK = 3

struct RUsage
ru_utime : Timeval
ru_stime : Timeval
Expand Down
11 changes: 11 additions & 0 deletions src/lib_c/i386-linux-musl/c/sys/resource.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
lib LibC
alias RlimT = ULongLong

struct Rlimit
rlim_cur : RlimT
rlim_max : RlimT
end

fun getrlimit(Int, Rlimit*) : Int

RLIMIT_STACK = 3

struct RUsage
ru_utime : Timeval
ru_stime : Timeval
Expand Down
11 changes: 11 additions & 0 deletions src/lib_c/x86_64-linux-musl/c/sys/resource.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
lib LibC
alias RlimT = ULongLong

struct Rlimit
rlim_cur : RlimT
rlim_max : RlimT
end

fun getrlimit(Int, Rlimit*) : Int

RLIMIT_STACK = 3

struct RUsage
ru_utime : Timeval
ru_stime : Timeval
Expand Down

0 comments on commit 46ca8fb

Please sign in to comment.