Test ignored-error storage across translation units

.gitea/workflows/ci.yaml

@@ -37,6 +37,23 @@ jobs:
           fail_on_failure: 'true'
       - run: echo "🍏 This job's status is ${{ job.status }}."
 
+  sanitizer:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v4
+      - name: dependencies
+        run: |
+          sudo apt-get update -y
+          sudo apt-get install -y cmake gcc moreutils
+      - name: build with AddressSanitizer and UBSan
+        run: |
+          cmake -S . -B build/asan -DAKERR_SANITIZE=address,undefined
+          cmake --build build/asan
+      - name: test with AddressSanitizer and UBSan
+        run: ctest --test-dir build/asan --output-on-failure
+      - run: echo "🍏 This job's status is ${{ job.status }}."
+
   coverage:
     runs-on: ubuntu-latest
     steps:

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.10)
 # 1.0.0 replaced the consumer-sized __AKERR_ERROR_NAMES array with private
 # storage. 2.0.0 makes the library thread safe, which is a second ABI break in
-# the same places: akerr_last_ignored became thread-local storage, and
+# the same places: __akerr_last_ignored became thread-local storage, and
 # ENSURE_ERROR_READY no longer takes the pool reference that akerr_next_error()
 # now takes for it. Consumer code compiled against a 1.x header would
 # double-count every reference. Hence the major bump and the SOVERSION, so a
@@ -282,6 +282,22 @@ foreach(_test IN LISTS AKERR_TESTS)
     endif()
 endforeach()
 
+# The ignored-error slot is declared in the public header but must have one
+# library definition. Compare its address from two translation units, and
+# compile the consumer-facing test with all ordinary warnings enabled: a
+# header-local TLS definition would both duplicate storage and warn when unused.
+add_executable(test_err_ignore_multitu
+               tests/err_ignore_multitu.c
+               tests/err_ignore_multitu_helper.c)
+target_include_directories(test_err_ignore_multitu
+                           PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/tests)
+target_link_libraries(test_err_ignore_multitu PRIVATE akerror)
+target_compile_options(test_err_ignore_multitu PRIVATE -Wall -Wextra -Werror)
+if(AKERR_THREAD_SAFE)
+    target_link_libraries(test_err_ignore_multitu PRIVATE Threads::Threads)
+endif()
+add_test(NAME err_ignore_multitu COMMAND test_err_ignore_multitu)
+
 # HANDLE_GROUP deliberately enters the next case label. Keep that public macro
 # compiling with the warning enabled, so a future macro edit cannot restore the
 # warning for consumers which adopt -Wextra.

README.md

@@ -170,7 +170,7 @@ the exit code was the status truncated to a byte, and every consumer status
 starts at 256. Use `akerr_exit()` instead of `exit()` — see
 [docs/exit-status.md](docs/exit-status.md). No ABI break.
 
-2.0.0 makes the library thread safe. That is an ABI break — `akerr_last_ignored`
+2.0.0 makes the library thread safe. That is an ABI break — `__akerr_last_ignored`
 became thread-local storage and the pool now takes its own reference — so
 everything built against a 1.x header must be rebuilt. 1.0.0 replaced the
 consumer-sized status-name array with a private, ownership-enforced registry.

UPGRADING.md

@@ -52,7 +52,7 @@ accident.
 
 What moved at the ABI:
 
-* `akerr_last_ignored` is thread-local storage. An ignored error is a fact
+* `__akerr_last_ignored` is thread-local storage. An ignored error is a fact
   about the thread that ignored it, and one shared slot had two threads
   overwriting each other's. The `IGNORE` macro expands at *your* call site, so
   your objects reference the symbol under whichever storage model your header

docs/thread-safety.md

@@ -14,12 +14,9 @@ What that covers:
   against each other and against lookups. Two threads reserving the same range
   cannot both win — exactly one gets `NULL` and the other gets
   `AKERR_STATUS_RANGE_OVERLAP` naming the winner.
-* **Per-thread state.** `IGNORE` copies the swallowed context into its
-  thread-local `akerr_last_ignored` snapshot before releasing the pool slot.
-  The snapshot remains valid until that thread ignores another error, so a
-  later pool checkout cannot overwrite it. The snapshot and the last-ditch
-  context used to report `akerr_release_error(NULL)` are thread-local, so
-  concurrent calls cannot overwrite each other's state.
+* **Per-thread state.** The context behind `IGNORE` (`__akerr_last_ignored`) and
+  the last-ditch context used to report `akerr_release_error(NULL)` are
+  thread-local, so one thread's ignored error is never another's.
 * **Handing a context from one thread to another.** A context is not thread
   state — it lives in `AKERR_ARRAY_ERROR`, which is process-global — so it
   outlives the thread that raised it. The reference count is the only field the

