Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -1153,10 +1153,21 @@ Known weaknesses, so you neither trip over them nor assume they are intentional:
(CR-2026-032).** `make test-c` builds the three previously-orphaned C test files and runs
`tools/c-corpus-harness.py`, which generates C that builds each expressible corpus schema
through the struct API, compiles it, and compares the decode against the vectors.
**453 of 453 attempted vectors pass** since CR-2026-033 added `tlv` (it was 50 of 50
before). 786 of 1239 are still in schemas the struct API cannot build - 34 need
`flagged`, 24 a `bitfield_string`, 15 more cases than `SCHEMA_MAX_CASES` allows, 3
`repeat`, one an enum/lookup `default` the struct has no slot for.
**488 of 488 attempted vectors pass** since CR-2026-033 added `tlv` and CR-2026-034
`flagged` (it was 50 of 50 before either). 751 of 1239 are still in schemas the struct API
cannot build - 26 use a `transform` chain, 24 a `bitfield_string`, 15 more cases than
`SCHEMA_MAX_CASES` allows, 3 `repeat`, 3 a `u32le16` **the interpreter has and the harness
has no constructor for**. The report names which side each limit is on, and that
distinction is load-bearing: `no constructor for type 'u32le16'` used to read as a C gap
when C decodes it perfectly well.

**A `flagged` construct's mask field must declare `var_name`.** This interpreter records a
value in its variable table only where a field declares one, while a YAML `flagged` names a
field - so the harness patches it in. `var_has()` exists so a missing reference is
`SCHEMA_ERR_MATCH` rather than a mask of zero, which would decode nothing and report
success. A `tlv`, `flagged` or `match` case body goes **above `field_count`**, reached only
through `case_def_t.field_start`; adding it as a counted field makes the top-level loop
decode it twice, which is what the existing `match` tests tolerate.

**`tlv` in C, and its limits.** A tag is packed into `case_def_t.match_value` - one byte
is its own value, two components are `(first << 8) | second`, which is exact because
Expand All @@ -1172,8 +1183,9 @@ Known weaknesses, so you neither trip over them nor assume they are intentional:
**The fixed-size limits are the real boundary, not an oversight.** `sizeof(schema_t)` is
51 KB because every `field_def` carries `cases[16]` and `lookup[16]` unconditionally.
Raising `SCHEMA_MAX_FIELDS` to fit mla20's 67 fields would put it past 110 KB, which
defeats the point of a firmware-tier interpreter. `flagged` is the largest remaining gap
and the next construct if C is to go further.
defeats the point of a firmware-tier interpreter. `repeat` is the last construct it has
no field type for, and at 3 schemas it is worth less than widening the harness to cover
`transform` (26) and `bitfield_string` (24).

Two things to keep straight when reading that report. **A skipped schema is not a passing
one**, and **a harness limitation is not a C gap** - inline `match`, `byte_group`,
Expand Down
96 changes: 95 additions & 1 deletion include/schema_interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ typedef enum {
FIELD_TYPE_UNKNOWN,
/* Appended after UNKNOWN on purpose: UNKNOWN is parse_type_string's sentinel and
* inserting before it would renumber it (CR-2026-033). */
FIELD_TYPE_TLV
FIELD_TYPE_TLV,
FIELD_TYPE_FLAGGED
} field_type_t;

typedef enum {
Expand Down Expand Up @@ -515,6 +516,16 @@ static inline void var_set(var_context_t* ctx, const char* name, int64_t value)
}
}

/* Whether a variable was ever set. `var_get` returns 0 for a miss, which for a `flagged`
* mask is indistinguishable from "no bits set" - so the construct would decode nothing and
* report success. The reference interpreter raises instead (CR-2026-034). */
static inline bool var_has(const var_context_t* ctx, const char* name) {
for (int i = 0; i < ctx->count; i++) {
if (strcmp(ctx->vars[i].name, name) == 0) return true;
}
return false;
}

