[binutils-gdb] bpf: fix relocation addend incorrect symbol value

David Faust dfaust@sourceware.org
Fri Jan 12 17:32:19 GMT 2024


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ba7c1e37cfb498263a4e309700c27696fa5ff1ad

commit ba7c1e37cfb498263a4e309700c27696fa5ff1ad
Author: David Faust <david.faust@oracle.com>
Date:   Wed Jan 10 15:05:36 2024 -0800

    bpf: fix relocation addend incorrect symbol value
    
    Relocations installed by the BPF ELF backend were sometimes incorrectly
    adding the symbol value to the relocation entry addend, when the correct
    relocation value was already stored in the addend. This could lead to a
    relocation effectively adding the symbol value twice.
    
    Fix that by making bpf_elf_generic_reloc () more similar to the flow of
    bfd_install_relocation in the case where howto->install_addend is set,
    which is how it ought to behave.
    
    bfd/
            * bpf-reloc.def (R_BPF_64_ABS32, R_BPF_64_ABS64)
            (R_BPF_64_NODYLD32): Set partial_inplace to true.
            * elf64-bpf.c (bpf_elf_generic_reloc): Do not include the value
            of the symbol when installing relocation. Copy some additional
            logic from bfd_elf_generic_reloc.
    
    gas/
            * testsuite/gas/bpf/bpf.exp: Run new test.
            * testsuite/gas/bpf/elf-relo-1.d: New.
            * testsuite/gas/bpf/elf-relo-1.s: New.

Diff:
---
 bfd/bpf-reloc.def                  |  6 +++---
 bfd/elf64-bpf.c                    | 34 +++++++++++++++++++++++-----------
 gas/testsuite/gas/bpf/bpf.exp      |  3 +++
 gas/testsuite/gas/bpf/elf-relo-1.d | 35 +++++++++++++++++++++++++++++++++++
 gas/testsuite/gas/bpf/elf-relo-1.s | 25 +++++++++++++++++++++++++
 5 files changed, 89 insertions(+), 14 deletions(-)

diff --git a/bfd/bpf-reloc.def b/bfd/bpf-reloc.def
index 42ba1a169ea..65228d44009 100644
--- a/bfd/bpf-reloc.def
+++ b/bfd/bpf-reloc.def
@@ -38,7 +38,7 @@
 	 complain_overflow_bitfield, /* complain_on_overflow */
 	 bpf_elf_generic_reloc, /* special_function */
 	 "R_BPF_64_ABS32",	/* name */
-	 false,			/* partial_inplace */
+	 true,			/* partial_inplace */
 	 0xffffffff,		/* src_mask */
 	 0xffffffff,		/* dst_mask */
 	 true)			/* pcrel_offset */
@@ -53,7 +53,7 @@
 	 complain_overflow_bitfield, /* complain_on_overflow */
 	 bpf_elf_generic_reloc, /* special_function */
 	 "R_BPF_64_ABS64",	/* name */
-	 false,			/* partial_inplace */
+	 true,			/* partial_inplace */
 	 0,			/* src_mask */
 	 MINUS_ONE,		/* dst_mask */
 	 true)			/* pcrel_offset */
@@ -103,7 +103,7 @@
 	 complain_overflow_bitfield, /* complain_on_overflow */
 	 bpf_elf_generic_reloc, /* special_function */
 	 "R_BPF_64_NODYLD32",	/* name */
-	 false,			/* partial_inplace */
+	 true,			/* partial_inplace */
 	 0xffffffff,		/* src_mask */
 	 0xffffffff,		/* dst_mask */
 	 true)			/* pcrel_offset */
diff --git a/bfd/elf64-bpf.c b/bfd/elf64-bpf.c
index 0bffe2c5717..c922a3eb48e 100644
--- a/bfd/elf64-bpf.c
+++ b/bfd/elf64-bpf.c
@@ -385,8 +385,7 @@ elf64_bpf_merge_private_bfd_data (bfd *ibfd, struct bfd_link_info *info)
 
 static bfd_reloc_status_type
 bpf_elf_generic_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
-		       void *data, asection *input_section,
-		       bfd *output_bfd ATTRIBUTE_UNUSED,
+		       void *data, asection *input_section, bfd *output_bfd,
 		       char **error_message ATTRIBUTE_UNUSED)
 {
 
@@ -394,6 +393,22 @@ bpf_elf_generic_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
   bfd_reloc_status_type status;
   bfd_byte *where;
 
+  /* From bfd_elf_generic_reloc.  */
+  if (output_bfd != NULL
+      && (symbol->flags & BSF_SECTION_SYM) == 0
+      && (! reloc_entry->howto->partial_inplace
+	  || reloc_entry->addend == 0))
+    {
+      reloc_entry->address += input_section->output_offset;
+      return bfd_reloc_ok;
+    }
+
+  if (output_bfd == NULL
+      && !reloc_entry->howto->pc_relative
+      && (symbol->section->flags & SEC_DEBUGGING) != 0
+      && (input_section->flags & SEC_DEBUGGING) != 0)
+    reloc_entry->addend -= symbol->section->output_section->vma;
+
   /* Sanity check that the address is in range.  */
   bfd_size_type end = bfd_get_section_limit_octets (abfd, input_section);
   bfd_size_type reloc_size;
@@ -407,18 +422,15 @@ bpf_elf_generic_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
       || end - reloc_entry->address < reloc_size)
     return bfd_reloc_outofrange;
 
