[PATCH] PR 34558: bpf: don't mis-assemble `gotol' with signed offset
Jose E. Marchesi
jemarch@gnu.org
Sun Aug 23 16:31:59 GMT 2026
Hi Vineet, thanks for the patch.
> `gotol +1' is assembled as if it were `goto l +1'
>
> `gotol +imm' is the canonical form in the BPF instruction set
> documentation, and it is what LLVM emits and accepts, so it needs to work.
>
> The reason is the asm templates for the two unconditional jumps:
>
> BPF_INSN_JAR "goto%w%d16"
> BPF_INSN_JAL "gotol%w%d32"
>
> where %w matches zero or more whitespace characters.
>
> JAR sorts before JAL in the opcode table so it tries to match first and
> succeeds as `goto' and `l +1', with `l' being an undefined symbol. This
> ends up as JA (opcode 0x05, with the displacement in the 16-bit `off'
> field) plus an R_BPF_GNU_64_16 relocation against an undefined symbol `l',
> rather than as JAL (opcode 0x06, displacement in the 32-bit `imm' field).
> No diagnostic is emitted. The signed form -1 is similarly affected. For
> non-signed forms, `gotol 1' or `gotol 1f', the remainder does not parse as
> a single expression, the JAR template fails, and JAL is reached and matched
> correctly. The normal dialect is not affected either, as `ja%W%d16'
> requires at least one whitespace character after the mnemonic.
>
> The fix is to reject a template whose literal text stops in the middle of a
> name: if the character last matched from the template is part of a name and
> the input continues with another name character, then the template has only
> matched a prefix of a longer mnemonic and does not apply. Other templates
> are then given a chance to match the whole mnemonic. This also covers the
> compound conditional jumps, whose templates embed `goto%w%d16'.
There are no "mnemonics" in pseudo-C syntax (not my idea).
BPF should remove the syntactic ambiguity (one of many) from the syntax
by mandating a whitespace after "goto" and "gotol" in these
instructions. Then you can use the existing tag %W in the templates.
>
> Existing coverage exercised `gotol' only with a label operand, which is why
> this went unnoticed.
>
> PR gas/34558
>
> gas/
> * config/tc-bpf.c (md_assemble): Do not let a template match when
> its literal text ends mid-name and the input continues with a name
> character.
> * testsuite/gas/bpf/jump-gotol-signed-pseudoc.s: New test.
> * testsuite/gas/bpf/jump-gotol-signed-pseudoc.d: New test.
> * testsuite/gas/bpf/bpf.exp: Run it.
> ---
> gas/config/tc-bpf.c | 23 ++++++++++++++++++-
> gas/testsuite/gas/bpf/bpf.exp | 1 +
> .../gas/bpf/jump-gotol-signed-pseudoc.d | 15 ++++++++++++
> .../gas/bpf/jump-gotol-signed-pseudoc.s | 10 ++++++++
> 4 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.d
> create mode 100644 gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.s
>
> diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
> index 8d48b128fb90..9b207c2e2389 100644
> --- a/gas/config/tc-bpf.c
> +++ b/gas/config/tc-bpf.c
> @@ -1506,7 +1506,28 @@ md_assemble (char *str ATTRIBUTE_UNUSED)
> }
> else if (*(p + 1) == 'w')
> {
> - /* Expect zero or more spaces. */
> + /* Expect zero or more spaces.
> +
> + If the template text matched so far ends in a name
> + character and the input continues with another name
> + character, then the template only matched a prefix of a
> + longer mnemonic written in the input, and this template
> + does not apply. Rejecting it here lets a subsequent
> + template have a go at the whole mnemonic.
> +
> + Without this, `gotol +1' matches the `goto%w%d16'
> + template, with `l +1' parsed as the branch offset
> + expression, silently assembling to JA (opcode 0x05) plus
> + a relocation against an undefined symbol `l' instead of
> + to JAL (opcode 0x06). */
> + if (!is_whitespace (*s)
> + && p > template
> + && is_part_of_name (*(p - 1))
> + && is_part_of_name (*s))
> + {
> + PARSE_ERROR ("expected white space, got '%s'", s);
> + break;
> + }
> while (is_whitespace (*s))
> s += 1;
> p += 2;
> diff --git a/gas/testsuite/gas/bpf/bpf.exp b/gas/testsuite/gas/bpf/bpf.exp
> index f19322122527..efd1eab08f72 100644
> --- a/gas/testsuite/gas/bpf/bpf.exp
> +++ b/gas/testsuite/gas/bpf/bpf.exp
> @@ -38,6 +38,7 @@ if {[istarget bpf*-*-*]} {
> run_dump_test jump-pseudoc
> run_dump_test jump32
> run_dump_test jump32-pseudoc
> + run_dump_test jump-gotol-signed-pseudoc
> run_dump_test atomic-v1
> run_dump_test atomic
> run_dump_test atomic-pseudoc
> diff --git a/gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.d b/gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.d
> new file mode 100644
> index 000000000000..ee4146acd996
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.d
> @@ -0,0 +1,15 @@
> +#as: -EL -mdialect=pseudoc
> +#objdump: -dr -M dec,pseudoc
> +#source: jump-gotol-signed-pseudoc.s
> +#name: eBPF gotol with signed offsets, pseudoc syntax
> +
> +.*: +file format .*bpf.*
> +
> +Disassembly of section .text:
> +
> +0+ <.text>:
> + 0: 06 00 00 00 01 00 00 00 gotol 1
> + 8: 06 00 00 00 ff ff ff ff gotol -1
> + 10: 06 00 00 00 01 00 00 00 gotol 1
> + 18: 06 00 00 00 00 00 00 00 gotol 0
> + 20: 06 00 00 00 00 00 00 00 gotol 0
> diff --git a/gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.s b/gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.s
> new file mode 100644
> index 000000000000..295afd6313d5
> --- /dev/null
> +++ b/gas/testsuite/gas/bpf/jump-gotol-signed-pseudoc.s
> @@ -0,0 +1,10 @@
> + # Signed branch offsets for the pseudo-C `gotol'. PR gas/34558:
> + # these used to match the shorter `goto' template, with the
> + # trailing `l' parsed as the start of the offset expression.
> + .text
> + gotol +1
> + gotol -1
> + gotol 1
> + gotol 1f
> +1:
> + gotol 0
More information about the Binutils
mailing list