define OZZ_BYTE_SWAP(a, b) {a = a ^ b; b = a ^ b; a = a ^ b;}

guillaumeblanc commented 2 years ago

Hi,

thanks for reporting the issue. Here's the original implementation for reference.

I'd have a few questions:

Is this a warning, or an error?
Could you give more details about the callstack? This is a macro, so the issue could be on the caller side.
What version of gcc are you using? Could you repro it on the linux CI?

The important thing for me is to understand if there is really an error, in which case it needs to be fixed. Your implementation is also accessing a and b, so I don't see how it fixes the error. What do you think?

Cheers, Guillaume

Form2 commented 2 years ago

Here's the first error with all related lines:

inlined from ‘void ozz::io::internal::Array<_Ty>::Load(ozz::io::IArchive&, uint32_t) const [with _Ty = float]’ at .../ozz-utils/ozz-animation/include/ozz/base/io/archive.h:369:1,
inlined from ‘static void ozz::io::Extern<_Ty>::Load(ozz::io::IArchive&, _Ty*, size_t, uint32_t) [with _Ty = const ozz::io::internal::Array<float>]’ at .../ozz-utils/ozz-animation/include/ozz/base/io/archive.h:280:18,
inlined from ‘void ozz::io::IArchive::operator>>(_Ty&) [with _Ty = const ozz::io::internal::Array<float>]’ at .../ozz-utils/ozz-animation/include/ozz/base/io/archive.h:198:22,
inlined from ‘static void ozz::io::Extern<ozz::math::Float4>::Load(ozz::io::IArchive&, ozz::math::Float4*, size_t, uint32_t)’ at .../ozz-utils/ozz-animation/src/base/maths/math_archive.cc:68:12:
.../ozz-utils/ozz-animation/include/ozz/base/endianness.h:73:10: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] 73 | (_b) = temp; \ | ~^~ .../ozz-utils/ozz-animation/include/ozz/base/endianness.h:106:7: note: in expansion of macro ‘OZZ_BYTE_SWAP’ 106 | OZZ_BYTE_SWAP(alias[i + 1], alias[i + 2]); | ^~~~~ In file included from .../ozz-utils/ozz-animation/include/ozz/base/maths/box.h:33, from .../ozz-utils/ozz-animation/src/base/maths/math_archive.cc:33: .../ozz-utils/ozz-animation/include/ozz/base/maths/vec_float.h: In static member function ‘static void ozz::io::Extern::Load(ozz::io::IArchive&, ozz::math::Float4*, size_t, uint32_t)’: .../ozz-utils/ozz-animation/include/ozz/base/maths/vec_float.h:100:9: note: at offset 26 into destination object ‘ozz::math::Float4::x’ of size 4

gcc --version shows:

gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0

Your question:

"Could you repro it on the linux CI?"

I'm not sure... I don't use github in a collaborative manner very often... So I'm afraid that you've gone over my head...

Your concern:

"The important thing for me is to understand if there is really an error"

I think that you are probably right to be suspicious... This is probably the compiler being annoying... I hope that opening this as an issue was helpful, but I'm afraid that it was not :)

guillaumeblanc commented 2 years ago

Hi,

yes for sure opening the issue is helpful, others might experience the same issue indeed.

I could repro the compiling issue with the CI, using a gcc 11 release build.

The serialization code considers x,y,... components as contiguous floats in memory, aiming to serialize them at once. My understanding is that the compiler considers them as different float members, hence generates an error when writing further than x.

I think the code is ok, but I understand the error. I need to dig further to see what can be done. Don't hesitate if you have an idea or a different analysis.

Thanks, Guillaume

Form2 commented 2 years ago

For what it's worth, I've been using the fix I first mentioned without problems. Glad I could help, and thanks for the reply.

KevinLium commented 1 year ago

mark ，thanks

guillaumeblanc commented 1 year ago

Suggested fix committed.

guillaumeblanc / ozz-animation

error "writing 1 byte into a region of size 0" on gcc release build #147

define OZZ_BYTE_SWAP(a, b) {a = a ^ b; b = a ^ b; a = a ^ b;}