Skip to content

Generated-code allocator correctness - #45

Merged
sqt merged 2 commits into
mainfrom
generated_code_allocator_correctness
Aug 25, 2026
Merged

Generated-code allocator correctness#45
sqt merged 2 commits into
mainfrom
generated_code_allocator_correctness

Conversation

@sqt

@sqt sqt commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Motivation

Several C-ABI code paths generated by the Zig and C++ backends hardcoded a fixed allocator (std.heap.c_allocator, plain new/std::unordered_map) instead of routing through the allocator an application actually configured (zzdds_create_factory_with_allocator, zidl_cdr_set_allocator). Under a real custom allocator — exactly what embedded/real-time consumers need — this caused genuine crashes (free(): invalid pointer, operator new() aborts). Found and fixed four confirmed, previously-undetected instances of this bug class during a broader design review of generated-type lifecycle and allocator sourcing.

Description of changes

  • New get_allocator vtable slot (mirrors the existing get_c_abi_handle slot) lets a generated C-ABI operation wrapper resolve the producing entity's own allocator instead of guessing at a process-wide default — fixes native-temporary-buffer frees for entity-sequence-returning operations (WaitSet::wait(), Subscriber::get_datareaders(), etc.) and an operation's own by-value return path.
  • New zidl_cdr_get_allocator() getter (paired with the existing setter) lets frees with no entity in scope — standalone sequence typedefs, C-ABI struct mirrors used for QoS/config types — route through the process-wide registered allocator instead of a hardcoded default.
  • Fixed the C++ backend's shared entity identity-cache (_familyCache) to use std::pmr::unordered_map, matching the already-pmr-routed values it stores.
  • New --audit-lifecycle diagnostic flag: classifies every operation's heap-owning output as entity-mediated vs. standalone, flagging ambiguous cases for human review — used to sweep dcps.idl/zzdds.idl for further instances of this bug class.
  • New permanent regression test scanning the Zig backend's own source for any reintroduced hardcoded-allocator pattern.

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown

Greptile Summary

The PR routes generated C-ABI lifecycle allocations through their configured allocator while preserving dependency-free Zig C-API generation by making native CDR allocator lookup opt-in.

  • Adds the native CDR allocator getter and entity-vtable allocator access.
  • Uses allocator-aware storage for generated C++ family identity caches.
  • Adds lifecycle auditing and allocator-regression coverage.
  • Gates the new zidl_cdr_get_allocator link dependency behind --zig-generate-c-api-cdr-allocator.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported native CDR link dependency is now isolated behind an explicit opt-in flag, and ordinary Zig C-API generation emits no reference to that external symbol.

Important Files Changed

Filename Overview
src/backend/zig.zig Routes generated C-ABI allocations and frees through entity or standalone allocator helpers, with the native CDR dependency correctly gated behind an explicit option.
src/backend/interface.zig Adds the opt-in Zig C-API CDR allocator option and documents its native-library dependency.
src/main.zig Parses and forwards the allocator-routing and lifecycle-audit options into backend generation.
packages/zidl-cdr/include/zidl_cdr.h Exposes the currently registered process-wide allocator through the native CDR API.
packages/zidl-cdr/src/zidl_cdr.c Implements the allocator getter consistently with the existing process-wide registration.
src/backend/cpp.zig Changes shared entity-family identity caches to PMR-backed unordered maps so container allocations use the configured C++ resource.
src/backend/lifecycle_audit.zig Adds diagnostic classification for generated heap-owning operation outputs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Generated C-ABI allocation or free] --> B{Entity allocator available?}
    B -- Yes --> C[Resolve allocator through entity vtable]
    B -- No --> D{CDR allocator option enabled?}
    D -- Yes --> E[zidl_cdr_get_allocator]
    D -- No --> F[std.heap.c_allocator fallback]
    E --> G[Registered process-wide allocator or libc fallback]
    C --> H[Matching allocation lifecycle]
    F --> H
    G --> H
Loading

Reviews (2): Last reviewed commit: "resolving review comments" | Re-trigger Greptile

Comment thread src/backend/zig.zig Outdated
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.72193% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/backend/lifecycle_audit.zig 96.42% 4 Missing ⚠️
packages/zidl-cdr/src/zidl_cdr.c 0.00% 2 Missing ⚠️
src/backend/zig.zig 96.82% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@sqt
sqt merged commit 20d35e9 into main Aug 25, 2026
8 checks passed
@sqt
sqt deleted the generated_code_allocator_correctness branch August 25, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant