[PATCH][V2] Change calling convention of dwarf_getmacros to allow opcode 0xff

Mark Wielaard mjw at redhat.com
Tue Dec 9 13:49:51 UTC 2014


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 at 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 at 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


More information about the elfutils-devel mailing list