[PATCH] ASTVector: Fix return value of various insert() methods

[Richard Smith]

Change LGTM. It'd be nice to have test coverage for this that doesn't
require running a sanitizer. Do we have any direct tests for ASTVector?


On Mon, Nov 18, 2013 at 9:35 PM, Will Dietz <wdietz2 at illinois.edu> wrote:

> Ping! :)
>
> ~Will
>
> On Mon, Nov 4, 2013 at 4:32 PM, Will Dietz <wdietz2 at illinois.edu> wrote:
> > Ping.
> >
> > It's easy to get clang to trigger this bug which results in an invalid
> > iterator to be returned (which the current code happens to ignore, but
> > that's just a lucky coincidence), as this regularly occurs during
> > execution of the lit tests.
> >
> > On a related note, any suggestions on how to create a simple dummy
> > ASTContext for testing? As noted in
> > the commit that originally added ASTVectorTest.cpp (r186253) this
> > blocks the creation of even basic
> > functionality tests for this data structure.
> >
> > ~Will
> >
> > On Mon, Oct 28, 2013 at 5:11 PM, Will Dietz <wdietz2 at illinois.edu>
> wrote:
> >> Error caught -fsanitize=pointer-overflow[1], curiously enough :).
> >>
> >> The pointer overflow occurred when insert() was invoked with From==To,
> >> which is done in quite a few places.  While std::vector::insert
> >> requires [From,To) to be valid, it looks like here From==To is
> >> intended to be supported[2], making the bug in the container not in
> >> its use.
> >>
> >> This patch fixes the overflow when From==To, as well as the return
> >> value in this variant as well as the "fill" variant, changing them to
> >> return an iterator pointing to the first of the inserted elements
> >> (like SmallVector does).
> >>
> >> See attached.
> >>
> >> ~Will
> >>
> >> [1] Patches coming soon.
> >> [2] See the implementation of append(), for example.
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131119/943d0ebc/attachment.html>