static inline int64_t var_get(var_context_t* ctx, const char* name) {
for (int i = 0; i < ctx->count; i++) {
if (strcmp(ctx->vars[i].name, name) == 0) {
Expand Down Expand Up @@ -560,6 +571,41 @@ static inline int schema_tlv_tag(const int* parts, int count) {
return packed;
}

/* A `flagged` construct: a previously decoded field read as a bitmask, with a group of
* fields per bit (PS-158..PS-163).
*
* `flags_field` names the field holding the mask. **That field must carry `var_name`**, or
* the mask cannot be found: this interpreter records a value in the variable table only
* where a field declares one, and `flagged` refers to a field by name rather than by
* variable. A builder that forgets it gets SCHEMA_ERR_MATCH rather than a silent decode of
* nothing (CR-2026-034).
*
* Reuses `match_var` and `cases[]`: a group is a case whose match_value is its bit
* position, and whose field_start/field_count point at a body placed above field_count. */
static inline field_def_t field_flagged(const char* flags_field) {
field_def_t f;
memset(&f, 0, sizeof(f));
f.type = FIELD_TYPE_FLAGGED;
strncpy(f.match_var, flags_field, SCHEMA_MAX_NAME_LEN - 1);
return f;
}

/* Add a flagged group: the bit that selects it, and the range of fields it contributes.
* Groups are evaluated in the order added, which is the order their bytes appear (PS-160). */
static inline bool field_add_flagged_group(field_def_t* f, int bit,
int field_start, int field_count) {
if (f->case_count >= SCHEMA_MAX_CASES) return false;
if (bit < 0 || bit > 63) return false;
case_def_t* c = &f->cases[f->case_count];
memset(c, 0, sizeof(*c));
c->match_value = bit;
c->match_list[0] = -1;
c->field_start = field_start;
c->field_count = field_count;
f->case_count++;
return true;
}

/* Add a tlv case: its packed tag, and the range of schema fields holding its body. */
static inline bool field_add_tlv_case(field_def_t* f, int packed_tag,
int field_start, int field_count) {
Expand Down Expand Up @@ -1033,6 +1079,54 @@ static inline int schema_decode_direction(
for (int i = 0; i < schema->field_count; i++) {
const field_def_t* field = &schema->fields[i];

/* A `flagged` construct: the bits of an already-decoded field select which groups
* contribute (PS-158). Groups are walked in the order they were added, which is the
* order their bytes appear (PS-160); a clear bit consumes nothing (PS-162) and a set
* one decodes its body into the flat result (PS-163). Added by CR-2026-034.
*
* The mask is read from the variable table, so the field holding it must declare
* `var_name` - see field_flagged(). A missing reference is an error rather than a
* mask of zero, which would decode nothing and report success. */
if (field->type == FIELD_TYPE_FLAGGED) {
const char* flags_name = field->match_var;
if (flags_name[0] == '$') flags_name++;
if (!var_has(&vars, flags_name)) {
result->error_code = SCHEMA_ERR_MATCH;
snprintf(result->error_msg, sizeof(result->error_msg),
"flagged field reference not found: %s", flags_name);
return SCHEMA_ERR_MATCH;
}
const int64_t flags = var_get(&vars, flags_name);

for (int c = 0; c < field->case_count; c++) {
const case_def_t* group = &field->cases[c];
if (((flags >> group->match_value) & 1) == 0) continue;

for (int f = 0; f < group->field_count; f++) {
int field_idx = group->field_start + f;
/* Bounded by the array: a group body is placed above field_count so
* the top-level loop does not walk it, exactly as a tlv case is. */
if (field_idx < 0 || field_idx >= SCHEMA_MAX_FIELDS) break;
if (result->field_count >= SCHEMA_MAX_FIELDS) break;

int rc = decode_field(
&schema->fields[field_idx],
buf, len, &pos,
&result->fields[result->field_count],
&vars, schema->endian
);
if (rc != SCHEMA_OK) {
result->error_code = rc;
return rc;
}
if (result->fields[result->field_count].valid) {
result->field_count++;
}
}
}
continue;
}

/* A tlv loop: read a tag, decode the case describing it, repeat to the end of the
* payload (PS-153, PS-154). Added by CR-2026-033; before it this interpreter had
* no field type for the construct at all, so 79 of the corpus's schemas could not
Expand Down
7 changes: 4 additions & 3 deletions tests/test_cr_2026_032_c_corpus_harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@

#: Constructs the C interpreter has no field type for. Distinct from what the harness
#: cannot build, and the distinction is the point.
#: `no tlv field type` was here until CR-2026-033 added one; the interpreter now decodes
#: the construct and the harness builds it, so it is no longer a reason at all.
C_GAPS = ("no flagged field type", "no constructor for type 'repeat'")
#: `no tlv field type` was here until CR-2026-033 added one and `no flagged field type`
#: until CR-2026-034; the interpreter decodes both now and the harness builds them, so
#: neither is a reason any more. `repeat` is the last construct it has no field type for.
C_GAPS = ("no 'repeat' field type",)


def harness(*args):
Expand Down
22 changes: 14 additions & 8 deletions tests/test_cr_2026_033_c_tlv.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
- **`unknown: raw` is not supported**, for the same reason: the captured bytes need
somewhere to go. `skip` and `error` are.

What the C interpreter still cannot do, now measured rather than assumed: 34 schemas need
`flagged`, 24 a `bitfield_string`, 15 more cases than `SCHEMA_MAX_CASES` allows, 3 `repeat`.
What the C interpreter still could not do when this landed: 34 schemas needed `flagged`
(closed by CR-2026-034), 24 a `bitfield_string`, 15 more cases than `SCHEMA_MAX_CASES`
allows, 3 `repeat`.
That last group is the fixed-size boundary and is the honest limit of a firmware-tier
interpreter: `sizeof(schema_t)` is already 51 KB because every `field_def` carries
`cases[16]` and `lookup[16]` unconditionally, and raising the limits to fit mla20's 67
Expand Down Expand Up @@ -68,9 +69,14 @@ def test_the_field_type_exists(self):
assert "FIELD_TYPE_TLV" in HEADER.read_text()

def test_it_is_appended_after_the_sentinel(self):
"""UNKNOWN is parse_type_string's sentinel; inserting before it renumbers it."""
"""UNKNOWN is parse_type_string's sentinel; inserting before it renumbers it.

Compared by position, not by adjacency to the closing brace. This asserted
`"FIELD_TYPE_TLV\n} field_type_t;"` - which pinned TLV as the *last* member, an
incidental fact that CR-2026-034 broke by appending FIELD_TYPE_FLAGGED after it.
"""
text = HEADER.read_text()
assert text.index("FIELD_TYPE_UNKNOWN,") < text.index("FIELD_TYPE_TLV\n} field_type_t;")
assert text.index("FIELD_TYPE_UNKNOWN,") < text.index("FIELD_TYPE_TLV")

def test_there_is_a_constructor_for_both_tag_forms(self):
text = HEADER.read_text()
Expand Down Expand Up @@ -172,10 +178,10 @@ def test_tlv_is_no_longer_a_skip_reason(self, report):
def test_the_accounting_still_holds(self, report):
assert report["attempted"] + report["skipped_vectors"] == report["corpus_vectors"]

def test_flagged_is_now_the_largest_gap(self, report):
"""Which makes it the next construct, if C is to go further."""
largest = max(report["skips"].items(), key=lambda kv: kv[1])
assert largest[0] == "no flagged field type", largest
def test_flagged_is_no_longer_a_gap_either(self, report):
"""It was the largest when this CR landed; CR-2026-034 closed it."""
assert not any("flagged field type" in r for r in report["skips"]), \
sorted(report["skips"])


class TestTheExistingSelftestsStillPass:
Expand Down
Loading
Loading