On Mon, 2014-12-08 at 14:23 +0100, Petr Machata wrote:
> diff --git a/libdw/ChangeLog b/libdw/ChangeLog
> index 37f94df..7129eab 100644
> --- a/libdw/ChangeLog
> +++ b/libdw/ChangeLog
> @@ -1,3 +1,10 @@
> +2014-12-02  Petr Machata  <pmachata@redhat.com>
> +
> +	* dwarf_getmacros.c (token_from_offset, offset_from_token): New
> +	helper functions.
> +	(do_dwarf_getmacros_die): Merge into dwarf_getmacros.
> +	* libdw.h (DWARF_GETMACROS_START): New macro.
> +
>  2014-11-24  Mark Wielaard  <mjw@redhat.com>
>  
>  	* dwarf_getsrclines.c (read_srclines): Check line_range is not zero
> diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
> index 8d9d78b..0ba3854 100644
> --- a/libdw/dwarf_getmacros.c
> +++ b/libdw/dwarf_getmacros.c
> @@ -384,13 +384,53 @@ read_macros (Dwarf *dbg, int sec_index,
>    return 0;
>  }
>  
> +/* Token layout:
> +
> +   - The highest bit is used for distinguishing between callers that
> +     know that opcode 0xff may have one of two incompatible meanings.
> +     The mask that we use for selecting this bit is
> +     DWARF_GETMACROS_START.
> +
> +   - The rest of the token (31 or 63 bits) encodes address inside the
> +     macro unit.
> +
> +   Besides, token value of 0 signals end of iteration and -1 is
> +   reserved for signaling errors.  That means it's impossible to
> +   represent maximum offset of a .debug_macro unit to new-style
> +   callers (which in practice decreases the permissible macro unit
> +   size by another 1 byte).  */
> +
> +static ptrdiff_t
> +token_from_offset (ptrdiff_t offset, bool accept_0xff)
> +{
> +  if (offset == -1 || offset == 0)
> +    return offset;
> +
> +  /* Make sure the offset didn't overflow into the flag bit.  */
> +  assert ((offset & DWARF_GETMACROS_START) == 0);

Very unlikely, but theoretically a valid thing that might happen I
assume. I am not a fan of asserts in library code in this case. Could we
signal an DWARF_E_INVALID_OFFSET error and return -1 instead? If this
could theoretically happen could you change the code to signal an error
and return -1 before committing. Otherwise just say it won't happen and
commit as is.

Looks good otherwise. The other asserts look like genuine should never
happen (when used correctly) issues.

Thanks,

Mark