[PATCH] backends,bpf: add proper relocation support
Due to libdw does not have proper BPF relocation support, the pahole cannot display filenames correctly for objects with default llvm options. So we have to invent a special option "llc -march=bpf -mattr=dwarfris" to prevent llvm from generating cross-section dwarf relocation records (https://reviews.llvm.org/rL326505). The pahole related discussion is in linux netdev mailing list (http://lists.openwall.net/netdev/2018/06/15/38, etc.) We would like to add proper BPF relocation support to libdw so eventually we could retire the special llc bpf flag "-mattr=dwarfris". The bpf relocations are defined in llvm_repo:include/llvm/BinaryFormat/ELFRelocs/BPF.def: ELF_RELOC(R_BPF_NONE,0) ELF_RELOC(R_BPF_64_64, 1) ELF_RELOC(R_BPF_64_32, 10) Removed the relocation type R_BPF_MAP_FD whoes name does not confirm to llvm definition and replaced it with R_BPF_64_64. The BPF object is just a relocatible object, not an executable or a shared library, so assign ELF type to REL only in bpf_reloc.def. Tested locally with building pahole with this patch and pahole is able to display structures in bpf object file properly. Signed-off-by: Yonghong Song --- backends/Makefile.am | 2 +- backends/bpf_init.c| 1 + backends/bpf_reloc.def | 3 ++- backends/bpf_symbol.c | 54 ++ libelf/elf.h | 3 ++- 5 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 backends/bpf_symbol.c diff --git a/backends/Makefile.am b/backends/Makefile.am index 80aa00e7..b5e721d8 100644 --- a/backends/Makefile.am +++ b/backends/Makefile.am @@ -126,7 +126,7 @@ am_libebl_m68k_pic_a_OBJECTS = $(m68k_SRCS:.c=.os) # an issue. m68k_corenote_no_Wpacked_not_aligned = yes -bpf_SRCS = bpf_init.c bpf_regs.c +bpf_SRCS = bpf_init.c bpf_regs.c bpf_symbol.c cpu_bpf = ../libcpu/libcpu_bpf.a libebl_bpf_pic_a_SOURCES = $(bpf_SRCS) am_libebl_bpf_pic_a_OBJECTS = $(bpf_SRCS:.c=.os) diff --git a/backends/bpf_init.c b/backends/bpf_init.c index 8ea1bc1a..a046e069 100644 --- a/backends/bpf_init.c +++ b/backends/bpf_init.c @@ -53,6 +53,7 @@ bpf_init (Elf *elf __attribute__ ((unused)), bpf_init_reloc (eh); HOOK (eh, register_info); HOOK (eh, disasm); + HOOK (eh, reloc_simple_type); return MODVERSION; } diff --git a/backends/bpf_reloc.def b/backends/bpf_reloc.def index a410da97..59f519b5 100644 --- a/backends/bpf_reloc.def +++ b/backends/bpf_reloc.def @@ -28,4 +28,5 @@ /* NAME, REL|EXEC|DYN*/ RELOC_TYPE (NONE, EXEC|DYN) -RELOC_TYPE (MAP_FD,REL|EXEC|DYN) +RELOC_TYPE (64_64, REL) +RELOC_TYPE (64_32, REL) diff --git a/backends/bpf_symbol.c b/backends/bpf_symbol.c new file mode 100644 index ..c9856f26 --- /dev/null +++ b/backends/bpf_symbol.c @@ -0,0 +1,54 @@ +/* BPF specific symbolic name handling. + This file is part of elfutils. + + This file is free software; you can redistribute it and/or modify + it under the terms of either + + * the GNU Lesser General Public License as published by the Free + Software Foundation; either version 3 of the License, or (at + your option) any later version + + or + + * the GNU General Public License as published by the Free + Software Foundation; either version 2 of the License, or (at + your option) any later version + + or both in parallel, as here. + + elfutils is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received copies of the GNU General Public License and + the GNU Lesser General Public License along with this program. If + not, see <http://www.gnu.org/licenses/>. */ + +#ifdef HAVE_CONFIG_H +# include +#endif + +#include +#include +#include +#include + +#define BACKEND bpf_ +#include "libebl_CPU.h" + + +/* Check for the simple reloc types. */ +Elf_Type +bpf_reloc_simple_type (Ebl *ebl __attribute__ ((unused)), int type) +{ + switch (type) +{ +case R_BPF_64_64: + return ELF_T_XWORD; +case R_BPF_64_32: + return ELF_T_WORD; +default: + return ELF_T_NUM; +} +} diff --git a/libelf/elf.h b/libelf/elf.h index f7748983..940e88dd 100644 --- a/libelf/elf.h +++ b/libelf/elf.h @@ -3848,7 +3848,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_641 +#define R_BPF_64_3210 /* Imagination Meta specific relocations. */ -- 2.14.3
[PATCH v2] backends,bpf: add proper relocation support
Due to libdw does not have proper BPF relocation support, the pahole cannot display filenames correctly for objects with default llvm options. So we have to invent a special option "llc -march=bpf -mattr=dwarfris" to prevent llvm from generating cross-section dwarf relocation records (https://reviews.llvm.org/rL326505). The pahole related discussion is in linux netdev mailing list (http://lists.openwall.net/netdev/2018/06/15/38, etc.) We would like to add proper BPF relocation support to libdw so eventually we could retire the special llc bpf flag "-mattr=dwarfris". The bpf relocations are defined in llvm_repo:include/llvm/BinaryFormat/ELFRelocs/BPF.def: ELF_RELOC(R_BPF_NONE,0) ELF_RELOC(R_BPF_64_64, 1) ELF_RELOC(R_BPF_64_32, 10) Removed the relocation type R_BPF_MAP_FD whoes name does not confirm to llvm definition and replaced it with R_BPF_64_64. The BPF object is just a relocatible object, not an executable or a shared library, so assign ELF type to REL only in bpf_reloc.def. Signed-off-by: Yonghong Song --- backends/ChangeLog | 7 + backends/Makefile.am| 2 +- backends/bpf_init.c | 1 + backends/bpf_reloc.def | 3 +- backends/bpf_symbol.c | 54 libelf/elf.h| 3 +- tests/ChangeLog | 9 ++ tests/Makefile.am | 5 +++- tests/run-reloc-bpf.sh | 33 ++ tests/testfile-bpf-reloc.expect.bz2 | Bin 0 -> 300 bytes tests/testfile-bpf-reloc.o.bz2 | Bin 0 -> 933 bytes 11 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 backends/bpf_symbol.c create mode 100755 tests/run-reloc-bpf.sh create mode 100644 tests/testfile-bpf-reloc.expect.bz2 create mode 100644 tests/testfile-bpf-reloc.o.bz2 Note: I didn't add the Changelog to libelf/elf.h as I anticipate the change will come from sync'ing with glibc. If this patch version looks good, I can send another revision once the libelf/elf.h is synced. diff --git a/backends/ChangeLog b/backends/ChangeLog index 0dde0ff3..1ffe5ba4 100644 --- a/backends/ChangeLog +++ b/backends/ChangeLog @@ -1,3 +1,10 @@ +2018-06-16 Yonghong Song + + * Makefile.am (bpf_SRCS): Add bpf_symbol.c. + * bpf_init.c (bpf_init): Add reloc_simple_type HOOK. + * bpf_reloc.def: Add RELOC_TYPE 64_64 and 64_32. + * bpf_symbol.c: New file. + 2018-05-15 Andreas Schwab * riscv_init.c (riscv_init): Hook check_special_symbol. diff --git a/backends/Makefile.am b/backends/Makefile.am index 80aa00e7..b5e721d8 100644 --- a/backends/Makefile.am +++ b/backends/Makefile.am @@ -126,7 +126,7 @@ am_libebl_m68k_pic_a_OBJECTS = $(m68k_SRCS:.c=.os) # an issue. m68k_corenote_no_Wpacked_not_aligned = yes -bpf_SRCS = bpf_init.c bpf_regs.c +bpf_SRCS = bpf_init.c bpf_regs.c bpf_symbol.c cpu_bpf = ../libcpu/libcpu_bpf.a libebl_bpf_pic_a_SOURCES = $(bpf_SRCS) am_libebl_bpf_pic_a_OBJECTS = $(bpf_SRCS:.c=.os) diff --git a/backends/bpf_init.c b/backends/bpf_init.c index 8ea1bc1a..a046e069 100644 --- a/backends/bpf_init.c +++ b/backends/bpf_init.c @@ -53,6 +53,7 @@ bpf_init (Elf *elf __attribute__ ((unused)), bpf_init_reloc (eh); HOOK (eh, register_info); HOOK (eh, disasm); + HOOK (eh, reloc_simple_type); return MODVERSION; } diff --git a/backends/bpf_reloc.def b/backends/bpf_reloc.def index a410da97..59f519b5 100644 --- a/backends/bpf_reloc.def +++ b/backends/bpf_reloc.def @@ -28,4 +28,5 @@ /* NAME, REL|EXEC|DYN*/ RELOC_TYPE (NONE, EXEC|DYN) -RELOC_TYPE (MAP_FD,REL|EXEC|DYN) +RELOC_TYPE (64_64, REL) +RELOC_TYPE (64_32, REL) diff --git a/backends/bpf_symbol.c b/backends/bpf_symbol.c new file mode 100644 index ..c9856f26 --- /dev/null +++ b/backends/bpf_symbol.c @@ -0,0 +1,54 @@ +/* BPF specific symbolic name handling. + This file is part of elfutils. + + This file is free software; you can redistribute it and/or modify + it under the terms of either + + * the GNU Lesser General Public License as published by the Free + Software Foundation; either version 3 of the License, or (at + your option) any later version + + or + + * the GNU General Public License as published by the Free + Software Foundation; either version 2 of the License, or (at + your option) any later version + + or both in parallel, as here. + + elfutils is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received copies of the GNU General Public License and + the GNU Lesser General Public License along with this program. If + not, see <http:
[PATCH elfutils 0/2] fix two /proc/pid/maps inode parsing issues
The inode number in /proc/pid/maps has type "unsigned long". The current parsing in libdwfl/linux-proc-maps.c and tests/backtrace-data.c is not correct and it triggered the following four test failures in one x64 box. FAIL: dwfl-bug-fd-leak FAIL: run-backtrace-data.sh FAIL: run-backtrace-dwarf.sh FAIL: vdsosyms The offending map entry 7f269223d000-7f269226b000 r-xp 00:50 10224326387095067468 /home/... has an inode number beyond the valid range of type "long". This patch set fixed the problem in libdwfl and tests respectively. Yonghong Song (2): [libdwfl] parse inode in /proc/pid/maps correctly [tests] parse inode in /proc/pid/maps correctly in run-backtrace-data.sh libdwfl/linux-proc-maps.c | 2 +- tests/backtrace-data.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.17.1
[PATCH elfutils 2/2] [tests] parse inode in /proc/pid/maps correctly in run-backtrace-data.sh
The backtrace-data.c parsed the inode in /proc/pid/maps with format "%*x". This caused failure if inode is big. For example, 7f269223d000-7f269226b000 r-xp 00:50 10224326387095067468 /home/... The correct format should be "%*lu" to reflect inode "unsigned long" type. But that caused the following compilation error. acktrace-data.c: In function ‘maps_lookup’: backtrace-data.c:109:22: error: use of assignment suppression and length modifier together in gnu_scanf format [-Werror=format=] i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*lu", &start, &end, &offset); Fix the test with inode format string "%*s" then. Signed-off-by: Yonghong Song --- tests/backtrace-data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/backtrace-data.c b/tests/backtrace-data.c index 3a91c664..85ae9729 100644 --- a/tests/backtrace-data.c +++ b/tests/backtrace-data.c @@ -106,7 +106,7 @@ maps_lookup (pid_t pid, Dwarf_Addr addr, GElf_Addr *basep) { // 37e3c22000-37e3c23000 rw-p 00022000 00:11 49532 /lib64/ld-2.14.90.so */ unsigned long start, end, offset; - i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*x", &start, &end, &offset); + i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*s", &start, &end, &offset); assert (errno == 0); if (i != 3) break; -- 2.17.1
[PATCH elfutils 1/2] [libdwfl] parse inode in /proc/pid/maps correctly
The inode number in /proc/pid/maps is displayed as "unsigned long" type. In one of our x64 system, we have inode number exceeding valid "long" type range, which caused the following test failure: FAIL: dwfl-bug-fd-leak FAIL: run-backtrace-dwarf.sh FAIL: vdsosyms The offending map entry: 7f269246b000-7f269246c000 rw-p 0002e000 00:50 10224326387095067468 /home/... This patch changed sscanf inode number type from PRIi64 to PRIu64 and fixed the problem. Signed-off-by: Yonghong Song --- libdwfl/linux-proc-maps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libdwfl/linux-proc-maps.c b/libdwfl/linux-proc-maps.c index c4438c0f..719cba68 100644 --- a/libdwfl/linux-proc-maps.c +++ b/libdwfl/linux-proc-maps.c @@ -217,7 +217,7 @@ proc_maps_report (Dwfl *dwfl, FILE *f, GElf_Addr sysinfo_ehdr, pid_t pid) uint64_t ino; int nread = -1; if (sscanf (line, "%" PRIx64 "-%" PRIx64 " %*s %" PRIx64 - " %x:%x %" PRIi64 " %n", + " %x:%x %" PRIu64 " %n", &start, &end, &offset, &dmajor, &dminor, &ino, &nread) < 6 || nread <= 0) { -- 2.17.1
Re: [PATCH elfutils 2/2] [tests] parse inode in /proc/pid/maps correctly in run-backtrace-data.sh
On 1/29/19 12:50 PM, Mark Wielaard wrote: > On Fri, Jan 25, 2019 at 01:20:09PM -0800, Yonghong Song wrote: >> The backtrace-data.c parsed the inode in /proc/pid/maps with >> format "%*x". >> This caused failure if inode is big. For example, >>7f269223d000-7f269226b000 r-xp 00:50 10224326387095067468 >> /home/... > > I have a bit of trouble replicating this (with a simple sscanf). > How exactly does it fail? The error message looks like: -bash-4.4$ cat run-backtrace-data.sh.log backtrace-data: /home/engshare/elfutils/0.174/src/elfutils-0.174/tests/backtrace-data.c:110: maps_lookup: Assertion `errno == 0' failed. /home/engshare/elfutils/0.174/src/elfutils-0.174/tests/test-subr.sh: line 84: 3123578 Aborted (core dumped) LD_LIBRARY_PATH="${built_library_path}${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH" $VALGRIND_CMD "$@" data: no main -bash-4.4$ The reason is errno is ERANGE. > >> The correct format should be "%*lu" to reflect inode "unsigned long" type. >> But that caused the following compilation error. >>acktrace-data.c: In function ‘maps_lookup’: >>backtrace-data.c:109:22: error: use of assignment suppression and length >> modifier >>together in gnu_scanf format [-Werror=format=] >> i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*lu", &start, &end, >> &offset); > > Not that it matters much, since we are really ignoring the rest of the > line and this is just a test. But I do wonder why %*u doesn't work. > The warning says you cannot combine a length specifier with > a ignored format specifier. Which kind of makes sense given that the > length is for the variable to assign the value for, not the format. > So it seems $*u should do the trick. But since I haven't been able > to make the original fail, I might be wrong. "%u" works as well. Let me submit another patch for this. Thanks! > > Cheers, > > Mark >
[PATCH elfutils] [tests] parse inode in /proc/pid/maps/correctly in run-backtrace-data.sh
The backtrace-data.c parsed the inode in /proc/pid/maps with format "%*x". This caused failure if inode is big. For example, 7f269223d000-7f269226b000 r-xp 00:50 10224326387095067468 /home/... The error likes below: -bash-4.4$ cat run-backtrace-data.sh.log backtrace-data: /home/engshare/elfutils/0.174/src/elfutils-0.174/tests/backtrace-data.c:110: maps_lookup: Assertion `errno == 0' failed. /home/engshare/elfutils/0.174/src/elfutils-0.174/tests/test-subr.sh: line 84: 3123578 Aborted (core dumped) LD_LIBRARY_PATH="${built_library_path}${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH" $VALGRIND_CMD "$@" data: no main -bash-4.4$ The reason is errno is ERANGE. Fix the test with inode format string "%*u" as inode here is presented as decimal numbers. Suggested-by: Mark Wielaard Signed-off-by: Yonghong Song --- tests/backtrace-data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/backtrace-data.c b/tests/backtrace-data.c index 3a91c664..b389d6af 100644 --- a/tests/backtrace-data.c +++ b/tests/backtrace-data.c @@ -106,7 +106,7 @@ maps_lookup (pid_t pid, Dwarf_Addr addr, GElf_Addr *basep) { // 37e3c22000-37e3c23000 rw-p 00022000 00:11 49532 /lib64/ld-2.14.90.so */ unsigned long start, end, offset; - i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*x", &start, &end, &offset); + i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*u", &start, &end, &offset); assert (errno == 0); if (i != 3) break; -- 2.17.1