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

Various fixes #41

Closed
wants to merge 8 commits into from
Closed

Conversation

mschlumpp
Copy link
Member

This PR consists of smaller fixes that accumulated over time. See the respective commits for a description and explanation of the changes.

Because `fetch` was not really a file, make always tried to remake the
version file. Therefore, it also needed to recompile the file as the
mtime of the version file was updated.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
While musl doesn't provide an actual implementation for the legacy
ucontext API, the header is still useful to compile software against
musl. Also, some software only relies on the definitions in the header.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
This header is deprecated but sometimes still used by software.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
The signals are currently ignored and that causes musl to `exit` the
thread, which doesn't cause any output. Therefore, it's very unclear
what happened. By calling `UK_CRASH` there is a clear indication that
something went wrong.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
On clang, this statement caused compiler errors instead of only
warnings.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
The `sys/resource.h` header adds #define-based redirection from
`prlimit64` to `prlimit` when `_GNU_SOURCE` is set. This causes our
`provided_syscalls` to incorrectly define `uk_syscall_r_prlimit` instead
of `uk_syscall_r_prlimit64`. By reordering, the headers we can side-step
this problem.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
The real one provided in `mprotect.c` works with the `mprotect` system
call introduced with the posix-mmap library.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
If these fields are not initialized as expected by musl, then calls
to pthread functions such `pthread_getspecific` can return unexpected
values or even crash.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Copy link
Member

@eduardvintila eduardvintila left a comment

Choose a reason for hiding this comment

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

Great fixes, @mschlumpp!

Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com

Copy link
Member

@mogasergiu mogasergiu left a comment

Choose a reason for hiding this comment

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

Forgot to add the RB :). LGTM.

Reviewed-by: Sergiu Moga sergiu.moga@protonmail.com

@razvand razvand added the enhancement New feature or request label May 4, 2023
@razvand razvand added this to the v0.14.0 (Prometheus) milestone May 4, 2023
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Approved-by: Razvan Deaconescu razvand@unikraft.io

unikraft-bot pushed a commit that referenced this pull request May 4, 2023
While musl doesn't provide an actual implementation for the legacy
ucontext API, the header is still useful to compile software against
musl. Also, some software only relies on the definitions in the header.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
unikraft-bot pushed a commit that referenced this pull request May 4, 2023
This header is deprecated but sometimes still used by software.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
unikraft-bot pushed a commit that referenced this pull request May 4, 2023
The signals are currently ignored and that causes musl to `exit` the
thread, which doesn't cause any output. Therefore, it's very unclear
what happened. By calling `UK_CRASH` there is a clear indication that
something went wrong.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
unikraft-bot pushed a commit that referenced this pull request May 4, 2023
On clang, this statement caused compiler errors instead of only
warnings.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
unikraft-bot pushed a commit that referenced this pull request May 4, 2023
The `sys/resource.h` header adds #define-based redirection from
`prlimit64` to `prlimit` when `_GNU_SOURCE` is set. This causes our
`provided_syscalls` to incorrectly define `uk_syscall_r_prlimit` instead
of `uk_syscall_r_prlimit64`. By reordering, the headers we can side-step
this problem.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
unikraft-bot pushed a commit that referenced this pull request May 4, 2023
The real one provided in `mprotect.c` works with the `mprotect` system
call introduced with the posix-mmap library.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
unikraft-bot pushed a commit that referenced this pull request May 4, 2023
If these fields are not initialized as expected by musl, then calls
to pthread functions such `pthread_getspecific` can return unexpected
values or even crash.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Sergiu Moga <sergiu.moga@protonmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/merged enhancement New feature or request
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

5 participants