wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity
myxce/src/docs/resreq-ast-cse-impl-plan.md

23 KiB
Raw Blame History

Resreq AST + CSE Implementation Plan

Generated from: resreq-ast-cse-design.md Date: 2026-04-28 Branch: merge_api_server

Summary Priority Order

P0 (blocking): makeString() TOCTOU fix, section.peg boundary bug      ✓ DONE
Phase 1: TDD test skeletons (parallel with Phase 3)                    ✓ DONE
Phase 2: Header definitions + ABI audit                                ✓ DONE
Phase 3: PEG grammar (parallel with Phase 1)                           ✓ DONE  143 PASS / 3 FAIL (stubs)
Phase 4a: Core interning + sel_optimize_level                          ✓ DONE  145 PASS / 1 FAIL (Phase 4b stub)
Phase 4b: Type validation (gated on P0-1)                              ✓ DONE  203 PASS (2026-04-29)
Phase 4c: AND/OR associativity flattening                              ✓ DONE  157 PASS (2026-04-29)
Phase 4d: Const-fold transform                                         ✓ DONE  (2026-04-29)
Phase 4e: Memoized eval                                                ✓ DONE  73 PASS (2026-04-29)
Phase 4f: GC + recompaction                                            ✓ DONE  73 PASS (2026-04-29)
Phase 5: Build wiring ✓ DONE
Phase 6: Scheduler integration                                         ✓ DONE  10 PASS (2026-04-30)

P0 — Critical Prerequisites (blocking gate)

Must complete before any Phase 4 work begins.

# Task Section Rationale
P0-1 Fix makeString() TOCTOU in src/lib/xstring.c DONE — extended lock region to cover makeInstance + key_add, eliminating the TOCTOU window. key_add can no longer return -EEXIST on a miss path. §5.4 Silent NULL return corrupts the interned AST in all §7 numeric-to-string coercions; blocks Phase 4b entirely
P0-2 Fix section.peg clause-boundary grammar DONE — Root cause: the original grammar used multiple ordered-choice alternatives in select_statement / usage_statement / span_statement, but PEG's e* always succeeds (matches empty), so alternatives 2+ were dead code (confirmed by packcc generating "defined but not used" for all pcc_action_select_statement_N functions). Fix: collapsed each statement rule into a single alternative with a compound negative lookahead covering all subsequent clause keywords. select_statement now stops at any of rusage[, span[, same[, affinity[, or ]; usage_statement stops at span[, same[, affinity[, or ]; span_statement stops at same[, affinity[, or ]. Both implicit (type==any span[ptile=1]) and explicit (select[type==any] span[ptile=1]) select forms verified by TDD: test_parse_section3 (implicit), test_parse_section_explicit_select_with_span, test_parse_section_explicit_select_rusage_span, test_parse_section_span_not_in_select — all PASS, no packcc dead-code warnings. §3.1 Silent span/rusage content bleed into selectString in all implicit-select + multi-clause inputs

Phase 1 — TDD Tests (write failing skeletons first) ✓ DONE

Priority: HIGH — must come before implementation (TDD contract)

Develop concurrently with Phase 3 (grammar is the test oracle).

