testgen: add %ifdef ERROR error-case coverage, fix branch-operand bugs

Add error-case ("negative test") coverage per the user's preferred
convention: rather than a separate source file, error-triggering
instruction lines are appended to the existing per-mnemonic .asm file
under a %ifdef ERROR guard, and the harness assembles the same file
twice -- once without -DERROR (existing positive-path coverage,
unaffected) and once with -DERROR (expected to fail, per nasm-t.py's
"error": "expected" json convention, matching the pre-existing
travis/ret/ret.json pattern).

The error material comes for free from lines the generator already
knows are bit-width-incompatible:

- Lines needing 64-bit encodings (reg64/imm64 operands, hireg r8-r15,
  apxreg r16-r31, etc. -- %needs64_token / build_variant_line) are, by
  construction, exactly the lines already excluded from the 16/32-bit
  "narrow" body. They're appended to the narrow file under %ifdef
  ERROR and probed at --bits 16/32 with -DERROR; only widths where the
  block actually fails become json entries.

- Symmetrically, CALL/JMP near-indirect targets via rm16/rm32 (only
  rm64 is valid in 64-bit mode -- confirmed empirically, matches the
  NOLONG flag on those insns.xda templates) are appended to the full
  (64-bit) body under %ifdef ERROR and probed at --bits 64 with
  -DERROR.

