netdev - Re: [PATCH bpf-next v5 00/10] BTF: BPF Type Format

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9fb59140-5c2b-820b-fff1-cbb85b5c22da@fb.com>
Date:   Fri, 15 Jun 2018 09:06:03 -0700
From:   Yonghong Song <yhs@...com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
CC:     Alexei Starovoitov <ast@...com>, Martin KaFai Lau <kafai@...com>,
        <netdev@...r.kernel.org>, Daniel Borkmann <daniel@...earbox.net>,
        <kernel-team@...com>, Wang Nan <wangnan0@...wei.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH bpf-next v5 00/10] BTF: BPF Type Format



On 6/15/18 7:24 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 14, 2018 at 09:56:43PM -0700, Yonghong Song escreveu:
>> I really want to get rid of this option as well. To make pahole work
>> with the default default format, I need to add bpf support to
>> libdwfl in elfutils repo. I will work on that.
> 
> Right, I haven't looked into detail, but perhaps we can do like we do in
> tools/perf/ where we add a feature test to check if some function is
> present in a library (elfutils even) and if so, use it, otherwise, use a
> copy that we carry in pahole.git.
> 
> For instance:
> 
> tools/perf/util/symbol-elf.c
> 
> #ifndef HAVE_ELF_GETPHDRNUM_SUPPORT
> static int elf_getphdrnum(Elf *elf, size_t *dst)
> {
>          GElf_Ehdr gehdr;
>          GElf_Ehdr *ehdr;
> 
>          ehdr = gelf_getehdr(elf, &gehdr);
>          if (!ehdr)
>                  return -1;
> 
>          *dst = ehdr->e_phnum;
> 
>          return 0;
> }
> #endif
> 
> And we have a feature test to check if that is present, simple one, if
> that builds and links, we have it, then the tools build Makefile magic
> will end up defining HAVE_ELF_GETPHDRNUM_SUPPORT and our copy doesn't
> get included, using what is in elfutils:
> 
> [acme@...et perf]$ cat tools/build/feature/test-libelf-getphdrnum.c
> // SPDX-License-Identifier: GPL-2.0
> #include <libelf.h>
> 
> int main(void)
> {
> 	size_t dst;
> 
> 	return elf_getphdrnum(0, &dst);
> }
> [acme@...et perf]$
> 
> [acme@...et perf]$ grep elf /tmp/build/perf/FEATURE-DUMP
> feature-libelf=1
> feature-libelf-getphdrnum=1
> feature-libelf-gelf_getnote=1
> feature-libelf-getshdrstrndx=1
> feature-libelf-mmap=1
> [acme@...et perf]$
> 
> This way a new pahole version won't get to wait till places where it
> gets built have these new functions and we stop using it as soon as the
> library get it.

Agreed that later on we can use feature testing to check pahole output 
is good or not without dwarfris option. If it is good, compilation does 
not need this option and we could deprecate dwarfris option and 
eventually remove it once BPF support is in all major elf libraries.

BTW, I have pushed the following commit
https://reviews.llvm.org/rL334839
to clang so now you can compile with attribute dwarfris directly with 
clang -tartget bpf.

-bash-4.2$ cat t.c
struct tt {
    int a;
    char b;
    int c;
};

int test(struct tt *a) {
   return a->a;
}
-bash-4.2$ clang -target bpf -O2 -g -c -Xclang -target-feature -Xclang 
+dwarfris t.c
-bash-4.2$ llvm-objdump -S -d t.o

t.o:    file format ELF64-BPF

Disassembly of section .text:
test:
; int test(struct tt *a) {
        0:       61 10 00 00 00 00 00 00         r0 = *(u32 *)(r1 + 0)
; return a->a;
        1:       95 00 00 00 00 00 00 00         exit
-bash-4.2$ pahole t.o
struct tt {
         int                        a;                    /*     0     4 */
         char                       b;                    /*     4     1 */

         /* XXX 3 bytes hole, try to pack */

         int                        c;                    /*     8     4 */

         /* size: 12, cachelines: 1, members: 3 */
         /* sum members: 9, holes: 1, sum holes: 3 */
         /* last cacheline: 12 bytes */
};
-bash-4.2$