# Task Notes
1-1 Write lib/testsuite/test_select.c skeletons: parse valid expressions, unknown resource names (error), type mismatches (error) DONE
1-2 Write tests: SELECT_MAX_INPUT_LEN (8192) overflow, SELECT_MAX_DEPTH (64) nesting overflow DONE
1-3 Write CSE tests: A&&B and B&&A produce same pointer; two makeSelectNode calls with same args return same pointer DONE — FAIL until Phase 4a
1-4 ~~Write eval tests: numeric >/</==, string ==, bool standalone, !/&&/`
1-5 Write GC tests: unlink job root → gc_sweep_select_nodes frees unreachable node; subsequent makeSelectNode re-interns fresh node DONE — FAIL until Phase 4f
1-6 Write GC predicate contract tests (sched_session_active, sched_eval_in_progress, staging_merge_complete) using fake_scheduler_t scaffold in test_select.c DONE — FAIL until Phase 4f Phase 6 wires real scheduler — signatures stable
1-7 Create shared resmgr fixture: src/lib/testsuite/test_resmgr_fixture.h + .c with make_test_resmgr()/free_test_resmgr() DONE

Phase 2 — Header Definitions ✓ DONE

Priority: HIGH — unblocks all implementation work

# Task Notes
2-1 Add res_type_t, sel_node_type_t, xce_select_node_t typedef + struct to include/resreq.h DONE
2-2 Fill struct xce_resreq_select_ with xce_select_node_t* root DONE
2-3 Declare all public APIs: makeSelectNode, eval_select, eval_select_simple, select_eval_ctx_*, sel_node_table_*, gc_sweep_select_nodes, recompact_node_ids DONE gc_mark_tree stays static in .c
2-4 Add var resmgr field to resreq_context_t (outer PEG auxil) DONE
2-5 Add valtype_to_res_type helper; omit res_type_to_valtype (no current consumer — YAGNI) DONE
2-6 Add xce_str_match as static inline wrapping strcmp DONE
2-7 CGO audit: grep -rn 'parse_resreq|C\.parse_resreq' src/frontend/ DONE — no CGO callers found
2-8 Update parse_resreq_primary and parse_resreq signatures to accept var resmgr DONE — call sites updated: schedobj.c:171, test_resreq.c:244/301/330

Phase 3 — PEG Grammar (lib/xce_select.peg) ✓ DONE

Priority: HIGH — develop concurrently with Phase 1

# Task Notes
3-1 Implement select_expr grammar (§3.1): ||, &&, !, comparisons, resname, literals, any DONE
3-2 Implement select_context_t auxil with resmgr, depth, errmsg[256], has_error DONE
3-3 Implement resname action: key_find, res_idx validity guard, valtype_to_res_type mapping DONE
3-4 Clause-keyword conflict check in resname action: check against {select, rusage, span, same, affinity} DONE
3-5 Implement SELECT_MAX_INPUT_LEN (8192) and SELECT_MAX_DEPTH (64) guards DONE
3-6 LSF-compatible disambiguation: bare word vs known resource name → SEL_RESNAME vs SEL_STR; any excluded by grammar; true/false produce migration diagnostic DONE
3-R1 [Review fix — HIGH] Switch SEL_STR from strdup/free to makeString/dereference in xce_select.peg and xce_select.c DONE — xstring-interned strings now refcounted; kill_select_node calls dereference()
3-R2 [Review fix — MEDIUM] Add _Static_assert(sizeof(uintptr_t) >= sizeof(double)) before makeSelectNode in xce_select.c DONE — guards the SEL_NUM bit-cast at compile time
3-R3 [Review fix — MEDIUM] Add !("any" ![a-zA-Z_0-9]) negative lookahead to resname_node and resname_leaf in xce_select.peg DONE — prevents any keyword from being tried as a resource name

Phase 4a — Core Node Allocation and Interning ✓ DONE

Priority: HIGH — foundational; unblocks 4b–4f

# Task Notes
4a-1 Implement sel_optimize_level (static _Atomic int, sentinel -1) with sel_optimize_set() write-once CAS and sel_get_optimize_level() accessor DONE
4a-2 Implement sel_make_key() — zero padding bytes via struct literal init; SEL_RESNAME zeroes op2 for key purposes (res_type stored in node, not in key) DONE
4a-3 Implement sel_key_hash over the 3 key fields DONE
4a-4 Implement sel_node_table_init / sel_node_table_fini — makeInstance(&kvtab_type, &params) with datakiller=NULL, zarray_init for linear nodes array DONE
4a-5 Implement makeSelectNode — CSE-disabled fast path; CSE-enabled path with commutativity sort, key lookup, TOCTOU double-check under lock, node_id assignment via atomic_fetch_add, zarray rollback via atomic_fetch_sub DONE
4a-6 Implement sel_node_to_key() for GC key reconstruction (must apply same commutativity sort as makeSelectNode) DONE
4a-7 Add sel_optimize_test_reset() test-only helper (#ifdef UNIT_TEST): atomic_store_explicit reset to -1 plus sel_node_table_fini DONE
4a-R1 [Review fix — HIGH] Move node_id assignment inside lock after key_add succeeds so TOCTOU losers need no counter rollback DONE Counter divergence under concurrent load
4a-R2 [Review fix — HIGH] Add dereference((char*)op1) on first-lock cache hit for SEL_STR — caller transfers xstring ownership DONE Ref leak on every cache hit for string nodes
4a-R3 [Review fix — HIGH] Add SEL_MAX_INTERNED_NODES cap check in both lock sections (first miss + TOCTOU re-check) DONE Unbounded table growth without cap
4a-R4 [Review fix — HIGH] Fix sel_node_table_fini to iterate and free all interned nodes before destroy(table) — NULL datakiller does not free them DONE All nodes leaked at teardown; LSAN clean now
4a-R5 [Review fix — MEDIUM] Set node->hashcode = sel_key_hash_fn(&key) in CSE path — was left 0, breaking Phase 4e memoization DONE
4a-R6 [Review fix — MEDIUM] Add dereference((char*)op1) on calloc failure in level==0 path DONE xstring ref leak on OOM
4a-R7 [Review fix — MEDIUM] Add -EINVAL guard on key_find — corrupt table must not silently proceed DONE
4a-R8 [Review fix — MEDIUM] Add -EEXIST recovery in key_add failure path — re-find and return the racing winner DONE Rare concurrent insert race
4a-R9 [Review fix — LOW] Change _Static_assert(sizeof(uintptr_t) >= ...) to == per spec requirement DONE

Phase 4b — Parse-Time Type Validation ✓ DONE (2026-04-29)

Priority: HIGH — gate: P0-1 makeString() fix must be merged first

# Task Notes
4b-1 Wire parse_select() into parse_resreq_primary before kill_resreq_data DONE Use-after-free if called after kill_resreq_data frees data.selectString
4b-2 Implement EQ/NE type validation: reject BOOL×STR; coerce SEL_NUM to SEL_STR when comparing with string resource (snprintf "%.17g"), NULL-check makeString result, symmetric coercion DONE Free orphaned SEL_NUM node in unoptimized mode before overwriting pointer
4b-3 Implement >/</>=/<= type validation: reject non-numeric operands DONE log_fatal (not assert) for belt-and-suspenders at eval time too
4b-4 Implement res_idx validity guard and unknown-resource error; add migration diagnostic for true/false identifiers (parse as SEL_RESNAME with no match) DONE
4b-5 Update kill_primary_resreq for conditional teardown: CSE-enabled → null root only; CSE-disabled → free_select_tree() then FREEUP DONE NULL store before FREEUP to prevent GC mark dangling-pointer race
4b-6 LC_NUMERIC enforcement: wrap PEG parser call in parse_select with newlocale(LC_NUMERIC_MASK, "C") / uselocale / freelocale; log_fatal on failure in scheduler, log_error+return in API server DONE _XOPEN_SOURCE 700 required before system headers
4b-7 strtod locale coverage check DONE — SAFE: packcc emits single strtod in pcc_action_value_node_1 (line 1593 of generated file), inside the uselocale scope — no locale vulnerability
4b-R1 [Review fix — HIGH] Add lock(auxil->resmgr, RTTI_LOCK_READ) / unlock(auxil->resmgr) around key_find in resolve_resname() (xce_select.peg:38) — missing read lock caused data race in multi-threaded scheduler DONE
4b-R2 [Review check — F2] STRING×NUM coercion error paths reviewed: makeSelectNode releases the xstring on OOM failure (level-0 path calls dereference(op1) before returning NULL); outer makeSelectNode(op_type) failure correctly calls SEL_DISCARD(l) + SEL_DISCARD(r) — no leak, false positive

Phase 4c — Commutativity + Associativity Flattening ✓ DONE (2026-04-29)

# Task Notes
4c-1 Verify commutativity sort in makeSelectNode step 1 covers AND/OR/EQ/NE; non-commutative GT/LT/GE/LE/NOT unchanged DONE — test_cse_gt_not_commutative confirms GT is not commutative; AND/OR/EQ/NE sort verified. Already integrated in §5.4 design
4c-2 ~~Implement post-parse flatten pass for &&/`
4c-3 In unoptimized mode: flatten_pass must free() each orphaned node — not its children (re-parented children must not be freed) DONE — level==0 path frees orphaned intermediate node (free(new_right); free(node)) after re-parenting A/B/C; SEL_NOT branch frees node only. Children freed would cause double-free when free_select_tree walks from root

