Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fakecgo: C functions require nosplit #295

Merged
merged 3 commits into from
Jan 6, 2025

Conversation

TotallyGamerJet
Copy link
Collaborator

fakecgo runs on the system stack so we shouldn't ever split the stack.

I also updated the size of sigset_t to be the correct size based on the headers. This was needed to make it possible to build with inlining disabled when nosplit was added.

Fixes #287

fakecgo runs on systemstack so we shouldn't ever split the stack.

I also updated the size of sigset_t to be the correct size based on the headers. This was needed to make it possible to build with inlining disabled when nosplit was added.

Fixes ebitengine#287
// Sources:
// Darwin - https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/sys/_types.h#L74
// FreeBSD - https://github.com/DoctorWkt/xv6-freebsd/blob/d2a294c2a984baed27676068b15ed9a29b06ab6f/include/signal.h#L98C9-L98C21
sigset_t [32]byte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about Linux?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh it is 128 bytes. Not sure what to do since if it's 128 then it's too big to build when inlining is disabled. I guess inline the functions by hand but that's kind of ugly.

https://github.com/torvalds/linux/blob/ab75170520d4964f3acf8bb1f91d34cbc650688e/arch/x86/include/asm/signal.h#L25

github.com/ebitengine/purego/internal/fakecgo.x_cgo_thread_start: nosplit stack over 792 byte limit
github.com/ebitengine/purego/internal/fakecgo.x_cgo_thread_start<1>
    grows 160 bytes, calls github.com/ebitengine/purego/internal/fakecgo._cgo_sys_thread_start<1>
        grows 400 bytes, calls github.com/ebitengine/purego/internal/fakecgo._cgo_try_pthread_create<1>
            grows 112 bytes, calls github.com/ebitengine/purego/internal/fakecgo.pthread_create<1>
                grows 128 bytes, calls github.com/ebitengine/purego/internal/fakecgo.call5<0>
                    grows 16 bytes, calls indirect
                        grows 0 bytes, calls runtime.morestack<0>
                        24 bytes over limit
            grows 112 bytes, calls github.com/ebitengine/purego/internal/fakecgo.nanosleep<1>
                grows 112 bytes, calls github.com/ebitengine/purego/internal/fakecgo.call5<0>
                    grows 16 bytes, calls indirect
                        grows 0 bytes, calls runtime.morestack<0>
                        8 bytes over limit

Copy link
Member

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM

@TotallyGamerJet TotallyGamerJet merged commit a32290b into ebitengine:main Jan 6, 2025
28 checks passed
@TotallyGamerJet TotallyGamerJet deleted the issue-287 branch January 6, 2025 14:47
hajimehoshi pushed a commit that referenced this pull request Jan 6, 2025
* fakecgo: C functions require nosplit

fakecgo runs on systemstack so we shouldn't ever split the stack.

Fixes #287
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

env CGO_ENABLED=0 go test "-gcflags=all=-N -l" -v ./... causes SEGV
2 participants