<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:12.8000001907349px">* some dlopen oddity that leads to an improperly constructed DenseMap</span></blockquote><div><br></div><div>This seems unlikely, since the issue is reproducible without using the python API (i.e., without dlopen).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> <span style="font-size:12.8000001907349px">explicit directions on </span><span style="font-size:12.8000001907349px">how to do so will be very helpful</span></blockquote><div><br></div><div><font face="monospace, monospace">$ cat a.cpp</font></div><div><font face="monospace, monospace">int main(int, char *[]) {} // no segfault without arguments</font></div><div><font face="monospace, monospace">$ make a CXXFLAGS=-g # no segfault without debug symbols</font></div><div><font face="monospace, monospace">$ /path/to/lldb a # or just "/path/to/lldb a -o 'b main' -o r" but another unrelated bug is causing this to occasionally hang if it doesn't segfault</font></div><div><font face="monospace, monospace">(lldb) b main</font></div><div><font face="monospace, monospace">(lldb) r</font></div><div><font face="monospace, monospace">Segmentation fault (core dumped)<br></font></div><div> </div><div>Hope this helps.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 4, 2015 at 12:21 AM, Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm at a loss as to how this could have happened.  Looks like we<br>
somehow end up with DenseMap::Buckets as 0x1 which is clearly not a<br>
valid pointer.<br>
<br>
I have a few theories that I've already discussed with Zachary on IRC:<br>
<br>
 * there is some racy accesses going on to the DenseMap and that is<br>
producing incorrect behavior<br>
<br>
 * some dlopen oddity that leads to an improperly constructed DenseMap<br>
<br>
 * for some reason the incrementEpoch(); in InsertIntoBucketImpl bumps<br>