Senior review fixes applied (2026-04-29):

  • [MEDIUM — OOM safety] In AND/OR branch: update node->u.binop.left/right in-place (level==0) immediately after recursive calls, before any makeSelectNode. This ensures return node after OOM returns a valid tree with live (not dangling) children.
  • [LOW — dead code] Removed dead if (node->type == SEL_STR ...) / if (new_right->type == SEL_STR ...) guards in AND/OR and SEL_NOT branches — those nodes are never SEL_STR in those arms.

Phase 4d — Const-Fold Transform ✓ DONE (2026-04-29)

# Task Notes
4d-1 resname == any → SEL_TRUE; resname != any → SEL_FALSE — check both child slots for SEL_ANY pointer (commutativity sort is ASLR-nondeterministic across runs) DONE
4d-2 Identity laws: SEL_TRUE && X → X; SEL_FALSE || X → X DONE
4d-3 Annihilation laws: SEL_FALSE && X → SEL_FALSE; SEL_TRUE || X → SEL_TRUE DONE
4d-4 Complement laws: !SEL_TRUE → SEL_FALSE; !SEL_FALSE → SEL_TRUE DONE Single-pass — const-fold shrinks trees, does not create new flattening opportunities

Phase 4e — Evaluation with Memoization ✓ DONE (2026-04-29)

