[cfe-commits] clang patch for bug 14021

[David Blaikie]

On Tue, Oct 16, 2012 at 8:12 AM, Robert Muth <robertm at google.com> wrote:
> On Mon, Oct 15, 2012 at 10:57 PM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
>> On 15 October 2012 19:02, Robert Muth <robertm at google.com> wrote:
>>> No small test, sadly.
>>> I managed to cut a 10MB reproducer down to 2MB over 2h
>>> but no further.
>>
>> The testcase in the bug report is already a lot smaller than that.
>> Running delta a bit more reduced it to the attached file, which I am
>> sure can be reduced a bit more.
>
> The reproducer in the bug has a lot of problems, e.g.
> it is only a fragment - not valid C++ code.
> It is also not a very stable reproducers meaning changing the clang
> command line slightly
> will change clang's memory allocation enough that the bug does not get triggered
> anymore and instead you get a compiler failure because the code is
> only a fragment.
>
> This stability issue would likely effect any test for this problem,
> even valgrind based ones.
>
> Having thought  about this a little more think the best way to
> approach the problem of invalidated iterators
> is not by adding tests each time we find one but by addressing it at a
> higher level, e.g. compiling clang or llvm in a
> special debug mode, linking in special debug versions of STL and
> llvm/ADT/ that will cause crashes more reliably.
> Apparently VS has a feature like that, c.f.
> http://stackoverflow.com/questions/2062956/c-best-way-to-check-if-an-iterator-is-valid

Fair point - Howard Hinnant started work on something like this for
libc++ but it's not been finished yet (put on the back burner). MSVC's
debug mode has really helped me on several occasions.

Takumi (who runs most/all of the MSVC bots) - do any of your bots
already run with MSVC's iterator debugging turned up/on? Would that be
a reasonable task? (& interestingly: does it catch this bug)

Robert - while that would help stabilize the behavior, would it
actually help catch this bug? Would it be possible for you to create a
reproduction that always violates the iterator invalidation contract
but doesn't necessarily crash? I wouldn't mind having that in the test
suite even in the absence of fancy iterator checkers being deployed
immediately. Though I'm not immediately sure how you'd construct such
a test case without those iterator checks. (maybe it could be ad-hoc'd
up with a few well-placed assertions & temporary (non-committed) extra
flags just so you can track that the dangerous loop is running during
the modification)

- David