[C++] Fix Undefined behavior for zero length vectors#5355
Conversation
|
|
||
| void push(const uint8_t *bytes, size_t num) { | ||
| memcpy(make_space(num), bytes, num); | ||
| // CreateVector above, can pass through a nullptr for bytes |
There was a problem hiding this comment.
If so, lets make the fix in CreateVector, not here. This is performance sensitive code, so I would like as few if-thens (or as far from the core) as possible.
There was a problem hiding this comment.
OK, are there benchmarks as part of the project I can run to see if there will be impact from my changes?
There was a problem hiding this comment.
No there aren't. There are some super old benchmarks in a branch that are win32 only.
Regardless, if you can pull an if-then further away from a hot function, why not do it? especially when all other callers of this function already comply with memcpy's rules?
There was a problem hiding this comment.
I think its fine, just wondering if I can assess the impact further upstream. I will update the PR in the next day or two. Thank you for the feedback.
|
@aardappel appveyor failures look unrelated to this change, but I'm not too familiar with the build, please let me know if you think this broke something? |
|
Yes, CI looks unrelated. |
2 participants
UBSan fails when checking for nullability with clang in this header file without this change.