[PATCH] backends,bpf: add proper relocation support

2018-06-15 Thread Yonghong Song
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

2018-06-16 Thread Yonghong Song
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

2019-01-25 Thread Yonghong Song
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

2019-01-25 Thread Yonghong Song
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

2019-01-25 Thread Yonghong Song
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

2019-01-29 Thread Yonghong Song


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

2019-01-29 Thread Yonghong Song
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