-  /*  Get the symbol value.  */
-  if (bfd_is_com_section (symbol->section))
-    relocation = 0;
-  else
-    relocation = symbol->value;
+  /* Behave similarly to bfd_install_relocation with install_addend set.
+     That is, just install the addend and do not include the value of
+     the symbol.  */
+  relocation = reloc_entry->addend;
 
   if (symbol->flags & BSF_SECTION_SYM)
     /* Relocation against a section symbol: add in the section base address.  */
     relocation += BASEADDR (symbol->section);
 
-  relocation += reloc_entry->addend;
-
   where = (bfd_byte *) data + reloc_entry->address;
 
   status = bfd_check_overflow (reloc_entry->howto->complain_on_overflow,
@@ -449,8 +461,8 @@ bpf_elf_generic_reloc (bfd *abfd, arelent *reloc_entry, asymbol *symbol,
 	       where + reloc_entry->howto->bitpos / 8);
     }
 
-  reloc_entry->addend = relocation;
-  reloc_entry->address += input_section->output_offset;
+  if (output_bfd != NULL)
+    reloc_entry->address += input_section->output_offset;
 
   return bfd_reloc_ok;
 }
diff --git a/gas/testsuite/gas/bpf/bpf.exp b/gas/testsuite/gas/bpf/bpf.exp
index 44b050e2a00..dae8bd924d0 100644
--- a/gas/testsuite/gas/bpf/bpf.exp
+++ b/gas/testsuite/gas/bpf/bpf.exp
@@ -83,4 +83,7 @@ if {[istarget bpf*-*-*]} {
 
     # Test that parser does not create undefined symbols
     run_dump_test asm-extra-sym-1
+
+    # Test relocation installation
+    run_dump_test elf-relo-1
 }
diff --git a/gas/testsuite/gas/bpf/elf-relo-1.d b/gas/testsuite/gas/bpf/elf-relo-1.d
new file mode 100644
index 00000000000..4a1c7944882
--- /dev/null
+++ b/gas/testsuite/gas/bpf/elf-relo-1.d
@@ -0,0 +1,35 @@
+#as: -EL -mdialect=normal
+#objdump: -tdr
+#source elf-relo-1.s
+#name: eBPF ELF relocations 1
+
+.*: +file format elf64-bpfle
+
+SYMBOL TABLE:
+0000000000000000 l    d  .text	0000000000000000 .text
+0000000000000000 l    d  .data	0000000000000000 .data
+0000000000000000 l    d  .bss	0000000000000000 .bss
+0000000000000006 l       .data	0000000000000000 bar
+0000000000000000 l     F .text	0000000000000000 baz
+0000000000000030 l     F .text	0000000000000000 qux
+0000000000000004 g       .data	0000000000000000 foo
+0000000000000000         \*UND\*	0000000000000000 somefunc
+
+
+
+Disassembly of section .text:
+
+0+ <baz>:
+   0:	18 01 00 00 00 00 00 00 	lddw %r1,0
+   8:	00 00 00 00 00 00 00 00 
+			0: R_BPF_64_64	foo
+  10:	b7 02 00 00 06 00 00 00 	mov %r2,6
+			14: R_BPF_64_ABS32	.data
+  18:	18 03 00 00 30 00 00 00 	lddw %r3,48
+  20:	00 00 00 00 00 00 00 00 
+			18: R_BPF_64_64	.text
+  28:	85 10 00 00 ff ff ff ff 	call -1
+			28: R_BPF_64_32	somefunc
+
+0+30 <qux>:
+  30:	95 00 00 00 00 00 00 00 	exit
diff --git a/gas/testsuite/gas/bpf/elf-relo-1.s b/gas/testsuite/gas/bpf/elf-relo-1.s
new file mode 100644
index 00000000000..23424e05907
--- /dev/null
+++ b/gas/testsuite/gas/bpf/elf-relo-1.s
@@ -0,0 +1,25 @@
+    .global foo
+    .data
+    .byte 1
+    .byte 2
+    .byte 3
+    .byte 4
+foo:
+    .byte 5
+    .byte 6
+bar:
+    .byte 7
+    .byte 8
+
+    .text
+    .align 3
+    .type baz, @function
+baz:
+    lddw    %r1, foo
+    mov     %r2, bar
+    lddw    %r3, qux
+    call    somefunc
+
+    .type qux, @function
+qux:
+    exit


More information about the Binutils-cvs mailing list