search

guillaumeblanc / ozz-animation

Open source c++ skeletal animation library and toolset
http://guillaumeblanc.github.io/ozz-animation/
Other
2.46k stars 302 forks source link

undefined behavior: memcpy in stream.cc #98

Closed rainerdeyke closed 4 years ago

rainerdeyke commented 4 years ago

As of version 0.13.0, In src/base/io/stream.cc, line 209, there is a call to std::memcpy with what could be a null buffer (and zero bytes to copy). This is undefined behavior, and causes my program to terminate if I use undefined behaviors sanitizer. It is not legal to call std::memcpy with nullptr arguments, even if the number of bytes to copy is set to zero.

guillaumeblanc commented 4 years ago

Hi,

good to know it's undefined! There might be other places in the code base!

I assume the case is during the first allocation, so buffer_ and allocsize are null/0. Is that correct? A solution could then be to allocate buffer with a default size in the constructor, or add a if before memcpy. I can't help but think it's a bit of a shame that it's undefined (with size of 0).

Would be awesome if you could send a PR that passes your undefined behaviors sanitizer tests. Can you ?

By the way, how do you do this undefined behavior sanitizer test? Is it with clang -fsanitize=undefined? Sounds interesting to have this from the CI.

Cheers, Guillaume

guillaumeblanc commented 4 years ago

Merged to develop branch. Thanks a lot!