the "Buckets" pointer by 1 instead of Epoch.  I don't see how this<br>
could happen except maybe due to a bug in the C++ compiler.<br>
<br>
Anyway, I'll try to reproduce this tomorrow (explicit directions on<br>
how to do so will be very helpful).  Please let me know if you have<br>
any other ideas.<br>
<br>
Thanks,<br>
-- Sanjoy<br>
<br>
On Tue, Mar 3, 2015 at 10:05 PM, Chaoren Lin <<a href="mailto:chaorenl@google.com">chaorenl@google.com</a>> wrote:<br>
> Author: chaoren<br>
> Date: Wed Mar  4 00:05:37 2015<br>
> New Revision: 231214<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=231214&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=231214&view=rev</a><br>
> Log:<br>
> Revert "[ADT] fail-fast iterators for DenseMap"<br>
><br>
> This reverts commit 4b7263d855006988854036b4a4891fcf19aebe65.<br>
><br>
> r231125 <a href="http://reviews.llvm.org/D7931" target="_blank">http://reviews.llvm.org/D7931</a><br>
><br>
> This was causing many LLDB tests to fail on OS X, Linux, and FreeBSD.<br>
><br>
> <a href="https://bpaste.net/show/6a23e1f53623" target="_blank">https://bpaste.net/show/6a23e1f53623</a><br>
><br>
> Removed:<br>
>     llvm/trunk/include/llvm/ADT/EpochTracker.h<br>
> Modified:<br>
>     llvm/trunk/include/llvm/ADT/DenseMap.h<br>
><br>
> Modified: llvm/trunk/include/llvm/ADT/DenseMap.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=231214&r1=231213&r2=231214&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=231214&r1=231213&r2=231214&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/ADT/DenseMap.h (original)<br>
> +++ llvm/trunk/include/llvm/ADT/DenseMap.h Wed Mar  4 00:05:37 2015<br>
> @@ -15,7 +15,6 @@<br>
>  #define LLVM_ADT_DENSEMAP_H<br>
><br>
>  #include "llvm/ADT/DenseMapInfo.h"<br>
> -#include "llvm/ADT/EpochTracker.h"<br>
>  #include "llvm/Support/AlignOf.h"<br>
>  #include "llvm/Support/Compiler.h"<br>
>  #include "llvm/Support/MathExtras.h"<br>
> @@ -51,7 +50,7 @@ class DenseMapIterator;<br>
><br>
>  template <typename DerivedT, typename KeyT, typename ValueT, typename KeyInfoT,<br>
>            typename BucketT><br>
> -class DenseMapBase : public DebugEpochBase {<br>
> +class DenseMapBase {<br>
>  public:<br>
>    typedef unsigned size_type;<br>
>    typedef KeyT key_type;<br>
> @@ -63,17 +62,16 @@ public:<br>
>        const_iterator;<br>
>    inline iterator begin() {<br>
>      // When the map is empty, avoid the overhead of AdvancePastEmptyBuckets().<br>
> -    return empty() ? end() : iterator(getBuckets(), getBucketsEnd(), *this);<br>
> +    return empty() ? end() : iterator(getBuckets(), getBucketsEnd());<br>
>    }<br>
>    inline iterator end() {<br>
> -    return iterator(getBucketsEnd(), getBucketsEnd(), *this, true);<br>
> +    return iterator(getBucketsEnd(), getBucketsEnd(), true);<br>
>    }<br>
>    inline const_iterator begin() const {<br>
> -    return empty() ? end()<br>
> -                   : const_iterator(getBuckets(), getBucketsEnd(), *this);<br>
> +    return empty() ? end() : const_iterator(getBuckets(), getBucketsEnd());<br>
>    }<br>
>    inline const_iterator end() const {<br>
> -    return const_iterator(getBucketsEnd(), getBucketsEnd(), *this, true);<br>
> +    return const_iterator(getBucketsEnd(), getBucketsEnd(), true);<br>
>    }<br>
><br>
>    bool LLVM_ATTRIBUTE_UNUSED_RESULT empty() const {<br>
> @@ -83,13 +81,11 @@ public:<br>
><br>
>    /// Grow the densemap so that it has at least Size buckets. Does not shrink<br>
>    void resize(size_type Size) {<br>
> -    incrementEpoch();<br>
>      if (Size > getNumBuckets())<br>
>        grow(Size);<br>
>    }<br>
><br>
>    void clear() {<br>
> -    incrementEpoch();<br>
>      if (getNumEntries() == 0 && getNumTombstones() == 0) return;<br>
><br>
>      // If the capacity of the array is huge, and the # elements used is small,<br>
> @@ -122,13 +118,13 @@ public:<br>
>    iterator find(const KeyT &Val) {<br>
>      BucketT *TheBucket;<br>
>      if (LookupBucketFor(Val, TheBucket))<br>
> -      return iterator(TheBucket, getBucketsEnd(), *this, true);<br>
> +      return iterator(TheBucket, getBucketsEnd(), true);<br>
>      return end();<br>
>    }<br>
>    const_iterator find(const KeyT &Val) const {<br>
>      const BucketT *TheBucket;<br>
>      if (LookupBucketFor(Val, TheBucket))<br>
> -      return const_iterator(TheBucket, getBucketsEnd(), *this, true);<br>
> +      return const_iterator(TheBucket, getBucketsEnd(), true);<br>
>      return end();<br>
>    }<br>
><br>
> @@ -141,14 +137,14 @@ public:<br>
>    iterator find_as(const LookupKeyT &Val) {<br>
>      BucketT *TheBucket;<br>
>      if (LookupBucketFor(Val, TheBucket))<br>
> -      return iterator(TheBucket, getBucketsEnd(), *this, true);<br>
> +      return iterator(TheBucket, getBucketsEnd(), true);<br>
>      return end();<br>
>    }<br>
>    template<class LookupKeyT><br>
>    const_iterator find_as(const LookupKeyT &Val) const {<br>
>      const BucketT *TheBucket;<br>
>      if (LookupBucketFor(Val, TheBucket))<br>
> -      return const_iterator(TheBucket, getBucketsEnd(), *this, true);<br>
> +      return const_iterator(TheBucket, getBucketsEnd(), true);<br>
>      return end();<br>
>    }<br>
><br>
> @@ -167,13 +163,12 @@ public:<br>
>    std::pair<iterator, bool> insert(const std::pair<KeyT, ValueT> &KV) {<br>
>      BucketT *TheBucket;<br>
>      if (LookupBucketFor(KV.first, TheBucket))<br>
> -      return std::make_pair(iterator(TheBucket, getBucketsEnd(), *this, true),<br>
> +      return std::make_pair(iterator(TheBucket, getBucketsEnd(), true),<br>
>                              false); // Already in map.<br>
><br>
>      // Otherwise, insert the new element.<br>
>      TheBucket = InsertIntoBucket(KV.first, KV.second, TheBucket);<br>
> -    return std::make_pair(iterator(TheBucket, getBucketsEnd(), *this, true),<br>
> -                          true);<br>
> +    return std::make_pair(iterator(TheBucket, getBucketsEnd(), true), true);<br>
>    }<br>
><br>
>    // Inserts key,value pair into the map if the key isn't already in the map.<br>
> @@ -182,15 +177,14 @@ public:<br>
>    std::pair<iterator, bool> insert(std::pair<KeyT, ValueT> &&KV) {<br>
>      BucketT *TheBucket;<br>
>      if (LookupBucketFor(KV.first, TheBucket))<br>
> -      return std::make_pair(iterator(TheBucket, getBucketsEnd(), *this, true),<br>
> +      return std::make_pair(iterator(TheBucket, getBucketsEnd(), true),<br>
>                              false); // Already in map.<br>
> -<br>
> +<br>
>      // Otherwise, insert the new element.<br>
>      TheBucket = InsertIntoBucket(std::move(KV.first),<br>
>                                   std::move(KV.second),<br>
>                                   TheBucket);<br>
> -    return std::make_pair(iterator(TheBucket, getBucketsEnd(), *this, true),<br>
> -                          true);<br>
> +    return std::make_pair(iterator(TheBucket, getBucketsEnd(), true), true);<br>
>    }<br>
><br>
>    /// insert - Range insertion of pairs.<br>
> @@ -437,8 +431,6 @@ private:<br>
>    }<br>
><br>
>    BucketT *InsertIntoBucketImpl(const KeyT &Key, BucketT *TheBucket) {<br>
> -    incrementEpoch();<br>
> -<br>
>      // If the load of the hash table is more than 3/4, or if fewer than 1/8 of<br>
>      // the buckets are empty (meaning that many are filled with tombstones),<br>
>      // grow the table.<br>
> @@ -995,10 +987,9 @@ private:<br>
><br>
>  template <typename KeyT, typename ValueT, typename KeyInfoT, typename Bucket,<br>
>            bool IsConst><br>
> -class DenseMapIterator : DebugEpochBase::HandleBase {<br>
> +class DenseMapIterator {<br>
>    typedef DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, true> ConstIterator;<br>
>    friend class DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, true>;<br>
> -  friend class DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, false>;<br>
><br>
>  public:<br>
>    typedef ptrdiff_t difference_type;<br>
> @@ -1012,10 +1003,8 @@ private:<br>
>  public:<br>
>    DenseMapIterator() : Ptr(nullptr), End(nullptr) {}<br>
><br>
> -  DenseMapIterator(pointer Pos, pointer E, const DebugEpochBase &Epoch,<br>
> -                   bool NoAdvance = false)<br>
> -      : DebugEpochBase::HandleBase(&Epoch), Ptr(Pos), End(E) {<br>
> -    assert(isHandleInSync() && "invalid construction!");<br>
> +  DenseMapIterator(pointer Pos, pointer E, bool NoAdvance = false)<br>
> +    : Ptr(Pos), End(E) {<br>
>      if (!NoAdvance) AdvancePastEmptyBuckets();<br>
>    }<br>
><br>
> @@ -1026,40 +1015,28 @@ public:<br>
>              typename = typename std::enable_if<!IsConstSrc && IsConst>::type><br>
>    DenseMapIterator(<br>
>        const DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, IsConstSrc> &I)<br>
> -      : DebugEpochBase::HandleBase(I), Ptr(I.Ptr), End(I.End) {}<br>
> +      : Ptr(I.Ptr), End(I.End) {}<br>
><br>
>    reference operator*() const {<br>
> -    assert(isHandleInSync() && "invalid iterator access!");<br>
>      return *Ptr;<br>
>    }<br>
>    pointer operator->() const {<br>
> -    assert(isHandleInSync() && "invalid iterator access!");<br>
>      return Ptr;<br>
>    }<br>
><br>
>    bool operator==(const ConstIterator &RHS) const {<br>
> -    assert((!Ptr || isHandleInSync()) && "handle not in sync!");<br>
> -    assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");<br>
> -    assert(getEpochAddress() == RHS.getEpochAddress() &&<br>
> -           "comparing incomparable iterators!");<br>
> -    return Ptr == RHS.Ptr;<br>
> +    return Ptr == RHS.operator->();<br>
>    }<br>
>    bool operator!=(const ConstIterator &RHS) const {<br>
> -    assert((!Ptr || isHandleInSync()) && "handle not in sync!");<br>
> -    assert((!RHS.Ptr || RHS.isHandleInSync()) && "handle not in sync!");<br>
> -    assert(getEpochAddress() == RHS.getEpochAddress() &&<br>
> -           "comparing incomparable iterators!");<br>
> -    return Ptr != RHS.Ptr;<br>
> +    return Ptr != RHS.operator->();<br>
>    }<br>
><br>
>    inline DenseMapIterator& operator++() {  // Preincrement<br>
> -    assert(isHandleInSync() && "invalid iterator access!");<br>
>      ++Ptr;<br>
>      AdvancePastEmptyBuckets();<br>
>      return *this;<br>
>    }<br>
>    DenseMapIterator operator++(int) {  // Postincrement<br>
> -    assert(isHandleInSync() && "invalid iterator access!");<br>
>      DenseMapIterator tmp = *this; ++*this; return tmp;<br>
>    }<br>
><br>
><br>
> Removed: llvm/trunk/include/llvm/ADT/EpochTracker.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/EpochTracker.h?rev=231213&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/EpochTracker.h?rev=231213&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/ADT/EpochTracker.h (original)<br>
> +++ llvm/trunk/include/llvm/ADT/EpochTracker.h (removed)<br>
> @@ -1,95 +0,0 @@<br>
> -//===- llvm/ADT/EpochTracker.h - ADT epoch tracking --------------*- C++ -*-==//<br>
> -//<br>
> -//                     The LLVM Compiler Infrastructure<br>
> -//<br>
> -// This file is distributed under the University of Illinois Open Source<br>
> -// License. See LICENSE.TXT for details.<br>
> -//<br>
> -//===----------------------------------------------------------------------===//<br>
> -//<br>
> -// This file defines the DebugEpochBase and DebugEpochBase::HandleBase classes.<br>
> -// These can be used to write iterators that are fail-fast when LLVM is built<br>
> -// with asserts enabled.<br>
> -//<br>
> -//===----------------------------------------------------------------------===//<br>
> -<br>
> -#ifndef LLVM_ADT_EPOCH_TRACKER_H<br>
> -#define LLVM_ADT_EPOCH_TRACKER_H<br>
> -<br>
> -#include <cstdint><br>
> -<br>
> -namespace llvm {<br>
> -<br>
> -#ifdef NDEBUG<br>
> -<br>
> -class DebugEpochBase {<br>
> -public:<br>
> -  void incrementEpoch() {}<br>
> -<br>
> -  class HandleBase {<br>
> -  public:<br>
> -    HandleBase() {}<br>
> -    explicit HandleBase(const DebugEpochBase *) {}<br>
> -    bool isHandleInSync() { return true; }<br>
> -  };<br>
> -};<br>
> -<br>
> -#else<br>
> -<br>
> -/// \brief A base class for data structure classes wishing to make iterators<br>
> -/// ("handles") pointing into themselves fail-fast.  When building without<br>
> -/// asserts, this class is empty and does nothing.<br>
> -///<br>
> -/// DebugEpochBase does not by itself track handles pointing into itself.  The<br>
> -/// expectation is that routines touching the handles will poll on<br>
> -/// isHandleInSync at appropriate points to assert that the handle they're using<br>
> -/// is still valid.<br>
> -///<br>
> -class DebugEpochBase {<br>
> -  uint64_t Epoch;<br>
> -<br>
> -public:<br>
> -  DebugEpochBase() : Epoch(0) {}<br>
> -<br>
> -  /// \brief Calling incrementEpoch invalidates all handles pointing into the<br>
> -  /// calling instance.<br>
> -  void incrementEpoch() { ++Epoch; }<br>
> -<br>
> -  /// \brief The destructor calls incrementEpoch to make use-after-free bugs<br>
> -  /// more likely to crash deterministically.<br>
> -  ~DebugEpochBase() { incrementEpoch(); }<br>
> -<br>
> -  /// \brief A base class for iterator classes ("handles") that wish to poll for<br>
> -  /// iterator invalidating modifications in the underlying data structure.<br>
> -  /// When LLVM is built without asserts, this class is empty and does nothing.<br>
> -  ///<br>
> -  /// HandleBase does not track the parent data structure by itself.  It expects<br>
> -  /// the routines modifying the data structure to call incrementEpoch when they<br>
> -  /// make an iterator-invalidating modification.<br>
> -  ///<br>
> -  class HandleBase {<br>
> -    const uint64_t *EpochAddress;<br>
> -    uint64_t EpochAtCreation;<br>
> -<br>
> -  public:<br>
> -    HandleBase() : EpochAddress(nullptr), EpochAtCreation(UINT64_MAX) {}<br>
> -<br>
> -    explicit HandleBase(const DebugEpochBase *Parent)<br>
> -        : EpochAddress(&Parent->Epoch), EpochAtCreation(Parent->Epoch) {}<br>
> -<br>
> -    /// \brief Returns true if the DebugEpochBase this Handle is linked to has<br>
> -    /// not called incrementEpoch on itself since the creation of this<br>
> -    /// HandleBase instance.<br>
> -    bool isHandleInSync() const { return *EpochAddress == EpochAtCreation; }<br>
> -<br>
> -    /// \brief Returns a pointer to the epoch word stored in the data structure<br>
> -    /// this handle points into.<br>
> -    const uint64_t *getEpochAddress() const { return EpochAddress; }<br>
> -  };<br>
> -};<br>
> -<br>
> -#endif<br>
> -<br>
> -} // namespace llvm<br>
> -<br>
> -#endif<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>