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

[Petr Machata <pmachata at redhat dot com>]

Mark Wielaard <mjw@redhat.com> writes:

> On Mon, 2014-12-08 at 14:23 +0100, Petr Machata wrote:
>> +  /* 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.

It might happen for macro sections that are larger than half of host
address space (e.g. 2G for 32-bit libdw).  The offset would then be
negative, but adding it to the data start would wrap around and do the
right thing, I think.  So it's the fact that we need that one bit for
something else that's limiting us here.

How about this?

diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
index 0ba3854..bd64d60 100644
--- a/libdw/dwarf_getmacros.c
+++ b/libdw/dwarf_getmacros.c
@@ -407,7 +407,11 @@ token_from_offset (ptrdiff_t offset, bool accept_0xff)
     return offset;
 
   /* Make sure the offset didn't overflow into the flag bit.  */
-  assert ((offset & DWARF_GETMACROS_START) == 0);
+  if ((offset & DWARF_GETMACROS_START) != 0)
+    {
+      __libdw_seterrno (DWARF_E_TOO_BIG);
+      return -1;
+    }
 
   if (accept_0xff)
     offset |= DWARF_GETMACROS_START;

Thanks,
Petr