Vassil Vassilev via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 30 12:13:30 PST 2016
On 30/01/16 18:36, David Blaikie wrote:
> On Sat, Jan 30, 2016 at 9:00 AM, Vassil Vassilev
> <v.g.vassilev at gmail.com <mailto:v.g.vassilev at gmail.com>> wrote:
> AFAICT the making a test case independent on STL is the hard part.
> I think it will be always failing due to the relocation of the
> memory region of the underlying SmallVector.
> Sorry, I think I didn't explain myself well. What I mean is - due to
> the instability of test cases for UB (especially library UB), we don't
> usually commit test cases for them - we just fix them without tests.
> About the only time I think committing a test is helpful for UB
> (especially library UB) is if we have a reliable way to test/catch the
> UB (eg: the sanitizers or a checking STL that fails fast on validation
> violations). So, while it would be good to provide/demonstrate your
> test case, I would not commit the test case unless it's a minimal
> reproduction in the presence of one of those tools (sanitizers or
> checking STL) - and would just commit the fix without a test case,
> (I'm not actually reviewing this patch - I don't know much about the
> modules code, but just providing a bit of context and first-pass-review)
Got it. Thanks!
> On 30/01/16 17:37, David Blaikie wrote:
>> Yeah, it's good to have a reproduction, but I wouldn't actually
>> commit one - unless you have a checked stl to test against that
>> always fails on possibly-invalidating sequences of operations,
>> then you'd have a fairly simple reproduction
>> On Jan 30, 2016 8:23 AM, "Vassil Vassilev"
>> <v.g.vassilev at gmail.com <mailto:v.g.vassilev at gmail.com>> wrote:
>> Our module builds choke on merging several modules,
>> containing declarations from STL (we are using libc++, no
>> When writing a new module containing the definition of a STL
>> reverse_iterator, it collects all its template
>> specializations. Some of the specializations need to be
>> deserialized from a third module. This triggers an iterator
>> invalidation bug because of the deserialization happening
>> when iterating the specializations container itself. This
>> happens only when the container's capacity is exceeded and
>> the relocation invalidates the iterators and at best causes a
>> crash (see valgrind reports in the bug report). Unfortunately
>> I haven't been able to make the reproducer independent on STL.
>> On 30/01/16 17:08, David Blaikie wrote:
>>> It might be handy to give an overview of the issue in the
>>> review (& certainly in the commit) message rather than only
>>> citing the bug number
>>> On Jan 30, 2016 6:49 AM, "Vassil Vassilev via cfe-commits"
>>> <cfe-commits at lists.llvm.org
>>> <mailto:cfe-commits at lists.llvm.org>> wrote:
>>> Attaching a fix to
>>> Please review.
>>> Many thanks!
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> <mailto:cfe-commits at lists.llvm.org>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits