public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: Mark Wielaard <mark@klomp.org>, libc-alpha@sourceware.org
Cc: Yonghong Song <yhs@fb.com>, Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH] elf.h: Add BPF relocation types.
Date: Thu, 21 Jun 2018 15:29:00 -0000	[thread overview]
Message-ID: <c68d5ec9-9cfb-9161-b383-d32dbe5287df@redhat.com> (raw)
In-Reply-To: <20180616214515.10737-1-mark@klomp.org>

On 06/16/2018 05:45 PM, Mark Wielaard wrote:
> The BPF ELF format has new relocation types R_BPF_64_64 and R_BPF_64_32.
> The existing R_BPF_MAP_FD was an extension that never got implemented.
> Remove it, because its constant conflicts with the official R_BPF_64_64.
> ---
>  ChangeLog | 5 +++++
>  elf/elf.h | 3 ++-
>  2 files changed, 7 insertions(+), 1 deletion(-)

I trust you that R_BPF_MAP_FD was never implemented, but I consider it
bad engineering to publish and then reuse a constant, even if you might
argue the entire pseudo-architecture is "unstable."

Having said that we should align the values here with the values being used
by llvm and Facebook. They represent the most recent values being used for
relocations in this pseudo-machine. Without an officially published ABI for
these relocations we have no real way to review what is canonical and what
is not.

I don't see any objections from Florian Weimer (the other reviewer), just
the same frowning face I have :/ at the lack of a proper engineering
specification.

OK to checkin.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/ChangeLog b/ChangeLog
> index 9dd87eebb8..40175d254a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-06-16  Mark Wielaard  <mark@klomp.org>
> +
> +	* elf/elf.h (R_BPF_MAP_FD): Removed.
> +	(R_BPF_64_64, R_BPF_64_32): New.
> +
>  2018-06-15  Samuel Thibault  <samuel.thibault@ens-lyon.org>
>  
>  	* include/sys/sendfile.h (__sendfile64): Declare hidden prototype.
> diff --git a/elf/elf.h b/elf/elf.h
> index a5b2811ce0..75043bcbf9 100644
> --- a/elf/elf.h
> +++ b/elf/elf.h
> @@ -3850,7 +3850,8 @@ enum
>  /* BPF specific declarations.  */
>  
>  #define R_BPF_NONE		0	/* No reloc */
> -#define R_BPF_MAP_FD		1	/* Map fd to pointer */
> +#define R_BPF_64_64		1
> +#define R_BPF_64_32		10

This matches what is in LLVM upstream (not the git mirror).

Changed by this:

commit db476dbf2ee03f8ebdacc439bdde1cf4f0ee629a
Author: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Mon Nov 21 06:21:23 2016 +0000

    [bpf] fix dwarf elf relocs and line numbers
    
    - teach RelocVisitor to recognize bpf relocations
    - fix AsmInfo->PointerSize to make sure dwarf is emitted correctly
    - add a test for the above
    
    Signed-off-by: Alexei Starovoitov <ast@kernel.org>
    
    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@287521 91177308-0d34-0410-b5e6-96231b3b80d8

>  
>  /* Imagination Meta specific relocations. */
>  
> 

  parent reply	other threads:[~2018-06-21 15:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-16 21:46 Mark Wielaard
2018-06-19 20:06 ` Florian Weimer
2018-06-19 20:34   ` Mark Wielaard
2018-06-19 20:38     ` Yonghong Song
2018-06-21 14:53 ` Florian Weimer
2018-06-21 15:29 ` Carlos O'Donell [this message]
2018-06-21 16:48   ` Mark Wielaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c68d5ec9-9cfb-9161-b383-d32dbe5287df@redhat.com \
    --to=carlos@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=mark@klomp.org \
    --cc=rth@twiddle.net \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).