Messages in this thread | | | Date | Tue, 7 Oct 2014 11:00:50 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v2] perf tools: fix off-by-one error in maps |
| |
Em Tue, Oct 07, 2014 at 02:47:19PM +0900, Namhyung Kim escreveu: > On Mon, 6 Oct 2014 10:35:32 +0200, Stephane Eranian wrote: > > This patch fixes off-by-one errors in the management of maps. > > A map is defined by start address and length as implemented by map__new():
> > map__init(map, type, start, start + len, pgoff, dso);
> > map->start = addr; > > map->end = end;
> > Consequently, the actual address range is ]start; end[ > > map->end is the first byte outside the range. This patch > > fixes two bugs where upper bound checking was off-by-one.
> > In V2, we fix map_groups__fixup_overlappings() some more > > where map->start was off-by-one as reported by Jiri.
> It seems we also need to fix maps__find():
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c > index b7090596ac50..107a8c90785b 100644 > --- a/tools/perf/util/map.c > +++ b/tools/perf/util/map.c > @@ -752,7 +752,7 @@ struct map *maps__find(struct rb_root *maps, u64 ip) > m = rb_entry(parent, struct map, rb_node); > if (ip < m->start) > p = &(*p)->rb_left; > - else if (ip > m->end) > + else if (ip >= m->end) > p = &(*p)->rb_right; > else > return m;
I keep thinking that this change is making things unclear.
I.e. the _start_ of a map (map->start) is _in_ the map, and the _end_ of a map (map->end) is _in_ the map as well.
if (addr > m->end)
is shorter than:
if (addr >= m->end)
"start" and "end" should have the same rule applied, i.e. if one is in, the other is in as well.
Etc.
- Arnaldo
| |