On Fri, 30 Aug 2013 23:55:12 +0200, Mark Wielaard wrote:
> DW_OP_implicit_value and DW_OP_GNU_implicit_pointer already had
> their own special accessors (dwarf_getlocation_implicit_value
> and dwarf_getlocation_implicit_pointer), but it seemed consistent
> to include them in the new more generic accessors too.

So dwarf_getlocation_implicit_value and dwarf_getlocation_implicit_pointer can
be deprecated - that is removed from the .h interfaces?  I haven't tried it
but with .symver also one could have only @version symbol without any
@@version symbol:
        dwarf_getlocation_implicit_value@ELFUTILS_0.143
        dwarf_getlocation_implicit_pointer@ELFUTILS_0.149


> --- a/libdw/dwarf_getlocation.c
> +++ b/libdw/dwarf_getlocation.c
> @@ -95,13 +95,15 @@ loc_compare (const void *p1, const void *p2)
>  /* For each DW_OP_implicit_value, we store a special entry in the cache.
>     This points us directly to the block data for later fetching.  */
>  static void
> -store_implicit_value (Dwarf *dbg, void **cache, Dwarf_Op *op,
> -                   unsigned char *data)
> +store_implicit_value (Dwarf *dbg, void **cache, Dwarf_Op *op)
>  {
>    struct loc_block_s *block = libdw_alloc (dbg, struct loc_block_s,
>                                          sizeof (struct loc_block_s), 1);
> +  const unsigned char *data = (const unsigned char *) op->number2;
> +  Dwarf_Word blength; // Ignored, equal to op->number.
> +  get_uleb128 (blength, data);
>    block->addr = op;
> -  block->data = data + op->number2;
> +  block->data = (unsigned char *) data;
>    block->length = op->number;
>    (void) tsearch (block, cache, loc_compare);
>  }
> @@ -412,11 +414,11 @@ __libdw_intern_expression (Dwarf *dbg, bool 
> other_byte_order,
>         if (unlikely (dbg == NULL))
>           goto invalid;
>  
> +       newloc->number2 = (Dwarf_Word) data; /* start of block inc. len.  */
>         /* XXX Check size.  */
>         get_uleb128 (newloc->number, data); /* Block length.  */
>         if (unlikely ((Dwarf_Word) (end_data - data) < newloc->number))
>           goto invalid;
> -       newloc->number2 = data - block->data; /* Relative block offset.  */

I think it breaks ABI.  Dwarf_Op is a public structure.  Layout of Dwarf_Op
for each of the operations is not explicitly documented but there are various
elfutils API details which are not completely documented.

I do not think it is such a concern but I do not want to be a joker.


>         data += newloc->number;               /* Skip the block.  */
>         break;
>  
> @@ -437,17 +439,20 @@ __libdw_intern_expression (Dwarf *dbg, bool 
> other_byte_order,
>         break;
>  
>       case DW_OP_GNU_const_type:
> -       /* XXX Check size.  */
> -       get_uleb128 (newloc->number, data);
> -       if (unlikely (data >= end_data))
> -         goto invalid;
> -       newloc->number2 = *data++; /* Block length.  */
> -       if (unlikely ((Dwarf_Word) (end_data - data) < newloc->number2))
> -         goto invalid;
> -       /* The third operand is relative block offset:
> -             newloc->number3 = data - block->data;
> -          We don't support this at this point.  */
> -       data += newloc->number2;              /* Skip the block.  */
> +       {
> +         size_t size;
> +
> +         /* XXX Check size.  */
> +         get_uleb128 (newloc->number, data);
> +         if (unlikely (data >= end_data))
> +           goto invalid;
> +
> +         newloc->number2 = (Dwarf_Word) data; /* start of block inc. len.  */
> +         size = *data++;
> +         if (unlikely ((Dwarf_Word) (end_data - data) < size))
> +           goto invalid;
> +         data += size;               /* Skip the block.  */

Likewise.


> +       }
>         break;
>  
>       default:


Thanks,
Jan

Reply via email to