From 744e901a0b16c20dd819efff508b20fa67a6b562 Mon Sep 17 00:00:00 2001
From: Sentinel Bot <noreply+26860585020226206@fb.com>
Date: Wed, 16 Sep 2026 02:31:33 -0700
Subject: [PATCH] Unbreak fbclock's Go build under clang 21 (#581)

Summary:

`buck2 test //time/fbclock/...` does not build on master: fbclock's Go tests, daemon and sidecar are all unbuildable.

    cgo: fbclock.go:72:13: unexpected type: (unsupported type AtomicType)

Clang 21 emits `DW_TAG_atomic_type` where clang 19 folded the atomic into its base integer. cgo reads DWARF, and Go's `debug/dwarf` cannot model that tag, so every struct reachable from `fbclock_lib` fails to convert.

Go never touches `seq` or `crc`, so only the cgo preamble now sees the plain base type. `fbclock.c` keeps the real atomics and static_asserts that the layout matches.

Sentinel-Council-Run: council_1789520658_fbd80dac
Sentinel-Council-Run: council_1789531426_e0c09b3e
Sentinel-Council-Run: council_1789538610_83d8818c
Sentinel-Harness: claude

*Modify your team agent prompt, check stats, and leave feedback: https://www.internalfb.com/sentinel_agent/rotations/clock*
Model used: Claude Opus 5 (claude-opus-5[1m])

Differential Revision: D120272839
---
 fbclock/fbclock.c  | 15 +++++++++++++++
 fbclock/fbclock.go |  4 ++++
 fbclock/fbclock.h  | 15 +++++++++++++--
 fbclock/shmem.go   |  4 ++++
 4 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/fbclock/fbclock.c b/fbclock/fbclock.c
index 47e8c392..58cc8082 100644
--- a/fbclock/fbclock.c
+++ b/fbclock/fbclock.c
@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 #include "fbclock.h"
+#include <assert.h> // for static_assert in C11
 #include <fcntl.h> // For O_* constants
 #include <linux/ptp_clock.h>
 #include <stdint.h>
@@ -29,6 +30,20 @@ limitations under the License.
 #include <arm_acle.h>
 #endif
 
+// This file implements the seqlock, so it must see the real atomics. Defining
+// FBCLOCK_CGO here would also leave the assert below comparing a type with
+// itself.
+#ifdef FBCLOCK_CGO
+#error "FBCLOCK_CGO is only for cgo's view of the header, never for fbclock.c"
+#endif
+
+// fbclock.h hands cgo a plain uint_fast64_t in place of atomic_uint64 so it can
+// convert the shm structs. That is only sound while the two lay out the same.
+static_assert(
+    sizeof(atomic_uint64) == sizeof(uint_fast64_t) &&
+        alignof(atomic_uint64) == alignof(uint_fast64_t),
+    "atomic_uint64 must match its base type's layout; see FBCLOCK_CGO in fbclock.h");
+
 #if defined(__GNUC__) && !defined(__OPTIMIZE__)
 #define fbclock_debug_print(fmt, ...)  \
   do {                                 \
diff --git a/fbclock/fbclock.go b/fbclock/fbclock.go
index 69e663f0..25cd6def 100644
--- a/fbclock/fbclock.go
+++ b/fbclock/fbclock.go
@@ -22,6 +22,10 @@ package fbclock
 #cgo LDFLAGS: -lrt
 #cgo amd64 CFLAGS: -msse4.2
 
+// Scoped to this preamble on purpose: a #cgo CFLAGS define would also reach
+// fbclock.c, which needs the real atomics. See FBCLOCK_CGO in fbclock.h.
+#define FBCLOCK_CGO 1
+
 #include "fbclock.h" // @oss-only
 // @fb-only: #include "time/fbclock/fbclock.h"
 
diff --git a/fbclock/fbclock.h b/fbclock/fbclock.h
index 1df2291c..473624db 100644
--- a/fbclock/fbclock.h
+++ b/fbclock/fbclock.h
@@ -16,7 +16,19 @@ limitations under the License.
 
 #pragma once
 
-#if defined(__cplusplus) && !defined(__clang__)
+#include <stdint.h> /* for proper fixed width types */
+
+// Only the cgo preambles in fbclock.go and shmem.go define FBCLOCK_CGO. cgo
+// builds its Go types from DWARF, and Go's debug/dwarf has no model for
+// DW_TAG_atomic_type, which clang 21 emits where clang 19 folded the atomic
+// into its base integer; every struct reachable from fbclock_lib then fails to
+// convert. Go needs the size and field offsets of the shm structs; it must
+// never touch seq or crc itself, so a plain base type serves it. fbclock.c
+// static_asserts that the substitution is layout-identical. The Rust bindings
+// solve the same problem with --opaque-type; see this directory's BUCK.
+#if defined(FBCLOCK_CGO)
+typedef uint_fast64_t atomic_uint64;
+#elif defined(__cplusplus) && !defined(__clang__)
 #include <atomic>
 typedef std::atomic_uint_fast64_t atomic_uint64;
 #else
@@ -24,7 +36,6 @@ typedef std::atomic_uint_fast64_t atomic_uint64;
 typedef atomic_uint_fast64_t atomic_uint64;
 #endif
 
-#include <stdint.h> /* for proper fixed width types */
 #ifndef __cplusplus
 #include <stdalign.h> /* for alignas in C; alignas is a keyword in C++ */
 #endif
diff --git a/fbclock/shmem.go b/fbclock/shmem.go
index d041f7a7..5d5c78fd 100644
--- a/fbclock/shmem.go
+++ b/fbclock/shmem.go
@@ -21,6 +21,10 @@ package fbclock
 /*
 #cgo LDFLAGS: -lrt
 
+// cgo compiles each preamble as its own translation unit, so every file that
+// includes the header needs this, not just fbclock.go. See FBCLOCK_CGO there.
+#define FBCLOCK_CGO 1
+
 #include "fbclock.h" // @oss-only
 // @fb-only: #include "time/fbclock/fbclock.h"