Skip to content

Commit 5093c57

Browse files
committed
fix: link safe-stack runtime for coverage
1 parent 4224e90 commit 5093c57

File tree

3 files changed

+12
-3
lines changed

3 files changed

+12
-3
lines changed

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,4 @@ See [docs/guides/DEBRIEF_FORMAT.md](docs/guides/DEBRIEF_FORMAT.md) for the JSONL
100100
{"date": "2025-10-20", "time": "04:42", "summary": "Tweaked PR guard env vars and hardened unsigned builder checks per latest review notes.", "topics": [{"topic": "PR guard env naming", "what": "Restored HEAD_SHA/BASE_REF env keys for lint-commits.sh compatibility.", "why": "Reviewer noted the script still references the original variables.", "context": "CI PR gate job was failing due to undefined variables.", "issue": "Renamed vars caused lint-commits.sh to read empty values.", "resolution": "Reintroduced HEAD_SHA and BASE_REF in the workflow step.", "future_work": "None.", "time_percent": 40}, {"topic": "Unsigned builder guard", "what": "Clamped numeric base to [2,16] and replaced the static_assert with a typedef-based compile-time check.", "why": "Feedback requested safe indexing into the digits alphabet and lint still flagged the assert expression.", "context": "metagraph_builder_append_unsigned handles arbitrary bases.", "issue": "Values >16 overflowed the lookup and clang-tidy kept complaining about implicit conversions.", "resolution": "Normalized base values and leveraged a typedef-sized array to enforce compile-time capacity.", "future_work": "Consider exposing constants for max supported base if more callers appear.", "time_percent": 60}], "key_decisions": ["Keep run-clang-tidy.sh interface unchanged by feeding its expected env vars instead of patching the script."], "action_items": []}
101101

102102
{"date": "2025-10-20", "time": "04:50", "summary": "Fixed sanitizer configuration fallout (safe-stack conflict and missing Valgrind target).", "topics": [{"topic": "safe-stack vs ASAN", "what": "Stopped appending -fsanitize=safe-stack when global sanitizers are enabled.", "why": "Clang refused to build sanitizer jobs with safe-stack alongside AddressSanitizer.", "context": "Quality Matrix release job and sanitizer workflow failures.", "issue": "Compiler error: 'invalid argument -fsanitize=safe-stack not allowed with -fsanitize=address'.", "resolution": "Only add safe-stack when sanitizers are off, preserving hardening for non-ASAN builds.", "future_work": "Consider reintroducing safe-stack for ARM shadow-call-stack variants later.", "time_percent": 70}, {"topic": "Valgrind target", "what": "Wrapped the Valgrind custom target in a TARGET check and pointed it at mg_tests.", "why": "Configuration failed because METAGRAPH_tests target never existed.", "context": "Sanitizers.cmake was referencing a stale target name.", "issue": "CMake generator expression resolved to a missing target, halting configure step.", "resolution": "Guarded the target and corrected the target file reference.", "future_work": "None.", "time_percent": 30}], "key_decisions": ["Prefer guarding optional hardening flags to keep sanitizer builds working."], "action_items": []}
103+
{"date":"2025-10-20","time":"12:34","summary":"Linked safe-stack runtime for coverage builds and modernized the unsigned printer guard in metagraph error builder.","topics":[{"topic":"Coverage build fix","what":"Added safe-stack to link flags when sanitizers are off","why":"Code coverage job was failing to link due to missing __safestack symbol","context":"GitHub Actions coverage workflow uses Clang 18 with safe-stack enabled by default security flags","issue":"Linker missing __safestack_unsafe_stack_ptr runtime","resolution":"Propagated -fsanitize=safe-stack to link options and validated coverage build locally","future_work":"Monitor next CI cycle to confirm the coverage job is green","time_percent":70},{"topic":"Static assert cleanup","what":"Replaced array typedef trick with _Static_assert","why":"Reviewer requested modern assertion idiom","context":"metagraph_builder_append_unsigned relies on 64-byte digit buffer","issue":"Legacy static assert style cluttered the code","resolution":"Used C23 _Static_assert to enforce buffer size","future_work":"None","time_percent":30}],"key_decisions":["Keep safe-stack off only when sanitizers are enabled; otherwise link runtime explicitly"],"action_items":[]}

cmake/CompilerFlags.cmake

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ set(METAGRAPH_SECURITY_FLAGS
4444
-fPIE
4545
)
4646

47+
set(METAGRAPH_SECURITY_LINK_FLAGS)
48+
4749
# Platform-specific security flags
4850
if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
4951
list(APPEND METAGRAPH_SECURITY_FLAGS
@@ -102,6 +104,9 @@ elseif(CMAKE_C_COMPILER_ID MATCHES "Clang")
102104
list(APPEND METAGRAPH_SECURITY_FLAGS
103105
-fsanitize=safe-stack
104106
)
107+
list(APPEND METAGRAPH_SECURITY_LINK_FLAGS
108+
-fsanitize=safe-stack
109+
)
105110
endif()
106111
endif()
107112
elseif(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
@@ -125,6 +130,10 @@ endif()
125130
add_compile_options(${METAGRAPH_WARNING_FLAGS})
126131
add_compile_options(${METAGRAPH_SECURITY_FLAGS})
127132

133+
if(METAGRAPH_SECURITY_LINK_FLAGS)
134+
add_link_options(${METAGRAPH_SECURITY_LINK_FLAGS})
135+
endif()
136+
128137
# Enable PIE for all builds (not just release)
129138
if(NOT CMAKE_C_COMPILER_ID STREQUAL "MSVC")
130139
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

src/error.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,8 @@ metagraph_builder_append_unsigned(metagraph_message_builder_t *builder,
209209
base = 16U;
210210
}
211211
char digits[64];
212-
typedef char
213-
metagraph_digits_static_assert_t[(sizeof digits >= 64U) ? 1 : -1];
214-
(void)sizeof(metagraph_digits_static_assert_t);
212+
_Static_assert(sizeof(digits) >= 64U,
213+
"digits buffer must be at least 64 bytes");
215214
const char *alphabet = "0123456789abcdef";
216215
if (uppercase) {
217216
alphabet = "0123456789ABCDEF";

0 commit comments

Comments
 (0)