Skip to content

Commit 081a097

Browse files
H. Peter Anvin (Intel)Copilot
andcommitted
testgen: document %ifdef ERROR error-case coverage
Adds a "Error-case (%ifdef ERROR) coverage" section describing the needs64/avoid64 dual-source design and the nasm-t.py json convention reused from travis/ret/ret.json, notes on the two branch-operand bugs discovered and fixed along the way, and updates the "Known limitations" and "Validation" sections with the final 2612/10 mnemonic counts and 1976/2612 error-coverage numbers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent eee1384 commit 081a097

1 file changed

Lines changed: 80 additions & 3 deletions

File tree

tools/testgen/README.md

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ into the existing `tools/travis/nasm-t.py` harness. This gives every
88
instruction-set pattern regression coverage without hand-writing
99
thousands of test cases.
1010

11-
The generated tree currently checked in lives at `travis/insns/` (2606
11+
The generated tree currently checked in lives at `travis/insns/` (2612
1212
mnemonic subdirectories as of the last full run).
1313

1414
## Usage
@@ -361,6 +361,64 @@ Two wrinkles this created and how they're handled:
361361
base-free scaled vsib index forms (`[xmm0*1]`). This sacrifices
362362
coverage of true `[base+index*scale+disp]` forms — a known,
363363
documented limitation, not a bug.
364+
- **16/32-bit-only tokens poisoning the 64-bit probe.** The mirror
365+
image of the first wrinkle: CALL/JMP's near-indirect targets accept
366+
`rm16`/`rm32`/`rm64` operands, but only `rm64` is valid in 64-bit
367+
mode (`call cx`/`call ecx` both fail to assemble under `--bits 64`,
368+
matching the `NOLONG` flag on those `insns.xda` templates) — unlike
369+
an ordinary `reg16`/`reg32` operand elsewhere, which assembles fine
370+
at any bit width. `branch_narrow_only()` flags lines built from these
371+
tokens (`avoid64`, mirroring `needs64`) and excludes them from the
372+
64-bit "full" body; see "Error-case (`%ifdef ERROR`) coverage" below
373+
for where they end up instead.
374+
375+
## Error-case (`%ifdef ERROR`) coverage
376+
377+
Verifying that an instruction *fails* to assemble the way it should
378+
(and prints the expected diagnostic) is itself test coverage, not just
379+
verifying the ways it succeeds. Per the preferred convention, error
380+
cases don't get a separate source file: known-bad lines are appended
381+
to the *same* per-mnemonic `.asm` file under a `%ifdef ERROR` guard,
382+
and the file is assembled twice — once plainly (existing behavior,
383+
unaffected, since the guarded block is invisible) and once with
384+
`-DERROR` defined (where the guarded block is included and expected to
385+
fail). This mirrors the pre-existing hand-written convention already
386+
used by `travis/ret/ret.json` (`"option": "-DERROR ...", "error":
387+
"expected"`).
388+
389+
Two kinds of already-known-bad lines feed this, both by-products of the
390+
bit-width handling above rather than newly invented content:
391+
392+
- Lines flagged `needs64` (64-bit-only operands, hireg/apxreg
393+
register-number lines) are appended to the 16/32-bit "narrow" body
394+
and probed with `-DERROR` at `--bits 16`/`32`.
395+
- Lines flagged `avoid64` (CALL/JMP's rm16/rm32 near-indirect targets)
396+
are appended to the 64-bit "full" body and probed with `-DERROR` at
397+
`--bits 64`.
398+
399+
Each candidate block is probed with `-DERROR` before becoming a json
400+
entry, and is only kept for the specific bit width(s) where it actually
401+
fails — this generator doesn't model every mode restriction, so a
402+
block that unexpectedly *does* assemble at some width simply doesn't
403+
get an "expected error" entry for that width, rather than becoming a
404+
false test failure. A mnemonic whose only surviving coverage would be
405+
error entries (no working positive-path line at any bit width) is
406+
still dropped entirely, same as before — "this instruction never
407+
assembles" isn't meaningful regression coverage by itself.
408+
409+
While wiring this up, two latent bugs in the pre-existing branch-target
410+
operand handling surfaced and were fixed: `gen_operand()`'s branch-mode
411+
substitution used to replace *every* operand of a branch mnemonic
412+
(`is_branch`) with the `.L1` local-label text, not just genuine
413+
relative-displacement targets — producing nonsense like `loop .L1,
414+
.L1` (LOOP's address-size-override form takes a fixed `cx`/`ecx`/`rcx`
415+
register, not a branch target) and `call .L1` for JMP/CALL's indirect
416+
(`rm16`/`32`/`64`) and far-pointer (`imm16:imm16`) operand forms. These
417+
bogus lines silently broke assembly for the whole mnemonic, and
418+
LOOP/LOOPE/LOOPNE/LOOPNZ/LOOPZ/JCXZ were silently dropped as a result.
419+
Restricting the substitution to base tokens matching
420+
`/^imm(?:8|16|32|64)$/` fixed both bugs and recovered all six
421+
previously-dropped mnemonics.
364422

365423
## Integration with `nasm-t.py`
366424

@@ -399,10 +457,10 @@ The generator prints a coverage summary at the end of each run
399457
unsupported operand-type tokens), so gaps are self-documenting. As of
400458
the last full run:
401459

402-
- **2606 / 2622 mnemonics generate at least one assemblable template**
460+
- **2612 / 2622 mnemonics generate at least one assemblable template**
403461
(includes all concrete expansions of the `cc`/`scc` condition-code
404462
families — see above).
405-
- **16 mnemonics produce zero output and are dropped**, mostly:
463+
- **10 mnemonics produce zero output and are dropped**, mostly:
406464
- `HINT_NOP0` .. `HINT_NOP63` placeholder mnemonics,
407465
- `LOADALL` / `LOADALL286`,
408466
- a handful of exotic AMX-transpose / APX instructions whose full
@@ -427,6 +485,11 @@ the last full run:
427485
*first* size-carrying memory operand in a template; templates with
428486
multiple independently-sizable memory operands aren't exhaustively
429487
covered.
488+
- Error-case coverage only exercises the two bit-width-incompatibility
489+
patterns the generator already tracks for other reasons (`needs64`
490+
and `avoid64` lines); it doesn't target other classes of assembly
491+
errors (e.g. invalid immediate ranges, disallowed operand
492+
combinations not tied to bit width, EVEX-decorator misuse).
430493
- ~70+ distinct operand-type tokens have generator support (see the
431494
`%fixed`/`%gen` tables at the top of the script); any new/renamed
432495
token introduced by a future `insns.dat` change that isn't in those
@@ -473,3 +536,17 @@ change these counts (1922 mnemonics gained at least one implicit-size
473536
line, verified via the same per-mnemonic `.json` entry-count
474537
comparison, identical 6955/6955). `make -j32 travis` continues to pass
475538
in ~26s.
539+
540+
Adding `%ifdef ERROR` error-case coverage — together with the
541+
branch-operand bug fixes it surfaced (see above) — changed the
542+
mnemonic-level counts for the first time since the initial hireg/apxreg
543+
update: 2612/2622 mnemonics now generate (up from 2606, since
544+
LOOP/LOOPE/LOOPNE/LOOPNZ/LOOPZ/JCXZ are no longer silently dropped),
545+
verified by diffing per-mnemonic *non-error* `.json` entry counts
546+
between a scratch regeneration and the previously-committed tree
547+
(identical aside from the six newly-recovered mnemonics — confirming
548+
no regressions in existing coverage). 1976/2612 mnemonics gained at
549+
least one error-case entry (3951 error json entries total). Full
550+
`nasm-t.py run` on the scratch tree: 10918/10918 PASS, 0 FAIL.
551+
`make -j32 travis` on the regenerated `travis/insns/` continues to pass
552+
in ~27s.

0 commit comments

Comments
 (0)