-
Notifications
You must be signed in to change notification settings - Fork 625
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 poll_oneoff, size_t < 8 bytes on some platforms #2152
Fix poll_oneoff, size_t < 8 bytes on some platforms #2152
Conversation
g0djan
commented
Apr 25, 2023
•
edited
Loading
edited
timeout = min(timeout, s->u.u.clock.timeout); | ||
&& (s->u.u.clock.flags & __WASI_SUBSCRIPTION_CLOCK_ABSTIME) == 0 | ||
&& s->u.u.clock.timeout < timeout) { | ||
timeout = s->u.u.clock.timeout; |
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.
Good catch; I wonder if other use of min (https://github.com/bytecodealliance/wasm-micro-runtime/pull/2152/files#diff-18a1c12794da572224835cdd9491ff7f93bc7ff81249c75ead3052e7f815f9f8R1021) would potentially be invalid too as it can go over size_t
type?
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.
Fair concern, fixed
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.
LGTM
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.
Looks good, thanks for fixing the issue
…platforms (bytecodealliance#2152) According to the 1999 ISO C standard (C99), size_t is an unsigned integer type of at least 16 bit (see sections 7.17 and 7.18.3), it may be uint32 in 32-bit platforms: https://en.cppreference.com/w/cpp/types/size_t Calling function `size_t min(size_t, size_t)` with two uint64 arguments may get invalid result. Co-authored-by: Georgii Rylov <godjan@amazon.co.uk>