[PATCH] PR 34558: bpf: don't mis-assemble `gotol' with signed offset
Jose E. Marchesi
jemarch@gnu.org
Mon Aug 24 13:46:13 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.
Expanding on this.
The usual assembly-like syntax allows using trivial tokenization with
fixed templates like in "ja%W%d16", in which one or more spaces separate
the opcode (mnemonic) from the assembler expression. This expression
could be very well something like 'ja+2' where 'ja' is a symbol, as in
virtually all assembly languages, there are no reserved words. This is
easy and efficient to implement because the mnemonics occupy a fixed
position in each line: it is this position that identifies them as
mnemonics.
The BPF pseudo-C assembly syntax tries to look like C, but I don't think
it is reasonable for an assembly language to require context-free
tokenisation. I think it is reasonable for BPF to mandate at least one
whitespace character between "goto" and "gotol" and their argument.
Does llvm generate instructions like goto10?
>>
>> 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