ros / roscpp_core

ros distribution sandbox
89 stars 116 forks source link

Fixed wrong serialization of messages on ARM with GCC 9.3 and -O3 optimizations #136

Closed peci1 closed 1 year ago

peci1 commented 1 year ago

Closes #130.

@randoms Do you have a way to verify this fix helps?

I was able to reproduce the bug on arm64 Jetson with GCC 9.3.0. When I tested with 9.4.0 (which is now the default in Focal), the bug did not manifest. So I limited the fix to the only known combination triggering this issue - arm64 + GCC 9.3.

peci1 commented 1 year ago

An easy way to verify for everybody - you can just apply the patch directly to your /opt/ros/noetic/include/ros/serialization.h file. Just back it up somewhere before you apply the patch. After that, force-rebuild the packages in your workspace and try if a custom-defined message that previously triggered the issue triggers it no more.

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/who-are-current-roscpp-core-maintaners/31247/5

randoms commented 1 year ago

@peci1 Thank you for your work. I have tested this bug on Raspberry Pi 4B with gcc 9.4, the bug did not manifest. It's hard to downgrade gcc on my device. so I didn't test your patch with gcc 9.3. Since gcc 9.4 becomes the system default gcc, I think the problem can be safely ignored.

randoms commented 1 year ago

I tested the patch with gcc 9.3, and I can confirm this patch works. I built gcc 9.3 from source, it toke me really a very long time.

peci1 commented 1 year ago

Thanks very much for the effort, @randoms ! I'll merge the fix.

peci1 commented 1 year ago

@Mergifyio backport melodic-devel

mergify[bot] commented 1 year ago

backport melodic-devel

✅ Backports have been created

* [#142 Fixed wrong serialization of messages on ARM with GCC 9.3 and -O3 optimizations (backport #136)](https://github.com/ros/roscpp_core/pull/142) has been created for branch `melodic-devel`