-
Notifications
You must be signed in to change notification settings - Fork 60
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
Fix freebsdNN compat for [u]intptr_t syscall arguments #2167
Conversation
bsdjhb
commented
Aug 1, 2024
- freebsd64_fcntl: Remove unnecessary cast
- freebsd32/64: Map [u]intptr_t syscall arguments to [u]intXX_t
- sysent: Regen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now feeling uncomfortable about fcntl being intptr_t
on 32-bit architectures with >31-bit userspace addresses. I think that may be making kernel addresses.
Yes, I wish it was a uintptr_t arg instead (I think) |
886f93d
to
7c533c6
Compare
freebsd32_fcntl() goes out of its way to use an unsigned cast to avoid sign extension for cases where it is either a pointer or flags field. |
I've changed this to add synthetic [u]intptrXX_t types that are used in place of [u]intptr_t. This avoids the false positive matches from isptrtype() in makesyscalls.lua and trims much of the noise from the diff. |
@@ -14,6 +14,7 @@ abi_long="int32_t" | |||
abi_u_long="uint32_t" | |||
abi_semid_t="int32_t" | |||
abi_size_t="uint32_t" | |||
abi_intptr_t="intptr32_t" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see what benefit the extra type gives over just saying int32_t?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It avoids all the extra gunk due to isptrtype() matching on non-pointer instances. I didn't see an easy way to fix it in makesyscalls.lua.
sys/sys/types.h
Outdated
typedef int32_t intptr32_t; | ||
typedef uint32_t uintptr32_t; | ||
typedef int64_t intptr64_t; | ||
typedef uint64_t uintptr64_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit weird to have this on ILP32, and a bit weird for these to exist outside of compat code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally wanted to put these in <compat/freebsdNN/freebsdNN.h>, but many places include freebsdNN_proto.h without freebsdNN.h and so failed to compile. One of those places is the generated freebsdNN_sysent.c, so I tried to find a header that was already included by various places. Given that it is kernel only I could guard the typedefs with #ifdef COMPAT_FREEBSDnn.
b1eef4d
to
d701684
Compare
d701684
to
26120a8
Compare