M2: Minimal Linux CRT shim (_start) and freestanding CI job #48
Conversation
…NOLINT; local tidy override What and why - Prevent double-free/UAF by making contexts registries, not owners. - Snapshot allocator in gitledger_error_t so release is safe post-context teardown. - Enforce lifecycle: in debug, abort if context has live errors; in release, refuse to destroy and log. - Remove all NOLINT and refactor to satisfy clang-tidy: split teardown helpers, safe snprintf+fwrite logging, named constants. - Restore global .clang-tidy and localize the bugprone-easily-swappable-parameters disable under libgitledger/core/ to account for C callback signatures. - Reaffirm host execution guard in AGENTS.md. Validation - Format + clang-tidy: clean locally. - Unit tests + ASAN/UBSAN + TSAN: green locally (macOS leak detection disabled as unsupported). Follow-ups - Add a unit test to assert teardown refusal semantics. - Optional single-shot structured diagnostic for lifecycle violations. No rebase, no amend, no force. Logged activity before committing.
…d API updates, test for teardown refusal - Build config: switch to C99, set strict flags (-Wall -Wextra -Werror -pedantic -std=c99 -Wno-fun), add -nostdlib to lib and executables. - Docs: memoizes -> memoizes; clarify Release refusal + stderr diagnostic; contexts register but don’t own errors. - Headers: remove unused gitledger_status_t; update lifetime comment to debug abort / release refusal; replace inline creators with caller-location macros; add try_release prototype. - Context: always-on diagnostic logging; lock errors list before destroy decision; add gitledger_context_try_release(); rename count helper to *_locked. - Error JSON: add rationale comments; CAS-publish json_cache and free via atomic exchange to avoid races. - Tests: safer malloc idiom; add Release-only teardown refusal test using try_release. - Activity: fix legacy epoch timestamps to RFC 3339 strings. - Meson: add strict c_args and -nostdlib link_args for test executables. - Container: export sanitizer/analyzer support env hints; Makefile: add frame pointers and TSAN -O1.
…ix test prototype
…o fix linker failures in CI
…ers; keep strict flags
- Remove -nostdlib from executables (CMake) to link libc in tests. - Make macros portable: avoid GNU ##__VA_ARGS__ by capturing fmt via __VA_ARGS__ only.
…d avoid int<->ptr casts
…x atomic pointer casts
…omic path under C11
…ibility=hidden) and add -nostdlib to all executables (gated for non‑MSVC)
…r non‑MSVC (parity with CMake)
…on returns required length and NUL-terminates
…ics (return required length; NUL on truncation; n=0 no write)
…ck-use-after-return); proper scan-build wrapping
…SON buffer allocations
…ormat attributes to constructors
…d OOM tracking diagnostic and destroy-path invariant comment; clarify header docs
… copying for stable snapshots
…lu); dedupe Meson std flag; add optional -nostdlib for executables (CMake+Meson); prune duplicate container env exports
…wrapping in error_test
…ict .clang-tidy (warnings-as-errors)
…ten .clang-tidy (clang-diagnostic, security, magic-numbers); MSVC to C99
…gnostics; tidy/format clean
… prior exec gating and purity flags
…); keep -nostdlib on library (non‑MSVC)
…onfigured hooks via core.hooksPath, verified tools/hooks/pre-commit runs the same containerised clang-format check as CI, and updated CONTRIBUTING with install/escape instructions. Local matrix green after check.\n\nThis keeps style drift out of the ledger and spares reviewers the churn.
… blurb under Contributing that points newcomers to:\n- make hooks-install (pre-commit runs the containerised format-check),\n- SKIP_FORMAT_CHECK escape hatch for WIP commits,\n- make test-both and make lint to mirror CI locally.\n\nCONTRIBUTING.md remains the canonical detailed guide.
…an ISSUE-DRAFT describing a minimal Linux x86_64 _start shim and a dedicated CI job that builds executables with -nostdlib via GITLEDGER_USE_NOSTDLIB/exec_nostdlib, keeping the main matrix unchanged.
…0\n\nRemoves the literal 10 from the loop to match write_decimal() and the enum constants; behavior unchanged.
…t/linux/x86_64/crt0.S (_start → main → SYS_exit)\n- Add tests/ffs_smoke.c (no libc; exercises version APIs)\n- CMake: gate -nostdlib + CRT to the smoke via GITLEDGER_USE_NOSTDLIB=ON; keep library -nostdlib\n- Meson: add gitledger_ffs_smoke only under -Dexec_nostdlib=true\n- CI: new freestanding-linux job builds and runs ffs_smoke on Ubuntu\n\nScope intentionally minimal; main matrix remains unchanged.
flyingrobots
added
milestone::M2
|
Warning Rate limit exceeded@flyingrobots has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (28)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| gitledger_context_track_error_internal(ctx, err); | ||
| return err; |
There was a problem hiding this comment.
When gitledger_context_alloc fails inside context_register_error, the helper logs a warning and returns without linking the error into the context’s registry. create_error_internal ignores that possibility and still returns the error with err->ctx pointing at the context. The context can subsequently be destroyed because it believes no errors are live, leaving the returned error with a dangling context pointer; any later call to gitledger_error_release or gitledger_error_json will dereference freed memory. The creation path should either detach the context on registration failure or propagate an error instead of returning an untracked object.
Useful? React with 👍 / 👎.
| lea 8(%rsp), %rsi // rsi = &argv[0] | ||
|
|
||
| // Call user main(argc, argv) | ||
| .extern main | ||
| call main |
There was a problem hiding this comment.
The new _start for the freestanding Linux shim calls main with the stack pointer exactly as provided by the kernel. Because a call instruction pushes a return address, main receives %rsp misaligned (16n–8) and the System V AMD64 ABI requirement that the stack be 16‑byte aligned on function entry is violated. Any main compiled with stack‑aligned SSE instructions can fault. Adjust %rsp (e.g. subtract 8 or mask to 16) before the call to maintain ABI alignment.
Useful? React with 👍 / 👎.
M2: Minimal Linux CRT shim (_start) and freestanding CI job (opt‑in)
Summary
Guardrails (as discussed)
CI expectations
References
Policy note