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

[Will Dietz]

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.

From d89275825fdfa88c78719df5489433277e56e735 Mon Sep 17 00:00:00 2001
From: Will Dietz <w...@wdtz.org>
Date: Mon, 28 Oct 2013 08:10:34 -0500
Subject: [PATCH] ASTVector: Fix return value of various insert() methods.

Error caught using -fsanitize=pointer-overflow.
---
 include/clang/AST/ASTVector.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/clang/AST/ASTVector.h b/include/clang/AST/ASTVector.h
index 6db918e..c6dc37b 100644
--- a/include/clang/AST/ASTVector.h
+++ b/include/clang/AST/ASTVector.h
@@ -245,14 +245,14 @@ public:
 
   iterator insert(const ASTContext &C, iterator I, size_type NumToInsert,
                   const T &Elt) {
+    // Convert iterator to elt# to avoid invalidating iterator when we reserve()
+    size_t InsertElt = I - this->begin();
+
     if (I == this->end()) {  // Important special case for empty vector.
       append(C, NumToInsert, Elt);
-      return this->end()-1;
+      return this->begin()+InsertElt;
     }
 
-    // Convert iterator to elt# to avoid invalidating iterator when we reserve()
-    size_t InsertElt = I - this->begin();
-
     // Ensure there is enough space.
     reserve(C, static_cast<unsigned>(this->size() + NumToInsert));
 
@@ -293,14 +293,15 @@ public:
 
   template<typename ItTy>
   iterator insert(const ASTContext &C, iterator I, ItTy From, ItTy To) {
+    // Convert iterator to elt# to avoid invalidating iterator when we reserve()
+    size_t InsertElt = I - this->begin();
+
     if (I == this->end()) {  // Important special case for empty vector.
       append(C, From, To);
-      return this->end()-1;
+      return this->begin()+InsertElt;
     }
 
     size_t NumToInsert = std::distance(From, To);
-    // Convert iterator to elt# to avoid invalidating iterator when we reserve()
-    size_t InsertElt = I - this->begin();
 
     // Ensure there is enough space.
     reserve(C, static_cast<unsigned>(this->size() + NumToInsert));
-- 
1.8.4.1


_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits