diff mbox series

Add partial support for R_BPF_64_NODLYD32 reloc

Message ID 878r7k719k.fsf@redhat.com
State New
Headers
Series Add partial support for R_BPF_64_NODLYD32 reloc |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_check--master-arm warning Patch is already merged

Commit Message

Nick Clifton Oct. 30, 2023, 12:17 p.m. UTC 
  Hi Cupertino,

  A bug report was recently filed with the Fedora binutils about
  the strip program being unable to handle the R_BPF_64_NODLYD32
  relocation:

https://bugzilla.redhat.com/show_bug.cgi?id=2245296

  It turns out that whilst the GNU tools do not generate this reloc
  other tools do, and strip complains when it detects its presence.

  I am going to apply the attached patch as a workaround so that
  strip will work, but I think that if we want the linker to be able
  to cope with bpf object files generated by other tools, then we will 
  need a better fix.  I am leaving this up to you... :-)

Cheers
  Nick

Comments

Cupertino Miranda Oct. 30, 2023, 1:01 p.m. UTC | #1 
Hi Nick,

Thanks for letting me know of this.
Indeed, I removed it since I failed to find a use case for it.
We clearly forgot the use of tools such as strip, when used with objects
created with clang.

AFAIK, BPF clang support does not allow to link BPF object files, and
the object files are used directly by libbpf without linking.
The discussion was that R_BPF_64_NODLYD32 was specific to clang object
files as I could not find a reason to generate those relocations in gas.

At this moment I am unshure what should be done in the linker for this
relocations. I promisse to come back to this ASAP.

Cheers,
Cupertino


Nick Clifton writes:

> Hi Cupertino,
>
>   A bug report was recently filed with the Fedora binutils about
>   the strip program being unable to handle the R_BPF_64_NODLYD32
>   Relocation:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2245296
>
>   It turns out that whilst the GNU tools do not generate this reloc
>   other tools do, and strip complains when it detects its presence.
>
>   I am going to apply the attached patch as a workaround so that
>   strip will work, but I think that if we want the linker to be able
>   to cope with bpf object files generated by other tools, then we will
>   need a better fix.  I am leaving this up to you... :-)
>
> Cheers
>   Nick
>
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 1b9d13add25..2b12eb35d27 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,7 @@
> +2023-10-30  Nick Clifton  <nickc@redhat.com>
> +
> +	* bpf-reloc.def (R_BPF_64_NODLD32): Add entry.
> +
>  2023-10-16  Nick Clifton  <nickc@redhat.com>
>
>  	PR 28910
> diff --git a/bfd/bpf-reloc.def b/bfd/bpf-reloc.def
> index 31f761d291d..7e7497892fa 100644
> --- a/bfd/bpf-reloc.def
> +++ b/bfd/bpf-reloc.def
> @@ -87,3 +87,20 @@
>          0xffff,                /* src_mask */
>          0xffff,                /* dst_mask */
>          true)                  /* pcrel_offset */
> +
> +  /* R_BPF_64_NODYLD32 is not used by GNU tools - but it is generated by LLVM.
> +     We provide an entry here so that tools like strip can safely handle BPF
> +     binaries generated by other tools.  */
> +  BPF_HOWTO (R_BPF_64_NODYLD32,	/* type */
> +	 0,			/* rightshift */
> +	 0,			/* size */
> +	 0,			/* bitsize */
> +	 false,			/* pc_relative */
> +	 0,			/* bitpos */
> +	 complain_overflow_dont, /* complain_on_overflow */
> +	 bpf_elf_generic_reloc, /* special_function */
> +	 "R_BPF_64_NODYLD32",	/* name */
> +	 false,			/* partial_inplace */
> +	 0,			/* src_mask */
> +	 0,			/* dst_mask */
> +	 false)			/* pcrel_offset */
> diff --git a/include/ChangeLog b/include/ChangeLog
> index 90ee73650c3..e0f0cb3edf4 100644
> --- a/include/ChangeLog
> +++ b/include/ChangeLog
> @@ -1,3 +1,8 @@
> +2023-10-30  Nick Clifton  <nickc@redhat.com>
> +
> +	* elf/bpf.h (R_BPF_64_NODYLD32): Add a note that this reloc is
> +	generated by other tools, eg LLVM.
> +
>  2023-09-28  Frederic Cambus  <fred@statdns.com>
>
>  	* elf/common.h (PT_OPENBSD_NOBTCFI): Define.
> diff --git a/include/elf/bpf.h b/include/elf/bpf.h
> index e4d416290a7..1ad074f3d0e 100644
> --- a/include/elf/bpf.h
> +++ b/include/elf/bpf.h
> @@ -30,8 +30,8 @@ START_RELOC_NUMBERS (elf_bpf_reloc_type)
>    RELOC_NUMBER (R_BPF_64_64,       		1)
>    RELOC_NUMBER (R_BPF_64_ABS64,    		2)
>    RELOC_NUMBER (R_BPF_64_ABS32,    		3)
> -/* R_BPF_64_NODYLD32 is not used by GNU tools.
> - * It is kept in this file to remind that the value is already taken. */
> +/* R_BPF_64_NODYLD32 is not used by GNU tools - but it is generated by LLVM.
> +   It is kept in this file to remind that the value is already taken.  */
>    RELOC_NUMBER (R_BPF_64_NODYLD32, 		4)
>    RELOC_NUMBER (R_BPF_64_32,      		10)
>    RELOC_NUMBER (R_BPF_GNU_64_16,                256)
Jose E. Marchesi Oct. 30, 2023, 1:59 p.m. UTC | #2 
> Hi Nick,
>
> Thanks for letting me know of this.
> Indeed, I removed it since I failed to find a use case for it.
> We clearly forgot the use of tools such as strip, when used with objects
> created with clang.
>
> AFAIK, BPF clang support does not allow to link BPF object files, and
> the object files are used directly by libbpf without linking.
> The discussion was that R_BPF_64_NODLYD32 was specific to clang object
> files as I could not find a reason to generate those relocations in gas.
>
> At this moment I am unshure what should be done in the linker for this
> relocations. I promisse to come back to this ASAP.

The R_BPF_64_NODLYD32 is like R_BPF_64_ABS32.  The only difference (and
the reason why it exists) is that it is ignored by the LLVM
ExecutionEngine.

So I think in the GNU tools R_BPF_64_NODLYD32 should be handled in
exactly the same way than R_BPF_64_ABS32.

> Cheers,
> Cupertino
>
>
> Nick Clifton writes:
>
>> Hi Cupertino,
>>
>>   A bug report was recently filed with the Fedora binutils about
>>   the strip program being unable to handle the R_BPF_64_NODLYD32
>>   Relocation:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=2245296
>>
>>   It turns out that whilst the GNU tools do not generate this reloc
>>   other tools do, and strip complains when it detects its presence.
>>
>>   I am going to apply the attached patch as a workaround so that
>>   strip will work, but I think that if we want the linker to be able
>>   to cope with bpf object files generated by other tools, then we will
>>   need a better fix.  I am leaving this up to you... :-)
>>
>> Cheers
>>   Nick
>>
>> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
>> index 1b9d13add25..2b12eb35d27 100644
>> --- a/bfd/ChangeLog
>> +++ b/bfd/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2023-10-30  Nick Clifton  <nickc@redhat.com>
>> +
>> +	* bpf-reloc.def (R_BPF_64_NODLD32): Add entry.
>> +
>>  2023-10-16  Nick Clifton  <nickc@redhat.com>
>>
>>  	PR 28910
>> diff --git a/bfd/bpf-reloc.def b/bfd/bpf-reloc.def
>> index 31f761d291d..7e7497892fa 100644
>> --- a/bfd/bpf-reloc.def
>> +++ b/bfd/bpf-reloc.def
>> @@ -87,3 +87,20 @@
>>          0xffff,                /* src_mask */
>>          0xffff,                /* dst_mask */
>>          true)                  /* pcrel_offset */
>> +
>> +  /* R_BPF_64_NODYLD32 is not used by GNU tools - but it is generated by LLVM.
>> +     We provide an entry here so that tools like strip can safely handle BPF
>> +     binaries generated by other tools.  */
>> +  BPF_HOWTO (R_BPF_64_NODYLD32,	/* type */
>> +	 0,			/* rightshift */
>> +	 0,			/* size */
>> +	 0,			/* bitsize */
>> +	 false,			/* pc_relative */
>> +	 0,			/* bitpos */
>> +	 complain_overflow_dont, /* complain_on_overflow */
>> +	 bpf_elf_generic_reloc, /* special_function */
>> +	 "R_BPF_64_NODYLD32",	/* name */
>> +	 false,			/* partial_inplace */
>> +	 0,			/* src_mask */
>> +	 0,			/* dst_mask */
>> +	 false)			/* pcrel_offset */
>> diff --git a/include/ChangeLog b/include/ChangeLog
>> index 90ee73650c3..e0f0cb3edf4 100644
>> --- a/include/ChangeLog
>> +++ b/include/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2023-10-30  Nick Clifton  <nickc@redhat.com>
>> +
>> +	* elf/bpf.h (R_BPF_64_NODYLD32): Add a note that this reloc is
>> +	generated by other tools, eg LLVM.
>> +
>>  2023-09-28  Frederic Cambus  <fred@statdns.com>
>>
>>  	* elf/common.h (PT_OPENBSD_NOBTCFI): Define.
>> diff --git a/include/elf/bpf.h b/include/elf/bpf.h
>> index e4d416290a7..1ad074f3d0e 100644
>> --- a/include/elf/bpf.h
>> +++ b/include/elf/bpf.h
>> @@ -30,8 +30,8 @@ START_RELOC_NUMBERS (elf_bpf_reloc_type)
>>    RELOC_NUMBER (R_BPF_64_64,       		1)
>>    RELOC_NUMBER (R_BPF_64_ABS64,    		2)
>>    RELOC_NUMBER (R_BPF_64_ABS32,    		3)
>> -/* R_BPF_64_NODYLD32 is not used by GNU tools.
>> - * It is kept in this file to remind that the value is already taken. */
>> +/* R_BPF_64_NODYLD32 is not used by GNU tools - but it is generated by LLVM.
>> +   It is kept in this file to remind that the value is already taken.  */
>>    RELOC_NUMBER (R_BPF_64_NODYLD32, 		4)
>>    RELOC_NUMBER (R_BPF_64_32,      		10)
>>    RELOC_NUMBER (R_BPF_GNU_64_16,                256)
diff mbox series

