[PATCH] libdw, readelf: Don't handle DW_FORM_data16 as expression block/location.

[Mark Wielaard]

Also found by afl-fuzz on the varlocs testcase.
DW_FORM_data16 is constant from according to the DWARF5 spec.
But since it is 128bits it isn't really representable as Dwarf_Word.
So we treat it as block form. But we cannot treat it as an expression
block. Make sure readelf prints it as a regular block and that
dwarf_getlocation[s|_addr] doesn't treat it as location expression.

Signed-off-by: Mark Wielaard 
---
 libdw/dwarf_getlocation.c | 44 +++-
 src/ChangeLog |  5 +
 src/readelf.c | 15 ++-
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
index 7f294fe..fc59a2a 100644
--- a/libdw/dwarf_getlocation.c
+++ b/libdw/dwarf_getlocation.c
@@ -174,6 +174,8 @@ check_constant_offset (Dwarf_Attribute *attr,
 default:
   return 1;
 
+  /* Note, we don't regard DW_FORM_data16 as a constant form,
+even though technically it is according to the standard.  */
 case DW_FORM_data1:
 case DW_FORM_data2:
 case DW_FORM_data4:
@@ -665,7 +667,13 @@ dwarf_getlocation (Dwarf_Attribute *attr, Dwarf_Op 
**llbuf, size_t *listlen)
   if (result != 1)
 return result;
 
-  /* If it has a block form, it's a single location expression.  */
+  /* If it has a block form, it's a single location expression.
+ Except for DW_FORM_data16, which is a 128bit constant.  */
+  if (attr->form == DW_FORM_data16)
+{
+  __libdw_seterrno (DWARF_E_NO_BLOCK);
+  return -1;
+}
   Dwarf_Block block;
   if (INTUSE(dwarf_formblock) (attr, &block) != 0)
 return -1;
@@ -863,9 +871,11 @@ dwarf_getlocation_addr (Dwarf_Attribute *attr, Dwarf_Addr 
address,
   if (llbufs == NULL)
 maxlocs = SIZE_MAX;
 
-  /* If it has a block form, it's a single location expression.  */
+  /* If it has a block form, it's a single location expression.
+ Except for DW_FORM_data16, which is a 128bit constant.  */
   Dwarf_Block block;
-  if (INTUSE(dwarf_formblock) (attr, &block) == 0)
+  if (attr->form != DW_FORM_data16
+  && INTUSE(dwarf_formblock) (attr, &block) == 0)
 {
   if (maxlocs == 0)
return 0;
@@ -876,11 +886,14 @@ dwarf_getlocation_addr (Dwarf_Attribute *attr, Dwarf_Addr 
address,
   return listlens[0] == 0 ? 0 : 1;
 }
 
-  int error = INTUSE(dwarf_errno) ();
-  if (unlikely (error != DWARF_E_NO_BLOCK))
+  if (attr->form != DW_FORM_data16)
 {
-  __libdw_seterrno (error);
-  return -1;
+  int error = INTUSE(dwarf_errno) ();
+  if (unlikely (error != DWARF_E_NO_BLOCK))
+   {
+ __libdw_seterrno (error);
+ return -1;
+   }
 }
 
   int result = check_constant_offset (attr, &llbufs[0], &listlens[0]);
@@ -938,9 +951,11 @@ dwarf_getlocations (Dwarf_Attribute *attr, ptrdiff_t 
offset, Dwarf_Addr *basep,
 
   if (offset == 0)
 {
-  /* If it has a block form, it's a single location expression.  */
+  /* If it has a block form, it's a single location expression.
+Except for DW_FORM_data16, which is a 128bit constant.  */
   Dwarf_Block block;
-  if (INTUSE(dwarf_formblock) (attr, &block) == 0)
+  if (attr->form != DW_FORM_data16
+ && INTUSE(dwarf_formblock) (attr, &block) == 0)
{
  if (getlocation (attr->cu, &block, expr, exprlen,
   cu_sec_idx (attr->cu)) != 0)
@@ -952,11 +967,14 @@ dwarf_getlocations (Dwarf_Attribute *attr, ptrdiff_t 
offset, Dwarf_Addr *basep,
  return 1;
}
 
-  int error = INTUSE(dwarf_errno) ();
-  if (unlikely (error != DWARF_E_NO_BLOCK))
+  if (attr->form != DW_FORM_data16)
{
- __libdw_seterrno (error);
- return -1;
+ int error = INTUSE(dwarf_errno) ();
+ if (unlikely (error != DWARF_E_NO_BLOCK))
+   {
+ __libdw_seterrno (error);
+ return -1;
+   }
}
 
   int result = check_constant_offset (attr, expr, exprlen);
diff --git a/src/ChangeLog b/src/ChangeLog
index 8fb418a..9b119b6 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-15  Mark Wielaard  
+
+   * readelf.c (attr_callback): Only print block as expressions if it
+   isn't DW_FORM_data16.
+
 2018-06-16  Mark Wielaard  
 
* readelf.c (print_debug_loc_section): Make sure next_off doesn't
diff --git a/src/readelf.c b/src/readelf.c
index 2e7378e..4172046 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -7483,11 +7483,16 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
case DW_AT_GNU_call_site_data_value:
case DW_AT_GNU_call_site_target:
case DW_AT_GNU_call_site_target_clobbered:
- putchar ('\n');
- print_ops (cbargs->dwflmod, cbargs->dbg,
-12 + level * 2, 12 + level * 2,
-cbargs->version, cbargs->addrsize, cbargs->offset_size,
-attrp->cu, block.length, block.data);
+ if (form !

Re: [PATCH] readelf: Handle signedness of DW_FORM_implicit_const and DW_AT_const_value.

[Mark Wielaard]

On Wed, Jun 13, 2018 at 02:51:43PM +0200, Mark Wielaard wrote:
> We only handles DW_FORM_sdata as a signed form, but DW_FORM_implicit_const
> is also signed by default. For DW_AT_const_value we can do a little better.
> GCC encodes some const_values with signed forms, even though the type
> is unsigned. Lookup the (base) type of the DIE and display the const value
> as their (signed) type/size (if we can determine that).
> 
> Add a new testcase run-readelf-const-values.sh that shows that.
> With the new testcase the const values would come out as follows:
> 
>   name (string) "i"
>   const_value  (implicit_const) 18446744073709551615
>   name (string) "j"
>   const_value  (implicit_const) 18446744073709551615
> 
>   name (string) "sc"
>   const_value  (sdata) -2
>   name (string) "uc"
>   const_value  (sdata) -2
>   name (string) "ss"
>   const_value  (sdata) -16
>   name (string) "us"
>   const_value  (sdata) -16
>   name (string) "si"
>   const_value  (sdata) -3
>   name (string) "ui"
>   const_value  (sdata) -94967296
>   name (string) "sl"
>   const_value  (sdata) -1
>   name (string) "ul"
>   const_value  (sdata) -1
> 
> With this patch they show up as:
> 
>   name (string) "i"
>   const_value  (implicit_const) -1
>   name (string) "j"
>   const_value  (implicit_const) -1
> 
>   name (string) "sc"
>   const_value  (sdata) -2
>   name (string) "uc"
>   const_value  (sdata) 254 (-2)
>   name (string) "ss"
>   const_value  (sdata) -16
>   name (string) "us"
>   const_value  (sdata) 65520 (-16)
>   name (string) "si"
>   const_value  (sdata) -3
>   name (string) "ui"
>   const_value  (sdata) 42 (-94967296)
>   name (string) "sl"
>   const_value  (sdata) -1
>   name (string) "ul"
>   const_value  (sdata) 18446744073709551615 (-1)
> 
> (for signed/unsigned int char, short and long)

Pushed to master.

Re: [PATCH] readelf: While printing .debug_loc make sure that next_off doesn't overflow.

[Mark Wielaard]

On Wed, Jun 13, 2018 at 03:24:45PM +0200, Mark Wielaard wrote:
> Found by the afl fuzzer. The next offset (after a locview) comes from a
> DIE loclist attribute. This could be a bogus value so large it overflows
> the buffer and makes us print past the end of buffer.

Pushed to master.

[PATCH] backends,bpf: add proper relocation support

[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 .  */
+
+#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