include/akerror.tmpl.h

@@ -15,7 +15,7 @@
  * scripts/generrno.sh stamps this value in at build time from the AKERR_THREADS
  * build option, the same way it stamps AKERR_LAST_ERRNO_VALUE. It is generated
  * rather than defined by the consumer on purpose: whether the library
- * serializes its global state and whether akerr_last_ignored is a
+ * serializes its global state and whether __akerr_last_ignored is a
  * thread-local are the same decision, and a consumer that disagreed with the
  * library about it would link against a differently shaped symbol.
  *
@@ -173,12 +173,11 @@ extern akerr_ErrorContext AKERR_ARRAY_ERROR[AKERR_MAX_ARRAY_ERROR];
 extern akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error;
 extern akerr_ErrorLogFunction akerr_log_method;
 /*
- * IGNORE()'s public per-thread snapshot. IGNORE() copies the swallowed error here
- * before releasing its pool context, so this remains a useful debugging aid
- * after the pool slot is reused. The snapshot is read-only and is replaced by
- * the next ignored error. Thread local only when AKERR_THREAD_SAFE is 1.
+ * The error IGNORE() last swallowed, per thread: an ignored error is a fact
+ * about the thread that ignored it, and one shared slot would have two threads
+ * overwriting each other's. Thread local only when AKERR_THREAD_SAFE is 1.
  */
-static AKERR_THREAD_LOCAL akerr_ErrorContext akerr_last_ignored;
+extern AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored;
 
 /*
  * Drop one reference, returning NULL once the last one is gone so the caller can
@@ -435,20 +434,10 @@ akerr_ErrorContext AKERR_NOIGNORE *__akerr_copy_string(char *destination, int ca
     FINISH_LOGIC(__err_context, true);
 
 #define IGNORE(__stmt)							\
-    do {								\
-	akerr_ErrorContext *__akerr_ignored = __stmt;				\
-	if ( __akerr_ignored != NULL ) {				\
-	    memcpy(&akerr_last_ignored, __akerr_ignored,	\
-		   sizeof(akerr_last_ignored));			\
-	    akerr_last_ignored.stacktracebufptr =		\
-		(char *)&akerr_last_ignored.stacktracebuf;	\
-	    akerr_ErrorContext *__akerr_ignored_snapshot =	\
-		&akerr_last_ignored;					\
-	    LOG_ERROR_WITH_MESSAGE(__akerr_ignored_snapshot,	\
-				   "** IGNORED ERROR **");		\
-	    RELEASE_ERROR(__akerr_ignored);				\
-	}								\
-    } while ( 0 )
+    __akerr_last_ignored = __stmt;					\
+    if ( __akerr_last_ignored != NULL ) {				\
+	LOG_ERROR_WITH_MESSAGE(__akerr_last_ignored, "** IGNORED ERROR **"); \
+    }
 
 #define CLEANUP					\
     }; 

scripts/mutation_test.py

@@ -25,6 +25,8 @@ Usage:
     --work DIR          scratch dir for the mutated copy (default: a temp dir)
     --timeout SECONDS   per-suite ctest timeout (default: 120)
     --threshold PCT     exit non-zero if mutation score < PCT (default: 0 = off)
+    --cmake-arg ARG     pass an additional argument to the mutant CMake configure;
+                        repeat for multiple arguments (e.g. -DAKERR_SANITIZE=thread)
     --list              only list the mutants that would be run, then exit
     --keep              keep the scratch working copy on exit (for debugging)
     -j N                (reserved) currently runs sequentially
@@ -199,10 +201,11 @@ def generate_mutants(root, rel_target):
 # --------------------------------------------------------------------------- #
 
 class Runner:
-    def __init__(self, work, timeout):
+    def __init__(self, work, timeout, cmake_args):
         self.work = work
         self.build = os.path.join(work, "build")
         self.timeout = timeout
+        self.cmake_args = cmake_args
 
     def _run(self, cmd, timeout=None):
         return subprocess.run(
@@ -211,7 +214,8 @@ class Runner:
         )
 
     def configure(self):
-        r = self._run(["cmake", "-S", ".", "-B", "build"], timeout=self.timeout)
+        r = self._run(["cmake", "-S", ".", "-B", "build", *self.cmake_args],
+                      timeout=self.timeout)
         return r.returncode == 0, r.stdout
 
     def build_and_test(self):
@@ -307,6 +311,8 @@ def main():
     ap.add_argument("--work", default=None)
     ap.add_argument("--timeout", type=int, default=120)
     ap.add_argument("--threshold", type=float, default=0.0)
+    ap.add_argument("--cmake-arg", action="append", default=[],
+                    help="pass an argument to the mutant CMake configure; repeatable")
     ap.add_argument("--junit", default=None,
                     help="write a JUnit XML report to this path")
     ap.add_argument("--max-mutants", type=int, default=0,
@@ -354,7 +360,7 @@ def main():
     print(f"\nCopying sources to scratch dir: {work}")
     copy_tree(root, work)
 
-    runner = Runner(work, args.timeout)
+    runner = Runner(work, args.timeout, args.cmake_arg)
 
     print("Configuring baseline ...")
     ok, out = runner.configure()

src/error.c

@@ -20,10 +20,10 @@
  * It is not small (an akerr_ErrorContext is tens of kilobytes), but the storage
  * is allocated per thread only when that thread first touches the library's
  * thread-local block, and the alternative is a shared buffer that two threads
- * can be writing at once. The per-thread IGNORE() snapshot lives in the public
- * template header because the macro copies into it at the call site.
+ * can be writing at once.
  */
 static AKERR_THREAD_LOCAL akerr_ErrorContext __akerr_last_ditch;