# Task Notes
4e-1 Implement eval_get_bool/eval_set_bool 2-bit bitmap accessors with node_id >= bool_bits_cap guards using log_fatal DONE _Static_assert(EVAL_NONE == 0) for memset-reset correctness
4e-2 ~~Implement eval_select_inner — recursive eval with explicit error propagation (if (lv < 0) return -1), NOT C &&/`
4e-3 Leaf cache writes in eval_select_inner: SEL_RESNAME writes to ctx->num_vals[node_id] or ctx->str_vals[node_id]; comparison ops call eval_select(child, ctx) then read cached arrays DONE Critical: get_leaf_value() must NOT be called directly — bypasses cache writes, silently reads 0.0/NULL
4e-4 Implement memoized eval_select wrapper — bitmap cache check before calling eval_select_inner; cache EVAL_ERROR too (node-structural errors repeat per host) DONE
4e-5 Implement eval_select_simple for unoptimized path — plain recursive tree-walk, no select_eval_ctx_t, thread-safe (no shared state) DONE
4e-6 Add eval_select guard: log_fatal if sel_get_optimize_level() <= 0 DONE
4e-7 Implement select_eval_ctx_init / select_eval_ctx_reset / select_eval_ctx_fini DONE node_count snapshot at init; reset uses snapshot, never re-reads sel_node_next_id
4e-8 Pre-session capacity check: memset extended bool_bits bytes after realloc (realloc does not zero extension); partial-OOM contract — all three reallocs (bool_bits, num_vals, str_vals) must succeed atomically or log_fatal DONE Mismatched capacities allow out-of-bounds write to num_vals with no inline bounds check

Phase 4f — Mark-and-Sweep GC + Recompaction ✓ DONE (2026-04-29)

# Task Notes
4f-1 Implement gc_mark_tree — recursive DFS, early return on node == NULL || node->gc_mark; handles leaf / SEL_NOT / binary node types DONE — __attribute__((unused)) for Phase 6 wire-in Recursion bounded by SELECT_MAX_DEPTH (64); any increase requires stack analysis
4f-2 Implement gc_sweep_select_nodes — Phase 6 guard placed before mark-clear (prevents two-call sweep-of-reachable-nodes bug); mark-clear iterates caps; sweep iterates caps, zarray_del before free DONE — live_jobs traversal deferred to Phase 6 (sched/types.h not in libxce scope) GC does NOT acquire sel_node_table.lock (idle-phase invariant)
4f-3 Insert atomic_thread_fence(memory_order_seq_cst) after all primary->select->root = NULL unlinks and before gc_sweep_select_nodes call — Phase 6 call-site task Without fence: CPU/compiler may reorder NULL store after GC mark pass
4f-4 Runtime GC idle invariant guards at GC call site (sched_session_active, sched_eval_in_progress, staging_merge_complete) — log_fatal not assert — Phase 6 call-site task → see 6-3/6-4 assert compiled out by NDEBUG; use-after-free would result
4f-5 Implement recompact_node_ids — iterate nodes zarray over caps slots (not count, gaps exist after zarray_del), assign sequential node_id, reset sel_node_next_id via atomic_store_explicit(..., memory_order_release) DONE node_id and zarray slot index are decoupled — do not conflate
4f-R1 [Review fix — HIGH] Move Phase 6 early-return guard to before mark-clear loop — prior ordering allowed two consecutive calls (njobs>0 then njobs==0) to sweep still-reachable nodes DONE Two-call scenario: first call clears all marks and returns; second call sweeps everything
4f-R2 [Review fix — HIGH] Swap free(node) / zarray_del order — zarray_del NULLs slot first, then free; eliminates latent use-after-free if killer is ever installed on nodes zarray DONE
4f-R3 [Bug fix] sel_node_table_fini iterated nodes.count instead of nodes.caps — after zarray_del creates gaps, new nodes slot into positions beyond count boundary and leaked DONE Manifested as 32-byte ASAN leak after GC sweep + reintern in same lifetime

Phase 5 — Build Wiring

# Task Notes
5-1 Add custom_target for xce_select.peg in lib/meson.build matching section_parser/rusage_parser pattern DONE Verify -o 'lib/xce_select' path format: grep -A8 'custom_target' src/lib/meson.build | grep '\-o'
5-2 Add xce_select_parser[0] (generated .c) to libxce_src DONE
5-3 Add test_resmgr_fixture.c to testsuite executable() sources in lib/meson.build DONE

Phase 6 — Scheduler Integration

Status: ✓ DONE — 2026-04-30 — 10/10 scheduler tests pass

# Task Notes
6-1 Pre-Phase-6 audit: confirm allocator-thread-only invariant DONE bb4a95c Single-threaded confirmed; stub at resource.c:127
6-2 Blocking gate: all sched_meet_host_selection call sites accounted for DONE bb4a95c No missed call sites
6-3 Add _Atomic int session_active, eval_in_progress, staging_merge_complete to xce_scheduler_t DONE 823a66e Use _Atomic int not volatile int — ARM atomicity guarantee
6-4 Implement GC predicate wrappers referencing real xce_scheduler_t fields DONE acda9bb Signatures unchanged; only wrapper body changed
6-5 Implement sched_meet_host_selection via eval_select_simple DONE 40261b7 Full implementation replacing stub that always returned 1
6-6 sched_meet_host_selection returns int (0/1); preason on fail DONE 40261b7 int return kept; preason wired
6-7 Implement build_host_eval_ctx DONE 907d8d8 Flattens worker->resvals zarray to numvals/strvals using Value_u_fvalue/svalue/bvalue
6-8 Materialize live_jobs flat array from sched->jobs kvmgr DONE 5267d36 Growth-loop heap alloc; OOM-safe (double-guarded: calloc fail + realloc null-sentinel 742d79e, fb5127a)
6-9 Wire sel_optimize_set(1) at scheduler startup DONE acda9bb Called in sched_init; no sel_optimize_set(0) needed (API server has separate process)
6-10 Set sel_alloc_thread_id; add thread-ID checks in eval_select and gc_sweep_select_nodes DONE b722e5c 1eb4eb1 _Atomic int registered flag + pthread_equal guard; registered before while(1) loop
6-11 Write integration/parity tests against known-good resreq/host pairs DONE 5fcf85c Numeric (SEL_GE) and string (SEL_EQ) pass/fail cases
6-12 atomic_thread_fence(memory_order_seq_cst) before gc_sweep_select_nodes DONE 5267d36 After all gc_mark_select_tree calls in sched_end_session
R1 [Review fix — CRITICAL] staging_merge_complete bootstrap deadlock — GC never ran DONE d438742 sched_begin_session never set it to 1; guard in sched_end_session always fired
R2 [Review fix — HIGH] recompact_node_ids() missing from GC protocol DONE d438742 Added after gc_sweep_select_nodes in sched_end_session

Key Implementation Traps

  1. free_select_tree vs GC mode confusion — calling free_select_tree in CSE-enabled mode or forgetting it in disabled mode both produce memory corruption. The sel_get_optimize_level() guard inside free_select_tree catches this with log_fatal.

  2. get_leaf_value() bypass bug — calling get_leaf_value() directly in comparison branches bypasses the num_vals/str_vals cache write. Comparisons silently read 0.0/NULL (calloc default) instead of actual host values. Every comparison operator must call eval_select(child, ctx) first, then read the typed cache array.

  3. TOCTOU in makeSelectNode — the double-check under lock (step 5) prevents node leaks when two threads race to intern the same node. The re-find must happen inside the lock or the winner could be freed before the loser reads it.

  4. atomic_thread_fence before GC — root-unlink writes (primary->select->root = NULL) must be visible to the GC mark pass. Without a seq_cst fence, CPU/compiler reordering causes GC to retain a node that should be swept (or vice versa).

  5. Partial-OOM in select_eval_ctx_t resize — if one of the three reallocs (bool_bits, num_vals, str_vals) succeeds and another fails, the capacities diverge and eval_select_inner can write out-of-bounds to num_vals (no inline bounds guard at write site). Treat all three as atomic — log_fatal on partial failure.