diff mbox series

[2/2] bpf: remove symbols created during failed parse

Message ID 20231114175805.7783-3-david.faust@oracle.com
State Unresolved
Headers
Series gas,bpf: cleanup bad symbols created while parsing |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

David Faust Nov. 14, 2023, 5:58 p.m. UTC 
  Parsing the BPF pseudo-c asm syntax requires attempting to parse an
instruction using a template that may later be determined to not match.
During this parsing, a call to expression () may end up creating one or
more symbols.  If the parsed instruction is later determined to not
match the template, then any symbols created during this process should
be discarded.

If such unused symbols are not discarded, they impede the loading of the
resulting BPF object by the Linux kernel.

gas/

	* config/tc-bpf.c (last_parsed_expr, old_symbol_lastP): New.
	(parse_expression): Track last_parsed_expr and old_symbol_lastP.
	(parse_error): Cleanup symbols created during a failed parse.
	* testsuite/gas/bpf/asm-extra-sym-1.d: New.
	* testsuite/gas/bpf/asm-extra-sym-1.s: New.
	* testsuite/gas/bpf/asm-extra-sym-2.d: New.
	* testsuite/gas/bpf/asm-extra-sym-2.s: New.
	* testsuite/gas/bpf/bpf.exp: Run new tests.
---
 gas/config/tc-bpf.c                     | 30 +++++++++++++++++++++++++
 gas/testsuite/gas/bpf/asm-extra-sym-1.d |  7 ++++++
 gas/testsuite/gas/bpf/asm-extra-sym-1.s |  1 +
 gas/testsuite/gas/bpf/asm-extra-sym-2.d |  7 ++++++
 gas/testsuite/gas/bpf/asm-extra-sym-2.s |  8 +++++++
 gas/testsuite/gas/bpf/bpf.exp           |  4 ++++
 6 files changed, 57 insertions(+)
 create mode 100644 gas/testsuite/gas/bpf/asm-extra-sym-1.d
 create mode 100644 gas/testsuite/gas/bpf/asm-extra-sym-1.s
 create mode 100644 gas/testsuite/gas/bpf/asm-extra-sym-2.d
 create mode 100644 gas/testsuite/gas/bpf/asm-extra-sym-2.s
diff mbox series

Patch

diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
index fd4144a354b..d64576415e1 100644
--- a/gas/config/tc-bpf.c
+++ b/gas/config/tc-bpf.c
@@ -1223,6 +1223,8 @@  add_relaxed_insn (struct bpf_insn *insn, expressionS *exp)
    See md_operand below to see how exp_parse_failed is used.  */
 
 static int exp_parse_failed = 0;
+static expressionS *last_parsed_expr = NULL;
+static symbolS *old_symbol_lastP = NULL;
 
 static char *
 parse_expression (char *s, expressionS *exp)
@@ -1232,10 +1234,13 @@  parse_expression (char *s, expressionS *exp)
 
   exp_parse_failed = 0;
   input_line_pointer = s;
+  old_symbol_lastP = symbol_lastP;
   expression (exp);
   s = input_line_pointer;
   input_line_pointer = saved_input_line_pointer;
 
+  last_parsed_expr = exp;
+
   switch (exp->X_op == O_absent || exp_parse_failed)
     return NULL;
 
@@ -1317,6 +1322,25 @@  parse_error (int length, const char *fmt, ...)
       va_end (args);
       partial_match_length = length;
     }
+
+  /* Cleanup any symbols created during the failed parsing.  */
+  if (last_parsed_expr
+      && (last_parsed_expr->X_add_symbol || last_parsed_expr->X_op_symbol))
+    {
+      /* NOTE: this logic exploits the implementation detail that a symbol
+	 created by expression () during parsing is appended to the list
+	 rather than potentially being inserted somewhere in the middle.  */
+      symbolS *sym = symbol_lastP;
+      while (sym != old_symbol_lastP)
+	{
+	  /* Must have created at least one symbol.  */
+	  symbol_remove (sym, &symbol_rootP, &symbol_lastP);
+	  symbol_table_remove (sym);
+	  sym = symbol_lastP;
+	}
+
+      old_symbol_lastP = symbol_lastP;
+    }
 }
 
 /* Assemble a machine instruction in STR and emit the frags/bytes it
@@ -1368,6 +1392,12 @@  md_assemble (char *str ATTRIBUTE_UNUSED)
       if (opcode->version > isa_spec)
         continue;
 
+      /* Track expression parsed while trying this opcode.  If this turns
+	 out to be the wrong opcode, we need to undo side effects of the
+	 expression parsing, such as creating a new undefined symbol.
+	 Set by parse_expression () and used by parse_error ().  */
+      last_parsed_expr = NULL;
+
       memset (&insn, 0, sizeof (struct bpf_insn));
       insn.size = 8;
       for (s = str, p = template; *p != '\0';)
diff --git a/gas/testsuite/gas/bpf/asm-extra-sym-1.d b/gas/testsuite/gas/bpf/asm-extra-sym-1.d
new file mode 100644
index 00000000000..56bdb7082f5
--- /dev/null
+++ b/gas/testsuite/gas/bpf/asm-extra-sym-1.d
@@ -0,0 +1,7 @@ 
+#as: -EL -mdialect=pseudoc
+#nm: --numeric-sort
+#source: asm-extra-sym-1.s
+#name: BPF pseudoc no extra symbols 1
+
+# Note: there should be no output from nm.
+# Previously a bug created an UND '*' symbol.
diff --git a/gas/testsuite/gas/bpf/asm-extra-sym-1.s b/gas/testsuite/gas/bpf/asm-extra-sym-1.s
new file mode 100644
index 00000000000..2cfa605a259
--- /dev/null
+++ b/gas/testsuite/gas/bpf/asm-extra-sym-1.s
@@ -0,0 +1 @@ 
+    r2 = *(u32*)(r1 + 8)
diff --git a/gas/testsuite/gas/bpf/asm-extra-sym-2.d b/gas/testsuite/gas/bpf/asm-extra-sym-2.d
new file mode 100644
index 00000000000..e17ae0f2422
--- /dev/null
+++ b/gas/testsuite/gas/bpf/asm-extra-sym-2.d
@@ -0,0 +1,7 @@ 
+#as: -EL -mdialect=pseudoc
+#nm: --numeric-sort
+#source: asm-extra-sym-2.s
+#name: BPF pseudoc no extra symbols 2
+
+[0-9a-f]+ t main
+[0-9a-f]+ t foo
diff --git a/gas/testsuite/gas/bpf/asm-extra-sym-2.s b/gas/testsuite/gas/bpf/asm-extra-sym-2.s
new file mode 100644
index 00000000000..ccbf43065d9
--- /dev/null
+++ b/gas/testsuite/gas/bpf/asm-extra-sym-2.s
@@ -0,0 +1,8 @@ 
+
+    .text
+main:
+    call foo
+    call foo
+foo:
+    r1 = 1
+    exit
diff --git a/gas/testsuite/gas/bpf/bpf.exp b/gas/testsuite/gas/bpf/bpf.exp
index 80f5a1dbc2d..680b8dbdb10 100644
--- a/gas/testsuite/gas/bpf/bpf.exp
+++ b/gas/testsuite/gas/bpf/bpf.exp
@@ -72,4 +72,8 @@  if {[istarget bpf*-*-*]} {
     run_dump_test disp16-overflow-relax
     run_dump_test disp32-overflow
     run_dump_test imm32-overflow
+
+    # Test that parser does not create undefined symbols
+    run_dump_test asm-extra-sym-1
+    run_dump_test asm-extra-sym-2
 }