+AKERR_THREAD_LOCAL akerr_ErrorContext *__akerr_last_ignored;
 akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error;
 akerr_ErrorLogFunction akerr_log_method = NULL;
 
@@ -233,6 +233,7 @@ static void akerr_init_state(void)
 	AKERR_ARRAY_ERROR[i].arrayid = i;
 	AKERR_ARRAY_ERROR[i].stacktracebufptr = (char *)&AKERR_ARRAY_ERROR[i].stacktracebuf;
     }
+    __akerr_last_ignored = NULL;
     (void)akerr_last_ditch_context();
     if ( akerr_log_method == NULL ) {
 	akerr_log_method = &akerr_default_logger;
@@ -663,7 +664,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *akerr_reserve_status_range_locked(int
 			"must not overflow int)",
 			count, first_status, owner == NULL ? "(null)" : owner,
 			AKERR_MAX_STATUS_RANGE_OWNER_LENGTH);
-    last_status = first_status + count - 1;
+    last_status = first_status + (count - 1);
 
     for ( int i = 0; i < akerr_status_range_count; i++ ) {
 	if ( first_status <= akerr_status_ranges[i].last &&

tests/MUTATION.md

@@ -32,6 +32,9 @@ scripts/mutation_test.py --target src/error.c --list
 
 # Gate CI: exit non-zero if the score drops below 90%
 scripts/mutation_test.py --threshold 90
+
+# Check concurrency mutants under ThreadSanitizer
+scripts/mutation_test.py --cmake-arg=-DAKERR_SANITIZE=thread
 ```
 
 Via CMake (configures a build first if needed):
@@ -40,8 +43,10 @@ Via CMake (configures a build first if needed):
 cmake --build build --target mutation
 ```
 
-Useful flags: `--timeout SECONDS` (per-suite build+test cap; a mutant that
-hangs is counted as killed), `--keep` (retain the scratch copy for debugging),
+Useful flags: `--cmake-arg ARG` (pass a CMake configure argument to every
+mutant build; repeat it for multiple arguments), `--timeout SECONDS` (per-suite
+build+test cap; a mutant that hangs is counted as killed), `--keep` (retain the
+scratch copy for debugging),
 `--work DIR` (use a specific scratch directory), `--junit FILE` (write a JUnit
 XML report — surviving mutants appear as failing test cases).
 
@@ -107,7 +112,7 @@ The remaining survivors are dominated by:
 
 * **Equivalent mutants** in `akerr_init`: deleting the `memset`/`NULL` setup of
   file-scope statics (`AKERR_ARRAY_ERROR`, `__akerr_last_ditch`,
-  `akerr_last_ignored`) changes nothing, because C already zero-initializes
+  `__akerr_last_ignored`) changes nothing, because C already zero-initializes
   objects with static storage duration. `int oldid = 0;` → `1` is likewise
   dead: it is overwritten before use, and so is clearing `akerr_initializing`
   at the end of initialization — nothing reads that flag once the once-routine

tests/err_ignore.c

@@ -1,8 +1,11 @@
 #include "akerror.h"
 #include "err_capture.h"
-#include <string.h>
 
-/* IGNORE snapshots and logs an error, releases its pool slot, then continues. */
+/*
+ * IGNORE deliberately swallows an error: it records the context in
+ * __akerr_last_ignored, logs it with an "IGNORED ERROR" marker, and lets
+ * execution continue.
+ */
 
 akerr_ErrorContext *boom(void)
 {
@@ -18,19 +21,11 @@ int main(void)
     PREPARE_ERROR(e);
     (void)e;
 
-    /* More failures than the pool has slots must remain safe: a leaking
-     * IGNORE used to exhaust the pool and terminate the process here. The
-     * copied snapshot must also survive the slot being reused on the next
-     * iteration. */
-    for ( int i = 0; i < AKERR_MAX_ARRAY_ERROR + 1; i++ ) {
-	IGNORE(boom());
-	AKERR_CHECK(akerr_last_ignored.status == AKERR_VALUE);
-	AKERR_CHECK(strcmp(akerr_last_ignored.message,
-			  "this error is ignored on purpose") == 0);
-	AKERR_CHECK(akerr_slots_in_use() == 0);
-    }
+    IGNORE(boom());
     reached_after_ignore = 1;
 
+    AKERR_CHECK(__akerr_last_ignored != NULL);
+    AKERR_CHECK(__akerr_last_ignored->status == AKERR_VALUE);
     AKERR_CHECK(reached_after_ignore == 1);
     AKERR_CHECK_CONTAINS("IGNORED ERROR");
     AKERR_CHECK_CONTAINS("this error is ignored on purpose");

tests/err_ignore_multitu.c

@@ -0,0 +1,18 @@
+#include "akerror.h"
+#include "err_ignore_multitu.h"
+#include <stdio.h>
+
+int main(void)
+{
+    akerr_ErrorContext **main_address = &__akerr_last_ignored;
+    akerr_ErrorContext **helper_address = akerr_multitu_snapshot_address();
+
+    if ( main_address != helper_address ) {
+	fprintf(stderr,
+		"CHECK FAILED: ignored-error storage differs between translation units\n");
+	return 1;
+    }
+
+    fprintf(stderr, "err_ignore_multitu ok\n");
+    return 0;
+}

tests/err_ignore_multitu.h

@@ -0,0 +1,8 @@
+#ifndef AKERR_TEST_IGNORE_MULTITU_H
+#define AKERR_TEST_IGNORE_MULTITU_H
+
+#include "akerror.h"
+
+akerr_ErrorContext **akerr_multitu_snapshot_address(void);
+
+#endif

tests/err_ignore_multitu_helper.c

@@ -0,0 +1,6 @@
+#include "err_ignore_multitu.h"
+
+akerr_ErrorContext **akerr_multitu_snapshot_address(void)
+{
+    return &__akerr_last_ignored;
+}

tests/err_threads_pool.c

@@ -100,21 +100,17 @@ static void *pool_body(void *raw)
 	one_checkout(arg);
     }
 
-    /* IGNORE's snapshot is thread-local while logging and remains valid after
-     * release. Concurrent ignored errors must all return their pool slots. */
+    /* An ignored error is a fact about the thread that ignored it: each thread
+     * must see its own, not the last one any thread swallowed. */
     IGNORE(ignorable(arg));
-    AKERR_TCHECK(arg, akerr_last_ignored.status == AKERR_IO);
-    AKERR_TCHECK(arg, strcmp(akerr_last_ignored.message, expected) == 0);
-
-    /* Reuse a slot after IGNORE and prove that the copied snapshot did not
-     * become an alias for the newly acquired context. */
-    akerr_ErrorContext *reused = akerr_next_error();
-    AKERR_TCHECK(arg, reused != NULL);
-    if ( reused != NULL ) {
-	RELEASE_ERROR(reused);
+    AKERR_TCHECK(arg, __akerr_last_ignored != NULL);
+    if ( __akerr_last_ignored != NULL ) {
+	AKERR_TCHECK(arg, __akerr_last_ignored->status == AKERR_IO);
+	AKERR_TCHECK(arg, strcmp(__akerr_last_ignored->message, expected) == 0);
     }
-    AKERR_TCHECK(arg, akerr_last_ignored.status == AKERR_IO);
-    AKERR_TCHECK(arg, strcmp(akerr_last_ignored.message, expected) == 0);
+    /* IGNORE keeps the reference by design; hand it back so the pool is empty
+     * at the end of the test. */
+    RELEASE_ERROR(__akerr_last_ignored);
 
     return NULL;
 }