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

Avoid syscall.Syscall use on OpenBSD #404

Merged
merged 1 commit into from
Feb 11, 2023
Merged

Conversation

jrick
Copy link
Contributor

@jrick jrick commented Feb 11, 2023

Syscall numbers are not stable on OpenBSD, and hardcoding the msync syscall number will break bbolt on future versions of OpenBSD. Use the libc wrapper provided by golang.org/x/sys/unix instead.

This fix can be confirmed by ktrace and kdump, which on OpenBSD -current, reports whether a syscall is made through syscall(2) or not. Before:

 95150 bbolt.test CALL  (via syscall) msync(0x2f4424000,0x8000,MS_INVALIDATE)

And after:

 12149 bbolt.test CALL  msync(0x255f69000,0x8000,MS_INVALIDATE)

Syscall numbers are not stable on OpenBSD, and hardcoding the msync
syscall number will break bbolt on future versions of OpenBSD.  Use
the libc wrapper provided by golang.org/x/sys/unix instead.

Signed-off-by: Josh Rickmar <[email protected]>
@ahrtr ahrtr added this to the v1.4.0 milestone Feb 11, 2023
@ahrtr
Copy link
Member

ahrtr commented Feb 11, 2023

Thanks @jrick for the PR, which looks good to me. But have you tested it on OpenBSD?

@jrick
Copy link
Contributor Author

jrick commented Feb 11, 2023

I have. It works the same as before (which is not perfect, there is a known bug in the kernel that the bbolt tests are exposing).

@ahrtr
Copy link
Member

ahrtr commented Feb 11, 2023

there is a known bug in the kernel that the bbolt tests are exposing

Could you be more specific on this and paste the error the test exposed so that we can reference?

@jrick
Copy link
Contributor Author

jrick commented Feb 11, 2023

See this thread: https://marc.info/?t=166694295600001&r=1&w=2

@ahrtr
Copy link
Member

ahrtr commented Feb 11, 2023

Thanks.

@ahrtr ahrtr merged commit 46437ce into etcd-io:master Feb 11, 2023
@ahrtr
Copy link
Member

ahrtr commented Feb 11, 2023

We might need to backport this PR to release-1.3. any objection? @ptabor @tjungblu

@jrick jrick deleted the openbsd_libc_msync branch February 11, 2023 21:57
@jrick
Copy link
Contributor Author

jrick commented Feb 11, 2023

To add to the sense of urgency, __syscall(2) is being removed now, and syscall(2) is planned to be removed shortly after 7.3 is released.

@ahrtr
Copy link
Member

ahrtr commented Feb 15, 2023

@jrick could you please backport this PR to release-1.3?

@jrick
Copy link
Contributor Author

jrick commented Feb 15, 2023

@jrick could you please backport this PR to release-1.3?

Done: #406

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants