[PATCH v3 1/6] bpf: diagnose instructions above the selected ISA spec
Jose E. Marchesi
jemarch@gnu.org
Mon Oct 12 11:42:20 GMT 2026
Hi Vineet.
This is OK for master.
Thanks!
> Using an instruction that exists but is not available in the selected ISA
> spec produces a misleading diagnostic:
>
> $ as -mdialect=pseudoc -misa-spec=v1 <<< 'gotol +1'
> Error: unrecognized instruction `gotol +1'
> Error: expected white space, got 'l +1'
>
> `gotol' is recognized; it just requires v4. The second line is noise: with
> no template matching, md_assemble reports the error of whichever template
> got furthest, which here is the unrelated `goto%w%d16'.
>
> md_assemble skips templates whose version exceeds isa_spec, so an
> instruction that is only spelled by such a template can never be told apart
> from a typo. Attempt those templates too, but only for the diagnostic: if
> one matches the whole statement, remember it and keep scanning, since a
> template within the ISA spec may still match. Only when nothing usable
> matches does the remembered one drive the error:
>
> Error: `gotol +1' requires ISA spec v4 or later
>
> Parse errors from these extra attempts are not recorded, or they would
> displace the diagnostics for genuinely unrecognized input. brkpt is worded
> separately, as xbpf is a superset of the BPF specs rather than a later
> version of them.
>
> gas/
> * config/tc-bpf.c (bpf_version_name): New function.
> (discard_deferred_symbols): New function, factored out of...
> (parse_error): ...here. Do not record a message while
> matching_gated_insn.
> (matching_gated_insn): New variable.
> (md_assemble): Attempt templates above the selected ISA spec and
> report the ISA spec they require when nothing else matches.
> * testsuite/gas/bpf/isa-spec-gated.s: New test.
> * testsuite/gas/bpf/isa-spec-gated.d: New test.
> * testsuite/gas/bpf/isa-spec-gated.l: New test.
> * testsuite/gas/bpf/isa-spec-gated-pseudoc.s: New test.
> * testsuite/gas/bpf/isa-spec-gated-pseudoc.d: New test.
> * testsuite/gas/bpf/isa-spec-gated-pseudoc.l: New test.
> * testsuite/gas/bpf/bpf.exp: Run them.
>
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
> ---
> gas/config/tc-bpf.c | 92 ++++++++++++++++---
> gas/testsuite/gas/bpf/bpf.exp | 2 +
> .../gas/bpf/isa-spec-gated-pseudoc.d | 4 +
> .../gas/bpf/isa-spec-gated-pseudoc.l | 5 +
> .../gas/bpf/isa-spec-gated-pseudoc.s | 8 ++
> gas/testsuite/gas/bpf/isa-spec-gated.d | 4 +
> gas/testsuite/gas/bpf/isa-spec-gated.l | 5 +
> gas/testsuite/gas/bpf/isa-spec-gated.s | 7 ++
> 8 files changed, 112 insertions(+), 15 deletions(-)
> create mode 100644 gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.d
> create mode 100644 gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.l
> create mode 100644 gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.s
> create mode 100644 gas/testsuite/gas/bpf/isa-spec-gated.d
> create mode 100644 gas/testsuite/gas/bpf/isa-spec-gated.l
> create mode 100644 gas/testsuite/gas/bpf/isa-spec-gated.s
>
> diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
> index e2472546aa61..d0393472dde4 100644
> --- a/gas/config/tc-bpf.c
> +++ b/gas/config/tc-bpf.c
> @@ -160,6 +160,22 @@ static int do_relax = 1;
>
> static int isa_spec = BPF_V4;
>
> +/* Return the name of ISA spec VERSION, as spelled in -misa-spec=. */
> +
> +static const char *
> +bpf_version_name (int version)
> +{
> + switch (version)
> + {
> + case BPF_V1: return "v1";
> + case BPF_V2: return "v2";
> + case BPF_V3: return "v3";
> + case BPF_V4: return "v4";
> + case BPF_XBPF: return "xbpf";
> + default: return "unknown";
> + }
> +}
> +
> /* The assembler supports two different dialects: "normal" syntax and
> "pseudoc" syntax. The dialect to use can be configured using
> command-line options. */
> @@ -1401,15 +1417,36 @@ bpf_parse_name (const char *name, expressionS *exp, enum expr_mode mode)
> return true;
> }
>
> +/* Discard the symbols deferred by a parse that we are not committing to.
> + They may potentially be reused in the future from the orphan list. */
> +
> +static void
> +discard_deferred_symbols (void)
> +{
> + while (deferred_sym_rootP)
> + {
> + symbolS *sym = deferred_sym_rootP;
> + symbol_remove (sym, &deferred_sym_rootP, &deferred_sym_lastP);
> + symbol_append (sym, orphan_sym_lastP, &orphan_sym_rootP,
> + &orphan_sym_lastP);
> + }
> +}
> +
> /* Collect a parse error message. */
>
> static int partial_match_length = 0;
> static char *errmsg = NULL;
>
> +/* Set while attempting to match a template that is not available in the
> + selected ISA spec. Such templates are tried only to improve diagnostics,
> + so their parse errors must not compete with those of the templates that
> + are actually available. */
> +static bool matching_gated_insn = false;
> +
> static void
> parse_error (int length, const char *fmt, ...)
> {
> - if (length > partial_match_length)
> + if (!matching_gated_insn && length > partial_match_length)
> {
> va_list args;
>
> @@ -1420,15 +1457,7 @@ parse_error (int length, const char *fmt, ...)
> partial_match_length = length;
> }
>
> - /* Discard deferred symbols from the failed parse. They may potentially
> - be reused in the future from the orphan list. */
> - while (deferred_sym_rootP)
> - {
> - symbolS *sym = deferred_sym_rootP;
> - symbol_remove (sym, &deferred_sym_rootP, &deferred_sym_lastP);
> - symbol_append (sym, orphan_sym_lastP, &orphan_sym_rootP,
> - &orphan_sym_lastP);
> - }
> + discard_deferred_symbols ();
> }
>
> /* Assemble a machine instruction in STR and emit the frags/bytes it
> @@ -1460,11 +1489,13 @@ md_assemble (char *str ATTRIBUTE_UNUSED)
> unsigned int idx = 0;
> struct bpf_insn insn;
> const struct bpf_opcode *opcode;
> + const struct bpf_opcode *gated_opcode = NULL;
>
> /* Initialize the global diagnostic variables. See the parse_error
> function above. */
> partial_match_length = 0;
> errmsg = NULL;
> + matching_gated_insn = false;
> g_curr_insn_str = str;
>
> #define PARSE_ERROR(...) parse_error (s > str ? s - str : 0, __VA_ARGS__)
> @@ -1476,9 +1507,14 @@ md_assemble (char *str ATTRIBUTE_UNUSED)
> const char *template
> = (asm_dialect == DIALECT_PSEUDOC ? opcode->pseudoc : opcode->normal);
>
> - /* Do not try to match opcodes with a higher version than the
> - selected ISA spec. */
> - if (opcode->version > isa_spec)
> + /* Opcodes with a higher version than the selected ISA spec cannot be
> + used, but still try to match them: if one matches we can tell the
> + user which ISA spec the instruction needs, rather than leaving them
> + with a parse error for whatever unrelated template happened to get
> + furthest. Remember the first such match in GATED_OPCODE and keep
> + looking for a usable template. */
> + matching_gated_insn = (opcode->version > isa_spec);
> + if (matching_gated_insn && gated_opcode != NULL)
> continue;
>
> memset (&insn, 0, sizeof (struct bpf_insn));
> @@ -1722,16 +1758,42 @@ md_assemble (char *str ATTRIBUTE_UNUSED)
> while (is_whitespace (*s))
> s += 1;
> if (is_end_of_stmt (*s))
> - /* We parsed an instruction successfully. */
> - break;
> + {
> + if (!matching_gated_insn)
> + /* We parsed an instruction successfully. */
> + break;
> +
> + /* The instruction exists, but not in the selected ISA spec.
> + Remember it for the diagnostic below and carry on: some
> + other template may still match within the ISA spec. */
> + gated_opcode = opcode;
> + discard_deferred_symbols ();
> + continue;
> + }
> PARSE_ERROR ("extra junk at end of line");
> }
> }
>
> + matching_gated_insn = false;
> +
> /* Mark that we are no longer parsing an instruction, bpf_parse_name does
> not interfere with symbols in e.g. assembler directives. */
> parsing_insn_operands = false;
>
> + if (opcode == NULL && gated_opcode != NULL)
> + {
> + /* XBPF is a superset of the BPF specs rather than a later version of
> + them, so `or later' would be misleading for its own instructions. */
> + if (gated_opcode->version == BPF_XBPF)
> + as_bad (_("`%s' requires ISA spec xbpf"), str);
> + else
> + as_bad (_("`%s' requires ISA spec %s or later"),
> + str, bpf_version_name (gated_opcode->version));
> + free (errmsg);
> + errmsg = NULL;
> + return;
> + }
> +
> if (opcode == NULL)
> {
> as_bad (_("unrecognized instruction `%s'"), str);
> diff --git a/gas/testsuite/gas/bpf/bpf.exp b/gas/testsuite/gas/bpf/bpf.exp
> index 9884b36678c2..055d957e916f 100644
> --- a/gas/testsuite/gas/bpf/bpf.exp
> +++ b/gas/testsuite/gas/bpf/bpf.exp
> @@ -40,6 +40,8 @@ if {[istarget bpf*-*-*]} {
> run_dump_test jump32-pseudoc
> run_dump_test jump-gotol-signed-pseudoc
> run_dump_test jump-gotol-cond-pseudoc
> + run_dump_test isa-spec-gated
> + run_dump_test isa-spec-gated-pseudoc
> run_dump_test atomic-v1
> run_dump_test atomic
> run_dump_test atomic-pseudoc
> diff --git a/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.d b/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.d
> new file mode 100644
> index 000000000000..7f2bb3eb32b9
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.d
> @@ -0,0 +1,4 @@
> +#as: -EL -mdialect=pseudoc -misa-spec=v1
> +#source: isa-spec-gated-pseudoc.s
> +#error_output: isa-spec-gated-pseudoc.l
> +#name: eBPF instructions above the selected ISA spec, pseudoc syntax
> diff --git a/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.l b/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.l
> new file mode 100644
> index 000000000000..0e9b93f6d788
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.l
> @@ -0,0 +1,5 @@
> +.*: Assembler messages:
> +.*:5: Error: `gotol \+1' requires ISA spec v4 or later
> +.*:6: Error: `r1 =bswap16 r1' requires ISA spec v4 or later
> +.*:7: Error: `r1 s/=r2' requires ISA spec v4 or later
> +.*:8: Error: `if w1==w2 goto 1' requires ISA spec v3 or later
> diff --git a/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.s b/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.s
> new file mode 100644
> index 000000000000..c17aa0678d59
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/isa-spec-gated-pseudoc.s
> @@ -0,0 +1,8 @@
> + # Likewise for the pseudo-C dialect. PR gas/34558: `gotol' below v4
> + # used to assemble silently as `goto' plus a relocation against an
> + # undefined symbol `l'.
> + .text
> + gotol +1
> + r1 = bswap16 r1
> + r1 s/= r2
> + if w1 == w2 goto 1
> diff --git a/gas/testsuite/gas/bpf/isa-spec-gated.d b/gas/testsuite/gas/bpf/isa-spec-gated.d
> new file mode 100644
> index 000000000000..137e868bae99
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/isa-spec-gated.d
> @@ -0,0 +1,4 @@
> +#as: -EL -misa-spec=v1
> +#source: isa-spec-gated.s
> +#error_output: isa-spec-gated.l
> +#name: eBPF instructions above the selected ISA spec, normal syntax
> diff --git a/gas/testsuite/gas/bpf/isa-spec-gated.l b/gas/testsuite/gas/bpf/isa-spec-gated.l
> new file mode 100644
> index 000000000000..5a65d79ff998
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/isa-spec-gated.l
> @@ -0,0 +1,5 @@
> +.*: Assembler messages:
> +.*:4: Error: `jal 1' requires ISA spec v4 or later
> +.*:5: Error: `movs %r1,%r2,8' requires ISA spec v4 or later
> +.*:6: Error: `aadd \[%r1\+8\],%r2' requires ISA spec v3 or later
> +.*:7: Error: `jeq32 %r1,%r2,1' requires ISA spec v3 or later
> diff --git a/gas/testsuite/gas/bpf/isa-spec-gated.s b/gas/testsuite/gas/bpf/isa-spec-gated.s
> new file mode 100644
> index 000000000000..acc17dea3341
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/isa-spec-gated.s
> @@ -0,0 +1,7 @@
> + # Instructions that exist, but not in the selected ISA spec, must be
> + # diagnosed as such rather than as a generic parse failure.
> + .text
> + jal 1
> + movs %r1, %r2, 8
> + aadd [%r1+8], %r2
> + jeq32 %r1, %r2, 1
More information about the bpf
mailing list