Patch

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 1b9d13add25..2b12eb35d27 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,7 @@ 
+2023-10-30  Nick Clifton  <nickc@redhat.com>
+
+	* bpf-reloc.def (R_BPF_64_NODLD32): Add entry.
+
 2023-10-16  Nick Clifton  <nickc@redhat.com>
 
 	PR 28910
diff --git a/bfd/bpf-reloc.def b/bfd/bpf-reloc.def
index 31f761d291d..7e7497892fa 100644
--- a/bfd/bpf-reloc.def
+++ b/bfd/bpf-reloc.def
@@ -87,3 +87,20 @@ 
         0xffff,                /* src_mask */
         0xffff,                /* dst_mask */
         true)                  /* pcrel_offset */
+
+  /* R_BPF_64_NODYLD32 is not used by GNU tools - but it is generated by LLVM.
+     We provide an entry here so that tools like strip can safely handle BPF
+     binaries generated by other tools.  */
+  BPF_HOWTO (R_BPF_64_NODYLD32,	/* type */
+	 0,			/* rightshift */
+	 0,			/* size */
+	 0,			/* bitsize */
+	 false,			/* pc_relative */
+	 0,			/* bitpos */
+	 complain_overflow_dont, /* complain_on_overflow */
+	 bpf_elf_generic_reloc, /* special_function */
+	 "R_BPF_64_NODYLD32",	/* name */
+	 false,			/* partial_inplace */
+	 0,			/* src_mask */
+	 0,			/* dst_mask */
+	 false)			/* pcrel_offset */
diff --git a/include/ChangeLog b/include/ChangeLog
index 90ee73650c3..e0f0cb3edf4 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@ 
+2023-10-30  Nick Clifton  <nickc@redhat.com>
+
+	* elf/bpf.h (R_BPF_64_NODYLD32): Add a note that this reloc is
+	generated by other tools, eg LLVM.
+
 2023-09-28  Frederic Cambus  <fred@statdns.com>
 
 	* elf/common.h (PT_OPENBSD_NOBTCFI): Define.
diff --git a/include/elf/bpf.h b/include/elf/bpf.h
index e4d416290a7..1ad074f3d0e 100644
--- a/include/elf/bpf.h
+++ b/include/elf/bpf.h
@@ -30,8 +30,8 @@  START_RELOC_NUMBERS (elf_bpf_reloc_type)
   RELOC_NUMBER (R_BPF_64_64,       		1)
   RELOC_NUMBER (R_BPF_64_ABS64,    		2)
   RELOC_NUMBER (R_BPF_64_ABS32,    		3)
-/* R_BPF_64_NODYLD32 is not used by GNU tools.
- * It is kept in this file to remind that the value is already taken. */
+/* R_BPF_64_NODYLD32 is not used by GNU tools - but it is generated by LLVM.
+   It is kept in this file to remind that the value is already taken.  */
   RELOC_NUMBER (R_BPF_64_NODYLD32, 		4)
   RELOC_NUMBER (R_BPF_64_32,      		10)
   RELOC_NUMBER (R_BPF_GNU_64_16,                256)