Mark Wielaard <m...@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