Re: [PATCH bpf-next v2] docs/bpf: add llvm_reloc.rst to explain llvm bpf relocations

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "Fāng-ruì Sòng" <maskray@google.com>
Cc: Yonghong Song <yhs@fb.com>, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Lorenz Bauer <lmb@cloudflare.com>
Subject: Re: [PATCH bpf-next v2] docs/bpf: add llvm_reloc.rst to explain llvm bpf relocations
Date: Tue, 8 Jun 2021 11:32:05 -0700	[thread overview]
Message-ID: <20210608183205.l22q43hinv6lzb4h@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAFP8O3KayCgP6OqF1Vx8afav==jkL038m0rK66b7jJ0DOO=uJQ@mail.gmail.com>

On Tue, Jun 08, 2021 at 09:33:28AM -0700, Fāng-ruì Sòng wrote:
> On Tue, Jun 8, 2021 at 8:49 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Mon, Jun 7, 2021 at 10:51 PM Fāng-ruì Sòng <maskray@google.com> wrote:
> > >
> > > You can rename R_BPF_64_64 to something more meaningful, e.g. R_BPF_64_LDIMM64.
> > > Then I am fine that such a relocation type applies inconsecutive bytes.
> > >
> > > See below. Just change every occurrence of the old name in llvm-project.
> >
> > No. We cannot rename them, because certain gnu tools resolve relos by name
> > and not by number.
> 
> How do the GNU tools resolve relocations by name instead of by
> relocation type number?
> I don't think this should and can be supported.
> 
> Most tools should do:
> if (type == R_BPF_64_64) do_something();
> 
> You are free to change them to
> if (type == R_BPF_64_LDIMM64) do_something();
> as long as R_BPF_64_LDIMM64 is defined as the number.

If you're going to succeed convincing elfutils maintainers to change
their whole design then we can realistically talk about renaming.
As a homework try cloning elfutils.git then change the name in backends/x86_64_reloc.def
or bpf_reloc.def while keeping the numbers and observe how the standard tools stop working.

Also R_BPF_64_64 may not be the best name, but R_BPF_64_LDIMM64 is
not a good name either. Most architectures avoid using instruction mnemonic
in relo names. The relo name should describe what it does instead of insn
it applies to. TLS, GOT, PLT, ABS are good suffixes to use. LDIMM64 - not really.
Instead of R_BPF_64_32 we could have used R_BPF_64_PC32, but not R_BPF_64_CALL32.
Anyway it's too late to change.