On 30/01/16 18:36, David Blaikie wrote:
On Sat, Jan 30, 2016 at 9:00 AM, Vassil Vassilev
<v.g.vassi...@gmail.com <mailto:v.g.vassi...@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,
usually.
(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!
--Vassil
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.vassi...@gmail.com <mailto:v.g.vassi...@gmail.com>> wrote:
Sorry.
Our module builds choke on merging several modules,
containing declarations from STL (we are using libc++, no
modulemaps).
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.
--Vassil
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@lists.llvm.org
<mailto:cfe-commits@lists.llvm.org>> wrote:
Attaching a fix to
https://llvm.org/bugs/show_bug.cgi?id=26237
Please review.
Many thanks!
--Vassil
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
<mailto:cfe-commits@lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
