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

add signal stubs to runtime #4322

Closed
wants to merge 4 commits into from

Conversation

leongross
Copy link
Contributor

@leongross leongross commented Jul 2, 2024

Linux applications might ignore a certain set of received signals.
For that the tinygo runtime has to expose said signals.

Since the tinygo runtime only has limited linux signal support, this PR adds stubs to the runtime and the os package.

Package to build with this PR: https://github.com/u-root/u-root/tree/main/cmds/exp/vmboot

@leongross leongross force-pushed the os/signal/signal_recv branch 2 times, most recently from d2d5d10 to 0fbac5a Compare July 16, 2024 11:25
@leongross leongross marked this pull request as ready for review July 16, 2024 11:28
@leongross leongross force-pushed the os/signal/signal_recv branch 2 times, most recently from 0d6b074 to abf5e88 Compare July 16, 2024 11:53
@leongross
Copy link
Contributor Author

The waspi2 test seem to fail due to

/opt/hostedtoolcache/go/1.22.5/x64/src/internal/testpty/pty_cgo.go:11:10: fatal: 'fcntl.h' file not found

Building for non-wasm seems to work as expected (testes on x86_64 linux)
@deadprogram @aykevl does someone have an idea why that occurs / how to fix that?

@deadprogram
Copy link
Member

@leongross can you please rebase this PR against the latest dev to see if it still fails CI?

@leongross leongross force-pushed the os/signal/signal_recv branch from abf5e88 to 86823ea Compare July 19, 2024 08:27
@leongross leongross force-pushed the os/signal/signal_recv branch 3 times, most recently from baca864 to 09eb4c3 Compare July 29, 2024 12:13
@leongross leongross changed the title add signal stubs in runtime add signal stubs to runtime Jul 29, 2024
@leongross leongross force-pushed the os/signal/signal_recv branch from d5bb260 to 6acba3b Compare July 29, 2024 12:57
@leongross
Copy link
Contributor Author

leongross commented Aug 1, 2024

Running the a local build results in symbols not being found. Also it looks like the wrong single library is used?

ld.lld: error: undefined symbol: os/signal.signal_recv
>>> referenced by signal_unix.go:23 (/usr/lib/golang/src/os/signal/signal_unix.go:23)
>>>               /tmp/tinygo2823474359/main.lto.main.o:(os/signal.loop)

ld.lld: error: undefined symbol: os/signal.signal_enable
>>> referenced by signal_unix.go:49 (/usr/lib/golang/src/os/signal/signal_unix.go:49)
>>>               /tmp/tinygo2823474359/main.lto.main.o:(os/signal.Notify)

Searching for the strings in the binary yields the linking symbols

$ nm build/tinygo | grep "os/signal.signal_recv"
0000000000585d20 t os/signal.signal_recv

The lubmusl error can be traced down to this include in muslbic that seems to miss fcnt.h in x86_64.

@leongross leongross force-pushed the os/signal/signal_recv branch from c01d23a to 99bc6f0 Compare August 2, 2024 12:48
@leongross
Copy link
Contributor Author

@dgryski any ideas why this might fail? I saw you worked on the runtime before.

@dgryski
Copy link
Member

dgryski commented Aug 2, 2024

The os/signal entry in goroot.go is still commented out. I think that needs to be set to false since we have our own complete implementation of that package now. Without that it's trying to run the Big Go os/signal tests which fail because our syscall packages are different.

@leongross leongross force-pushed the os/signal/signal_recv branch from 94c5aa6 to 70fda27 Compare August 2, 2024 14:19
@aykevl
Copy link
Member

aykevl commented Aug 2, 2024

@leongross I looked at how difficult it would be to support signals (instead of just stubbing them out), and got it working on Linux: #4378
What do you think?

(Note: if you try this, make sure to run tinygo clean so that musl is rebuilt. Otherwise signals will just result in a hang or crash.)

@leongross leongross mentioned this pull request Sep 2, 2024
@leongross
Copy link
Contributor Author

This PR is superseded by #4378.

@leongross leongross closed this Oct 10, 2024
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.

4 participants