-
Notifications
You must be signed in to change notification settings - Fork 112
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
aligned_alloc requirements update. #681
Conversation
@bsdjhb and @jrtc27 is this change acceptable to you.
So this seems reasonable to check.
I do wonder if there should be a mitigations feature enforce libc spec, so that uses know when they are violating the spec, but for performance sensitive use cases, we do not need to take the instructions for checking these conditions. E.g. add something to: snmalloc/src/snmalloc/ds_core/mitigations.h Lines 68 to 211 in fb776da
@nwf do you have thoughts on this. |
No objection to enforcing that. The OpenZFS case that motivated removing the overly-strict requirement does |
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
So I dug a bit deeper into this. We did use to have this assertion and removed it here: It seems that the spec was loosened by This seems to be in the the N2310 draft for C17. So the spec does not require an allocator to fail on this case. I would sooner leave the check out as it will be faster. @devnexen did you have a strong reason for wanting this? |
Looking at a C23 draft there (N3220) there is no restriction given for size, even (inheriting the same behaviour for 0 as malloc as that's hoisted out into the section introduction). So yeah whilst this restriction is fine for C11 it is wrong for newer standards. |
I would not mind then to enable this additional check only for C11 ? |
You have no idea what C standard the caller is compiled for. |
true true ... ok gonna drop this check then. |
d158ec5
to
7110655
Compare
in addition, specs requires size to be a multiple of alignment.