[PATCH] [ADT][CMake][AutoConf] fail-fast iterators for DenseMap
Zachary Turner
zturner at google.com
Mon Mar 16 10:46:47 PDT 2015
As far as I know this is fine, because LLDB always builds the clang that it
links against. It's certainly the case for the Windows and Linux
buildbots. +Greg Clayton in case he knows something different on the apple
side.
On Mon, Mar 16, 2015 at 10:45 AM Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2015 Mar 16, at 09:03, Zachary Turner <zturner at google.com> wrote:
> >
> > If I understand correctly, this broke before because LLDB would redefine
> _NDEBUG sometimes, but not other times when including clang headers.
> >
>
> I think LLDB's NDEBUG inconsistency is a separate issue, and should
> probably be cleaned up/removed (as I pointed out, the original reason
> for redefining NDEBUG was cleaned up years ago).
>
> If the LLVM/clang libraries that LLDB links against are built with
> FAIL_FAST=false, and LLDB builds with FAIL_FAST=true, then you're
> still going to have an ABI problem.
>
> IIUC, with this patch LLDB would need to match FAIL_FAST with the
> LLVM/clang libraries it's linking against.
>
> > Now, since this isn't based on _NDEBUG, this is no longer an issue. If
> so, looks good to me.
> > On Mon, Mar 16, 2015 at 8:35 AM David Blaikie <dblaikie at gmail.com>
> wrote:
> > On Sun, Mar 15, 2015 at 10:48 PM, Sanjoy Das <
> sanjoy at playingwithpointers.com> wrote:
> > Hi chandlerc, dexonsmith, rnk, zturner,
> >
> > This is a re-do of D7931 with the following changes:
> >
> > * the `EpochTracker` class is now conditional on
> > `LLVM_ENABLE_FAIL_FAST_ITERATORS` and not `NDEBUG`.
> >
> > It'd be nice to have a more general conditional name so we can add other
> ABI-breaking debug checks under the same flag. (strawman:
> LLVM_ABI_BREAKING_DEBUG ?)
> >
> >
> > * **The most significant change**: `LLVM_ENABLE_FAIL_FAST_ITERATORS`
> > can be forced to be set or unset independent of `NDEBUG`. By
> > default, cmake and autoconf set it (thus enabling fail-fast
> > iterators) for with-asserts builds and unset it for without-asserts
> > builds.
> >
> > *fingers crossed that this is sufficient for other users* (that they're
> willing to manually unset this value for their debug builds if they need
> ABI compatibility)
> >
> >
> > With these changes, projects that require ABI compatibility between
> > `defined(NDEBUG)` builds and `!defined(NDEBUG)` builds can get that by
> > clamping `LLVM_ENABLE_FAIL_FAST_ITERATORS` to on or off.
> >
> > == Minor changes from D7931 ==
> >
> > * `HandleBase::getEpochAddress` returns a `void *` to emphasise that
> > the result is for comparison, not derefernecing.
> >
> > * `HandleBase::getEpochAddress` was missing in the fail-fast-disabled
> > version of `HandleBase`. This was not a problem earlier since the
> > function is only called from `assert`s and when `assert`s were
> > enabled, we'd always choose the fail-fast-enabled version of
> > `HandleBase`. Since this is no longer the case (a with-asserts
> > build may be using the fail-fast-disabled version of `HandleBase`),
> > the missing method shows up as an obvious compiler error.
> >
> > Original summary for D7931:
> > This patch is an attempt at making `DenseMapIterator`s "fail-fast".
> > Fail-fast iterators that have been invalidated due to insertion into
> > the host `DenseMap` deterministically trip an assert (in debug mode)
> > on access, instead of non-deterministically hitting memory corruption
> > issues.
> >
> > http://reviews.llvm.org/D8351
> >
> > Files:
> > CMakeLists.txt
> > Makefile.config.in
> > Makefile.rules
> > autoconf/configure.ac
> > cmake/modules/HandleLLVMOptions.cmake
> > configure
> > docs/CMake.rst
> > include/llvm/ADT/DenseMap.h
> > include/llvm/ADT/EpochTracker.h
> >
> > EMAIL PREFERENCES
> > http://reviews.llvm.org/settings/panel/emailpreferences/
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150316/72a24713/attachment.html>
More information about the llvm-commits
mailing list