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 <[email protected]> wrote: > Ping! :) > > ~Will > > On Mon, Nov 4, 2013 at 4:32 PM, Will Dietz <[email protected]> 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 <[email protected]> > 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
