On Fri, Dec 26, 2014 at 02:12:14PM -0700, Josh Stone wrote:
> /* The lines are sorted by address, so we can use binary
search. */
> - size_t l = 0, u = nlines;
> + size_t l = 0, u = nlines - 1;
> while (l < u)
What if nlines == 0?
I wanted to say that cannot happen. If nlines == 0 then cu->lines would
be NULL. But looking at the code there is a theoretical possibility
that isn't the case. It seems unlikely to happen in practice, but we
should guard against it indeed.
> + size_t idx = u - (u -l) / 2;
[nit] missing a space: "(u - l)"
Fixed.
> + /* This is guaranteed for us by libdw read_srclines. */
> if (nlines > 0)
> assert (lines->info[nlines - 1].end_sequence);
In case your answer above was that nlines is never 0, then this "if" is
redundant.
I moved the if up before the while loop.
> + /* The last line which is less than or equal to addr is what
we want,
> + except with an end_sequence which can only be strictly equal. */
> + Dwarf_Line *line = &lines->info[l];
> + if (line->addr == addr || (! line->end_sequence && line->addr
< addr))
> + return &lines->info[l];
BTW, it seems like the end ought to be exclusive, i.e. [start,end). I
only wrote my commit this way because some existing tests depended on
end-inclusive behavior, and I forgot to argue it. :) Thoughts?
I think you are right and those tests, addrscopes and funcscopes, are
wrong. They use dwfl_module_getsrc to find the line associated with start
and end of the scope. But for the end they use the value of the high_pc
attribute. The high_pc attributes indicates the first address beyond
the current scope of the associated DIE. So the tests should use
highpc - 1 as end of scope.
I created a patch to change dwfl_module_getsrc to not match against
a line with end_sequence set, changed the tests to use highpc -1.
And adjusted this patch for dwarf_getsrc_die. to match the new
behavior. Do those changes look correct to you?
Thanks,
Mark