[llvm] r243996 - Avoid passing nullptr to std::equal.

[Yaron Keren via llvm-commits]

An  issue that was patched several times in LLVM is std::equal(nullptr,
nullptr, nullptr) where VC asserts on the third nullptr. Earlier in the
thread Marshal wrote this should return true and not assert.

It is also possible to localize _DEBUG_POINTER_IMPL effect to algorithm
only by having llvm-algorithm.h:

#define _DEBUG_POINTER_IMPL
#include <algorithm>

and have every llvm source include llvm-algorithm.h instead of algorithm.


2015-08-18 2:19 GMT+03:00 Aaron Ballman <aaron at aaronballman.com>:

> On Mon, Aug 17, 2015 at 6:49 PM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
> >
> >> On 2015-Aug-17, at 15:27, Aaron Ballman <aaron at aaronballman.com> wrote:
> >>
> >> On Mon, Aug 17, 2015 at 6:18 PM, Yaron Keren <yaron.keren at gmail.com>
> wrote:
> >>> +aaron, reid
> >>>
> >>> Here is a patch disabling the iterator null pointer checking in Visual
> C++
> >>> debug mode. This makes std::equal behave as expected instead of
> asserting on
> >>> std::equal(nullpt,r nullptr, nullptr), saving an extra check just for
> Visual
> >>> C++ debug mode here and there.
> >>>
> >>> It should not cause any practical problem debugging as if a null
> iterator is
> >>> actually dereferenced (as opposed to compared) the debugger will stop
> on
> >>> location.
> >>>
> >>> What do you think?
> >>
> >> Mildly terrified. :-) This also disables really useful debugging tools
> >> in other parts of the STL that have definitely found bugs for us in
> >> the past. A lot of <algorithm>, <istream>, <ostream>, <locale>,
> >> <memory>, <string>, and <utility> make use of _DEBUG_POINTER, which
> >> this patch would disable.
> >>
> >> I would *strongly* prefer to not disable that debugging functionality,
> >> especially since this runtime-level debugging has found bugs our own
> >> tests have not.
> >>
> >> ~Aaron
> >
> > This _DEBUG_POINTER macro seems to violate the standard in ways that
> > require weird workarounds for LLVM's uses of the STL.
>
> In what way is it violating the standard? (I apologize if this has
> been answered before, but I didn't see anyone citing the standard,
> just some reasonable guesses.) If you believe there is a bug with
> MSVC, has it been reported to Microsoft, and have they weighed in on
> the topic?
>
> ~Aaron
>
> > It seems better to me to make MSVC's STL standards-compliant (when
> > building LLVM) than to duplicate all the STL algorithms inside
> > `namespace llvm`, which IMO is the best alternative.
> >
> > It would be awesome if there were fewer "we don't follow the standard"
> > gotchas that most of us only find, post-commit, in MSVC buildbots.
> >
> >>> 2015-08-17 23:45 GMT+03:00 Duncan P. N. Exon Smith <
> dexonsmith at apple.com>:
> >>>>
> >>>>
> >>>>> On 2015-Aug-17, at 13:43, Yaron Keren <yaron.keren at gmail.com> wrote:
> >>>>>
> >>>>> I'm trying another approach. The non-null validation goes through a
> >>>>> macro defined to a function which possibly may be predefined to
> nothing.
> >>>>> This will disable all null checking on iterators in debug mode which
> is
> >>>>> relevant to additional functions beyond std::equal. In practice it
> should
> >>>>> not be a problem since if a null iterator is actually dereferenced
> (not only
> >>>>> compared) then the debugger stops with invalid access anyhow.
> >>>>
> >>>> Well, this would be ideal :).
> >>>
> >>>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150818/8735fe50/attachment.html>