Each candidate block is probed before being turned into a json entry,
so a line that unexpectedly *does* assemble at some width (this
generator doesn't model every mode restriction) doesn't turn into a
bogus "expected error" test; a mnemonic whose *only* surviving
coverage would be error entries is also rejected (see below), since
"this never assembles" isn't meaningful regression coverage on its
own.

While wiring this up, discovered and fixed two related bugs in the
existing branch-mnemonic handling (gen_operand()'s is_branch
substitution):

1. is_branch replaced *every* operand of a branch mnemonic with the
   ".L1" local-label text, not just genuine relative/near/short/abs
   branch-displacement operands. This produced nonsensical lines like
   "loop .L1, .L1" (LOOP's address-size-override form takes a fixed
   "cx"/"ecx"/"rcx" second operand, not a branch target) and "call
   .L1" for JMP/CALL's indirect (rm16/32/64) and far-pointer
   (imm16:imm16) forms instead of an actual register/memory operand.
   These bogus lines silently poisoned assembly for the whole
   mnemonic, and LOOP/LOOPE/LOOPNE/LOOPNZ/LOOPZ/JCXZ were silently
   dropped entirely as a result (present in the "16 dropped" list).
   Restricting the substitution to base tokens matching
   /^imm(?:8|16|32|64)$/ fixes both LOOP's operand and JMP/CALL's
   indirect/far forms, and recovers all six previously-dropped
   mnemonics with correct coverage.

2. Once (1) exposed genuine rm16/rm32 operand generation for CALL/JMP,
   a new bit-width interaction appeared: rm16/rm32 near-indirect
   targets are only valid in 16/32-bit mode (unlike ordinary reg16/32
   operands elsewhere, which work at any bit width), so a line built
   from one now broke 64-bit assembly for the whole mnemonic the same
   way a needs64 line breaks 16/32-bit assembly. Added
   branch_narrow_only() and a parallel avoid64 line flag (mirroring
   needs64) to exclude these lines from the 64-bit "full" body -- this
   is also what feeds the new symmetric 64-bit error-case coverage
   described above.

Also added a safety-net to the existing "couldn't assemble in any
mode, drop the directory" check: a directory is now only kept if it
has at least one *non*-error json entry, preventing a future bug
symmetric to (1) from silently producing a directory whose only
content is error-case entries.

Verified via full scratch regeneration (2612 mnemonics generated / 10
dropped, up from 2606/16 thanks to the LOOP-family/JCXZ fix) +
nasm-t.py run (10918/10918 PASS, 0 FAIL) + per-mnemonic non-error
.json entry-count diff against the prior committed tree (identical
except the 6 newly-recovered mnemonics, confirming no regressions).
1976/2612 mnemonics gained at least one error-case entry (3951 error
json entries total). Regenerated travis/insns/ and validated via
'make -j32 travis' (all PASS, ~27s).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: H. Peter Anvin (Intel)

tools/testgen/gen-insn-tests.pl

@@ -404,7 +404,17 @@ sub base_token {
 sub gen_operand {
     my ($rng, $tok, $mnem, $is_branch, $variant) = @_;
     my $base = base_token($tok);
-    if ($is_branch) {
+    # Only genuine relative/near/short/abs branch-displacement operands
+    # (plain imm8/16/32/64, before the trailing "|near"/"|short"/"|abs"
+    # qualifier that base_token() already stripped) get replaced with a
+    # local label -- NOT every operand of a "branch mnemonic". Some
+    # branch mnemonics also have indirect (rm16/32/64), far-pointer
+    # (colon-compound "imm16:imm16"), or fixed-register (reg_cx, for
+    # LOOP's address-size-override form) operand forms, none of which
+    # are relative-displacement targets, and would otherwise get
+    # nonsensically replaced with the branch label too (producing
+    # invalid syntax like "loop .L1, .L1" instead of "loop .L1, cx").
+    if ($is_branch && $base =~ /^imm(?:8|16|32|64)$/) {
         return '.L1';
     }
     if (exists $fixed{$base}) {
@@ -417,6 +427,21 @@ sub gen_operand {
     return undef;
 }
 
+# CALL/JMP near-indirect targets (the only branch-mnemonic templates
+# with rm*-typed operands) restrict the operand-size override: rm64 is
+# the only form valid in 64-bit mode -- rm16/rm32 forms genuinely only
+# assemble in 16/32-bit mode (confirmed empirically: "call cx"/
+# "call ecx" both fail under --bits 64, matching the NOLONG flag on
+# those insns.xda templates). This is unrelated to %needs64_token
+# (which flags the opposite direction -- 64-bit-*only* operands), so a
+# separate helper marks lines built from these tokens as excluded from
+# the 64-bit "full" body, mirroring how needs64 lines are excluded from
+# the 16/32-bit "narrow" body.
+sub branch_narrow_only {
+    my ($is_branch, $base) = @_;
+    return ($is_branch && $base =~ /^rm(?:16|32)$/) ? 1 : 0;
+}
+
 # Base tokens whose register pool has hireg (r8-r15 range) / apxreg
 # (r16-r31 range) tiers available -- i.e. templates containing at
 # least one of these are candidates for the extra register-number-
@@ -632,6 +657,7 @@ sub build_dispboundary_line {
     my @operands;
     my $used_mem = 0;
     my $needs64 = 0;
+    my $avoid64 = 0;
     for my $tok (@$ops) {
         my $base = base_token($tok);
         if (!$used_mem && exists $mem_sizebits{$base}) {
@@ -640,15 +666,17 @@ sub build_dispboundary_line {
             $txt = "$memsize{$sizebits} $txt" if $sizebits && $memsize{$sizebits};
             push @operands, $txt;
             $used_mem = 1;
+            $avoid64 = 1 if branch_narrow_only($is_branch, $base);
             next;
         }
         my $val = gen_operand($rng, $tok, $mnem, $is_branch);
         return undef unless defined $val;
         $needs64 = 1 if $needs64_token{$base};
+        $avoid64 = 1 if branch_narrow_only($is_branch, $base);
         push @operands, $val;
     }
     return undef unless $used_mem;
-    return { text => lc($mnem) . ' ' . join(', ', @operands), needs64 => $needs64 };
+    return { text => lc($mnem) . ' ' . join(', ', @operands), needs64 => $needs64, avoid64 => $avoid64 };
 }
 
 # Implicitly-sized memory operand coverage.
@@ -677,20 +705,23 @@ sub build_implicitsize_line {
     my @operands;
     my $used_mem = 0;
     my $needs64 = 0;
+    my $avoid64 = 0;
     for my $tok (@$ops) {
         my $base = base_token($tok);
         if (!$used_mem && ($mem_sizebits{$base} // 0)) {
             push @operands, mem_operand($rng, 0);
             $used_mem = 1;
+            $avoid64 = 1 if branch_narrow_only($is_branch, $base);
             next;
         }
         my $val = gen_operand($rng, $tok, $mnem, $is_branch);
         return undef unless defined $val;
         $needs64 = 1 if $needs64_token{$base};
+        $avoid64 = 1 if branch_narrow_only($is_branch, $base);
         push @operands, $val;
     }
     return undef unless $used_mem;
-    return { text => lc($mnem) . ' ' . join(', ', @operands), needs64 => $needs64 };
+    return { text => lc($mnem) . ' ' . join(', ', @operands), needs64 => $needs64, avoid64 => $avoid64 };
 }
 
 sub make_rng {
@@ -768,16 +799,20 @@ sub render_body {
             my @operands;
             my $ok = 1;
             my $needs64 = 0;
+            my $avoid64 = 0;
             for my $tok (@{ $t->{ops} }) {
                 my $val = gen_operand($rng, $tok, $mnem, $is_branch);
                 if (!defined $val) { $ok = 0; last; }
-                $needs64 = 1 if $needs64_token{ base_token($tok) };
+                my $base = base_token($tok);
+                $needs64 = 1 if $needs64_token{$base};
+                $avoid64 = 1 if branch_narrow_only($is_branch, $base);
                 push @operands, $val;
             }
             next unless $ok;
             push @lines, {
                 text    => lc($mnem) . (@operands ? ' ' . join(', ', @operands) : ''),
                 needs64 => $needs64,
+                avoid64 => $avoid64,
             };
 
             # Optional-operand ('*'/'?') coverage: also emit the
@@ -791,9 +826,11 @@ sub render_body {
                 push @lines, {
                     text    => lc($mnem) . (@reduced ? ' ' . join(', ', @reduced) : ''),
                     needs64 => $needs64,
+                    avoid64 => $avoid64,
                 };
             }
 
+
             # Register-number coverage: once per template (not once per
             # --variants instance), also try building an all-hireg
             # (r8-r15/xmm8-15/...) and an all-apxreg (r16-r31/xmm16-31)
@@ -848,7 +885,17 @@ sub render_body {
     for my $t (@dedup_all) {
         for my $disp (1, 64) {
             my $dl = build_dispboundary_line($rng, $mnem, $t->{ops}, $is_branch, $disp);
-            push @extra_dispboundary, $dl if defined $dl;
+            # avoid64 candidates (CALL/JMP rm16/rm32 near-indirect
+            # targets -- see branch_narrow_only()) are dropped here
+            # rather than fed into the bits=64 probe: they're
+            # guaranteed to fail at 64-bit, and folding them into this
+            # bucket would either cost the whole bucket its otherwise-
+            # valid candidates (probe rejects the merge) or need a
+            # second parallel probe path for no real benefit, since
+            # this supplementary boundary coverage isn't the base
+            # per-mnemonic content the 64-bit error-case block (below)
+            # is built from.
+            push @extra_dispboundary, $dl if defined $dl && !$dl->{avoid64};
         }
     }
 
@@ -861,12 +908,21 @@ sub render_body {
     # which this generator doesn't parse directly (see comment above).
     for my $t (@dedup_all) {
         my $dl = build_implicitsize_line($rng, $mnem, $t->{ops}, $is_branch);
-        push @extra_implicitsize, $dl if defined $dl;
+        push @extra_implicitsize, $dl if defined $dl && !$dl->{avoid64};
     }
 
+
     next unless @lines;   # nothing we knew how to generate
 
     my @narrow_lines = grep { !$_->{needs64} } @lines;
+    # Lines that only assemble in 16/32-bit mode (currently just
+    # CALL/JMP rm16/rm32 near-indirect targets -- branch_narrow_only())
+    # must symmetrically be excluded from the 64-bit "full" body: they
+    # aren't flagged needs64 (they don't require 64-bit -- the opposite
+    # holds, they're incompatible with it), so they'd otherwise poison
+    # bits=64 assembly for the *entire* file the same way a needs64
+    # line would poison bits=16/32.
+    my @lines64 = grep { !$_->{avoid64} } @lines;
 
     my $dirname = "$out_dir/" . lc($mnem);
     make_path($dirname);
@@ -923,7 +979,7 @@ sub render_body {
         return $ok;
     };
 
-    my @full_lines = @lines;
+    my @full_lines = @lines64;
     for my $cat (\@extra_hireg, \@extra_apxreg, \@extra_mask, \@extra_maskz,
                  \@extra_broadcast, \@extra_saeer, \@extra_dispboundary,
                  \@extra_implicitsize) {
@@ -945,6 +1001,47 @@ sub render_body {
         close($nfh);
     }
 
+    # Error-case coverage: lines that only assemble in 64-bit mode
+    # (reg64/imm64 operands, hireg r8-r15, apxreg r16-r31, etc. -- see
+    # %needs64_token / build_variant_line) are, by construction, exactly
+    # the lines excluded from the 16/32-bit "narrow" body above. Rather
+    # than inventing a separate error-only source file, append them to
+    # the *same* narrow file under a %ifdef ERROR guard (per the user's
+    # preferred convention: one .asm file, assembled twice -- with and
+    # without -DERROR). Without -DERROR the block is invisible and
+    # narrow-mode coverage is unaffected; with -DERROR the block is
+    # included and attempting to assemble it at --bits 16/32 should
+    # fail with a mode/operand-mismatch diagnostic, which is exactly
+    # the negative-test coverage we want. Each bit width is probed
+    # independently below and only kept if it actually fails, so a line
+    # that unexpectedly *does* assemble at some width (this generator
+    # doesn't model every mode restriction) doesn't turn into a bogus
+    # "expected error" test.
+    my @error_lines = grep { $_->{needs64} } @full_lines;
+    my $has_narrow_file = ($asmfile_narrow ne $asmfile_full);
+    if ($has_narrow_file && @error_lines) {
+        open(my $efh, '>>', "$dirname/$asmfile_narrow") or die $!;
+        print $efh "\n%ifdef ERROR\n";
+        print $efh join('', map { "\t$_->{text}\n" } @error_lines);
+        print $efh "%endif\n";
+        close($efh);
+    }
+
+    # Symmetric case: lines that only assemble in 16/32-bit mode
+    # (CALL/JMP rm16/rm32 near-indirect targets -- branch_narrow_only())
+    # are excluded from @full_lines above; append them to the *full*
+    # (64-bit) body under the same %ifdef ERROR convention, so
+    # attempting to assemble them at --bits 64 with -DERROR is
+    # exercised as an expected-error case too.
+    my @error64_lines = grep { $_->{avoid64} } @lines;
+    if (@error64_lines) {
+        open(my $e6fh, '>>', "$dirname/$asmfile_full") or die $!;
+        print $e6fh "\n%ifdef ERROR\n";
+        print $e6fh join('', map { "\t$_->{text}\n" } @error64_lines);
+        print $e6fh "%endif\n";
+        close($e6fh);
+    }
+
     my @json_entries;
     for my $bits (16, 32, 64) {
         my $asmfile = ($bits == 64 || !@narrow_lines) ? $asmfile_full : $asmfile_narrow;
@@ -979,9 +1076,47 @@ sub render_body {
         }
     }
 
-    if (!@json_entries) {
+    # Error-case coverage: probe the %ifdef ERROR block(s) appended
+    # above with -DERROR defined, and only turn each into a json
+    # "expected error" entry for the (source file, bit width) pairs
+    # where it actually fails to assemble.
+    my $probe_error_bits = sub {
+        my ($src, @bitlist) = @_;
+        for my $bits (@bitlist) {
+            my $errbin = lc($mnem) . ".errprobe$bits.bin";
+            my $errstderr = lc($mnem) . ".stderr${bits}err";
+            my $relsrc = "$dirref/$src";
+            my $cmd = sprintf('%s --bits %d -DERROR -f bin %s %s -o %s.tmp 2>%s',
+                               $nasm_bin, $bits, $warn_opts, $relsrc,
+                               "$dirname/$errbin", "$dirname/$errstderr");
+            system($cmd);
+            my $rc = $? >> 8;
+            unlink("$dirname/$errbin.tmp");
+            my $fails = ($rc != 0);
+            unlink("$dirname/$errstderr");
+            if ($fails) {
+                push @json_entries, {
+                    id       => lc($mnem) . $bits . 'err',
+                    bits     => $bits,
+                    source   => $src,
+                    opts     => "-DERROR $warn_opts",
+                    stderr   => $errstderr,
+                    is_error => 1,
+                };
+            }
+        }
+    };
+    $probe_error_bits->($asmfile_narrow, 16, 32) if $has_narrow_file && @error_lines;
+    $probe_error_bits->($asmfile_full, 64) if @error64_lines;
+
+    if (!grep { !$_->{is_error} } @json_entries) {
         # Couldn't assemble in any mode -- drop the whole directory,
-        # don't leave a dangling/empty test case behind.
+        # don't leave a dangling/empty test case behind. (Error-case
+        # entries alone don't count: a directory whose only "coverage"
+        # is "this asm never assembles" isn't meaningful regression
+        # coverage and would only mask a real generation bug -- see
+        # the is_branch operand-substitution fix above, discovered via
+        # exactly this scenario.)
         unlink("$dirname/$asmfile_full");
         unlink("$dirname/$asmfile_narrow") if $asmfile_narrow ne $asmfile_full;
         rmdir($dirname);
@@ -995,6 +1130,22 @@ sub render_body {
     for my $e (@json_entries) {
         print $jf ",\n" unless $first;
         $first = 0;
+        if ($e->{is_error}) {
+            printf $jf <<"JSON", $mnem, $e->{id}, $e->{source}, $e->{bits}, $e->{opts}, $dirname, $e->{stderr};
+ {
+  "description": "Pseudorandom error-case test for %s",
+  "id": "%s",
+  "format": "bin",
+  "source": "%s",
+  "option": "--bits %d %s -I./%s/",
+  "target": [
+   { "stderr": "%s" }
+  ],
+  "error": "expected"
+ }
+JSON
+            next;
+        }
         my $stderr_target = $e->{stderr}
             ? qq(,\n   { "stderr": "$e->{stderr}" })
             : '';

travis/insns/aadd/aadd.json

@@ -9,4 +9,28 @@
    { "output": "aadd.bin64", "match": "aadd.bin64.t" }
   ]
  }
+,
+ {
+  "description": "Pseudorandom error-case test for AADD",
+  "id": "aadd16err",
+  "format": "bin",
+  "source": "aadd_narrow.asm",
+  "option": "--bits 16 -DERROR -w-ea-absolute -w-implicit-abs-deprecated -I./travis/insns/aadd/",
+  "target": [
+   { "stderr": "aadd.stderr16err" }
+  ],
+  "error": "expected"
+ }
+,
+ {
+  "description": "Pseudorandom error-case test for AADD",
+  "id": "aadd32err",
+  "format": "bin",
+  "source": "aadd_narrow.asm",
+  "option": "--bits 32 -DERROR -w-ea-absolute -w-implicit-abs-deprecated -I./travis/insns/aadd/",
+  "target": [
+   { "stderr": "aadd.stderr32err" }
+  ],
+  "error": "expected"
+ }
 ]

travis/insns/aadd/aadd.stderr16err

@@ -0,0 +1,11 @@
+./travis/insns/aadd/aadd_narrow.asm:1: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:2: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:5: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:6: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:7: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:8: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:9: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:10: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:11: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:12: error: instruction not supported in 16-bit mode
+./travis/insns/aadd/aadd_narrow.asm:13: error: instruction not supported in 16-bit mode

travis/insns/aadd/aadd.stderr32err

@@ -0,0 +1,11 @@
+./travis/insns/aadd/aadd_narrow.asm:1: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:2: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:5: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:6: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:7: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:8: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:9: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:10: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:11: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:12: error: instruction not supported in 32-bit mode
+./travis/insns/aadd/aadd_narrow.asm:13: error: instruction not supported in 32-bit mode

travis/insns/aadd/aadd_narrow.asm

@@ -1,2 +1,14 @@
 	aadd dword [0xe1b], ebx
 	aadd dword [0x7dc], ecx
+
+%ifdef ERROR
+	aadd qword [0xeca], rcx
+	aadd qword [0x4e4], rcx
+	aadd dword [0xbc9], r13d
+	aadd qword [0xa2a], r10
+	aadd dword [0xcad], r16d
+	aadd qword [0x5c3], r17
+	aadd qword [eax+1], rbx
+	aadd qword [eax+64], rcx
+	aadd [0x5d8], rdi
+%endif

travis/insns/aand/aand.json

@@ -9,4 +9,28 @@
    { "output": "aand.bin64", "match": "aand.bin64.t" }
   ]
  }
+,
+ {
+  "description": "Pseudorandom error-case test for AAND",
+  "id": "aand16err",
+  "format": "bin",
+  "source": "aand_narrow.asm",
+  "option": "--bits 16 -DERROR -w-ea-absolute -w-implicit-abs-deprecated -I./travis/insns/aand/",
+  "target": [
+   { "stderr": "aand.stderr16err" }
+  ],
+  "error": "expected"
+ }
+,
+ {
+  "description": "Pseudorandom error-case test for AAND",
+  "id": "aand32err",
+  "format": "bin",
+  "source": "aand_narrow.asm",
+  "option": "--bits 32 -DERROR -w-ea-absolute -w-implicit-abs-deprecated -I./travis/insns/aand/",
+  "target": [
+   { "stderr": "aand.stderr32err" }
+  ],
+  "error": "expected"
+ }
 ]