This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH][V2] Change calling convention of dwarf_getmacros to allow opcode 0xff
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 09 Dec 2014 14:49:51 +0100
- Subject: Re: [PATCH][V2] Change calling convention of dwarf_getmacros to allow opcode 0xff
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