Skip to content

Commit a1b6a27

Browse files
committed
Workaround for misalignment in heterogeneous builds
- Added a workaround for misaligned SubmapSlice structure between cartographer and cartographer_ros built using different compilers or compile contexts - Rearranged SubmapSlice field order after static analysis performed with clangd - Added commented non-portable workaround with \#pragma pack - Closes cartographer-project#1909 Signed-off-by: Andrey Vukolov <[email protected]>
1 parent ef00de2 commit a1b6a27

File tree

1 file changed

+45
-10
lines changed

1 file changed

+45
-10
lines changed

Diff for: cartographer/io/submap_painter.h

+45-10
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
#include "cartographer/mapping/value_conversion_tables.h"
2727
#include "cartographer/transform/rigid_transform.h"
2828

29+
// Workaround to prevent SubmapSlice structure alignment issue with cartographer_ros
30+
// Github issue #1909
31+
// #pragma pack(16)
32+
2933
namespace cartographer {
3034
namespace io {
3135

@@ -39,23 +43,54 @@ struct PaintSubmapSlicesResult {
3943
Eigen::Array2f origin;
4044
};
4145

42-
struct SubmapSlice {
46+
/* Structure rearranged according to the static analyzer recommendation
47+
* SA Message:
48+
* Excessive padding in 'struct cartographer::io::SubmapSlice' (32 padding bytes,
49+
* where 0 is optimal). Optimal fields order:
50+
* - slice_pose,
51+
* - pose,
52+
* - resolution,
53+
* - surface,
54+
* - cairo_data,
55+
* - width,
56+
* - height,
57+
* - version,
58+
* - metadata_version,
59+
* consider reordering the fields or adding explicit padding members
60+
* [clang-analyzer-optin.performance.Padding]
61+
*
62+
* Old version:
63+
* // Texture data.
64+
* int width;
65+
* int height;
66+
* int version;
67+
* double resolution;
68+
* ::cartographer::transform::Rigid3d slice_pose;
69+
* ::cartographer::io::UniqueCairoSurfacePtr surface;
70+
* // Pixel data used by 'surface'. Must outlive 'surface'.
71+
* std::vector<uint32_t> cairo_data;
72+
* // Metadata.
73+
* ::cartographer::transform::Rigid3d pose;
74+
* int metadata_version = -1;
75+
*/
76+
77+
// Another C++11-compliant workaround to prevent SubmapSlice structure
78+
// alignment issue with cartographer_ros
79+
// Github issue #1909
80+
struct alignas(32) SubmapSlice {
81+
// Pixel data used by 'surface'. Must outlive 'surface'.
82+
std::vector<uint32_t> cairo_data;
83+
::cartographer::io::UniqueCairoSurfacePtr surface;
4384
SubmapSlice()
4485
: surface(::cartographer::io::MakeUniqueCairoSurfacePtr(nullptr)) {}
45-
46-
// Texture data.
86+
double resolution;
87+
// Metadata.
4788
int width;
4889
int height;
4990
int version;
50-
double resolution;
91+
int metadata_version = -1;
5192
::cartographer::transform::Rigid3d slice_pose;
52-
::cartographer::io::UniqueCairoSurfacePtr surface;
53-
// Pixel data used by 'surface'. Must outlive 'surface'.
54-
std::vector<uint32_t> cairo_data;
55-
56-
// Metadata.
5793
::cartographer::transform::Rigid3d pose;
58-
int metadata_version = -1;
5994
};
6095

6196
struct SubmapTexture {

0 commit comments

Comments
 (0)