[PATCH] PR 34558: bpf: don't mis-assemble `gotol' with signed offset

[Vineet Gupta]

  `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'.

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
-- 
2.53.0-Meta