[cfe-commits] clang patch for bug 14021

Robert Muth robertm at google.com
Tue Oct 16 10:09:55 PDT 2012

On Tue, Oct 16, 2012 at 11:47 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 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)

Without knowing much about how MSVC is accomplishing this,
it probably relies on extra book keeping inside STL.
The container implicated here , DenseMap, is not an STL container,
though, so some extra
work would be required to make it fit the MSVC framework.

> Robert - while that would help stabilize the behavior, would it
> actually help catch this bug? Would it be possible for you to create a

I am pretty sure it would catch this bug *reliably*, because
the reproducer does add items to  DenseMap while iterating over it which
 invalidates the iterators "de jure".
But this invalidation only becomes visible when it is accompanied by a resizing
of the DenseMap and that seems to depend on the concrete program run, e.g.
we can influence the resize trigger by changing the length of strings
passed on clang command line.

> 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

The best I could do so far was a 2MB reproducer which was not reliable
and since the reproducer is so large it is hard to get to the essence
of the problem.
I was hoping that one of the clang/c++ wizards would look at the diagnosis in
and go:
"yaeh, of course there is a scenario that when you try to pick the right
constructors for an expression, additional constructors are being
added to the list of candidates because of some template instantiation."

> 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

More information about the cfe-commits mailing list