[Lldb-commits] [PATCH] D50290: Fix a bug in VMRange

[Zachary Turner via lldb-commits]

I think we have llvm::contains() which would allow you to make this
slightly better. Other than that, good find!


On Fri, Aug 3, 2018 at 5:49 PM Leonard Mosescu via Phabricator <
reviews at reviews.llvm.org> wrote:

> lemo created this revision.
> lemo added reviewers: zturner, labath, teemperor.
> lemo added a project: LLDB.
>
> I noticed a suspicious failure:
>
> [ RUN      ] VMRange.CollectionContains
> llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure
> Value of: VMRange::ContainsRange(collection, VMRange(0x100, 0x104))
>
>   Actual: false
>
> Expected: true
>
> Looking at the code, it is a very real bug:
>
>   class RangeInRangeUnaryPredicate {
>   public:
>     RangeInRangeUnaryPredicate(VMRange range) : _range(range) {} // note
> that _range binds to a temporary!
>     bool operator()(const VMRange &range) const {
>       return range.Contains(_range);
>     }
>     const VMRange &_range;
>   };
>
> This change fixes the bug.
>
>
> Repository:
>   rLLDB LLDB
>
> https://reviews.llvm.org/D50290
>
> Files:
>   include/lldb/Utility/VMRange.h
>   source/Utility/VMRange.cpp
>
>
> Index: source/Utility/VMRange.cpp
> ===================================================================
> --- source/Utility/VMRange.cpp
> +++ source/Utility/VMRange.cpp
> @@ -24,14 +24,16 @@
>
>  bool VMRange::ContainsValue(const VMRange::collection &coll,
>                              lldb::addr_t value) {
> -  ValueInRangeUnaryPredicate in_range_predicate(value);
> -  return llvm::find_if(coll, in_range_predicate) != coll.end();
> +  return llvm::find_if(coll, [&](const VMRange &r) {
> +           return r.Contains(value);
> +         }) != coll.end();
>  }
>
>  bool VMRange::ContainsRange(const VMRange::collection &coll,
>                              const VMRange &range) {
> -  RangeInRangeUnaryPredicate in_range_predicate(range);
> -  return llvm::find_if(coll, in_range_predicate) != coll.end();
> +  return llvm::find_if(coll, [&](const VMRange &r) {
> +           return r.Contains(range);
> +         }) != coll.end();
>  }
>
>  void VMRange::Dump(Stream *s, lldb::addr_t offset, uint32_t addr_width)
> const {
> Index: include/lldb/Utility/VMRange.h
> ===================================================================
> --- include/lldb/Utility/VMRange.h
> +++ include/lldb/Utility/VMRange.h
> @@ -87,24 +87,6 @@
>    void Dump(Stream *s, lldb::addr_t base_addr = 0,
>              uint32_t addr_width = 8) const;
>
> -  class ValueInRangeUnaryPredicate {
> -  public:
> -    ValueInRangeUnaryPredicate(lldb::addr_t value) : _value(value) {}
> -    bool operator()(const VMRange &range) const {
> -      return range.Contains(_value);
> -    }
> -    lldb::addr_t _value;
> -  };
> -
> -  class RangeInRangeUnaryPredicate {
> -  public:
> -    RangeInRangeUnaryPredicate(VMRange range) : _range(range) {}
> -    bool operator()(const VMRange &range) const {
> -      return range.Contains(_range);
> -    }
> -    const VMRange &_range;
> -  };
> -
>    static bool ContainsValue(const VMRange::collection &coll,
>                              lldb::addr_t value);
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180803/a7bd6a73/attachment.html>