Skip to content

Commit fac6968

Browse files
gmarullclaude
andcommitted
fw/services/analytics: actually pack the native heartbeat record
The packed attribute was placed before the struct keyword, where GCC silently ignores it, so native_heartbeat_record was serialized with natural alignment (e.g. timestamp at offset 8 instead of 1, plus padding between every misaligned member). The DLS blob therefore did not match the compact layout the source implies. Because the BLE disconnect counters sit at the end of the record, they absorbed all the upstream offset drift and decoded as garbage (the ble_disconnect_conn_spvn_tmo_count field in particular). Blobs whose tail fields happened to be zero still decoded plausibly, masking the issue. Move the attribute between the struct keyword and the tag using the PACKED macro so it is honored, yielding the intended compact layout. Add a static assertion that the record size equals the sum of its members (computed from analytics.def) to catch any future padding regression. The record version is left unchanged: decoders resolve the per-build layout from DWARF via the embedded build_id, so this is not an encoding scheme change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Gerard Marull-Paretas <gerard@teslabs.com>
1 parent dacd9d3 commit fac6968

1 file changed

Lines changed: 22 additions & 1 deletion

File tree

src/fw/services/analytics/native.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "pbl/services/data_logging/data_logging_service.h"
1212
#include "system/logging.h"
1313
#include "system/passert.h"
14+
#include "util/attributes.h"
1415
#include "util/build_id.h"
1516
#include "util/math.h"
1617
#include "util/size.h"
@@ -19,7 +20,7 @@
1920
#define NATIVE_HEARTBEAT_RECORD_VERSION 1
2021

2122
/* Heartbeat record logged to DLS */
22-
__attribute__((packed)) struct native_heartbeat_record {
23+
struct PACKED native_heartbeat_record {
2324
uint8_t version;
2425
uint64_t timestamp;
2526
uint8_t build_id[BUILD_ID_EXPECTED_LEN];
@@ -42,6 +43,26 @@ __attribute__((packed)) struct native_heartbeat_record {
4243
#undef PBL_ANALYTICS_METRIC_DEFINE_STRING
4344
};
4445

46+
/* The record is logged as a raw byte blob, so it must have no padding. */
47+
_Static_assert(
48+
sizeof(struct native_heartbeat_record) ==
49+
sizeof(uint8_t) + sizeof(uint64_t) + BUILD_ID_EXPECTED_LEN
50+
#define PBL_ANALYTICS_METRIC_DEFINE_UNSIGNED(key) +sizeof(uint32_t)
51+
#define PBL_ANALYTICS_METRIC_DEFINE_SIGNED(key) +sizeof(int32_t)
52+
#define PBL_ANALYTICS_METRIC_DEFINE_SCALED_UNSIGNED(key, scale) +sizeof(uint32_t) + sizeof(uint16_t)
53+
#define PBL_ANALYTICS_METRIC_DEFINE_SCALED_SIGNED(key, scale) +sizeof(int32_t) + sizeof(uint16_t)
54+
#define PBL_ANALYTICS_METRIC_DEFINE_TIMER(key) +sizeof(uint32_t)
55+
#define PBL_ANALYTICS_METRIC_DEFINE_STRING(key, len) +((len) + 1)
56+
#include "pbl/services/analytics/analytics.def"
57+
#undef PBL_ANALYTICS_METRIC_DEFINE_UNSIGNED
58+
#undef PBL_ANALYTICS_METRIC_DEFINE_SIGNED
59+
#undef PBL_ANALYTICS_METRIC_DEFINE_SCALED_UNSIGNED
60+
#undef PBL_ANALYTICS_METRIC_DEFINE_SCALED_SIGNED
61+
#undef PBL_ANALYTICS_METRIC_DEFINE_TIMER
62+
#undef PBL_ANALYTICS_METRIC_DEFINE_STRING
63+
,
64+
"native_heartbeat_record must be packed (no padding)");
65+
4566
/* Type-specific internal index enums (dense, no gaps) */
4667

4768
enum native_integer_index {

0 commit comments

Comments
 (0)