Richard, Chandler asked me about merging this and the other memcpy-ub patches to 3.7. I'd like to hear what you think.
On the one hand, this doesn't fix a regression from previous releases and the issue it addresses is probably fairly benign at the moment. On the other hand, the patches do fix undefined behaviour and look pretty straight-forward. What do you think? Thanks, Hans On Mon, Aug 3, 2015 at 8:52 PM, Chandler Carruth <chandl...@gmail.com> wrote: > Author: chandlerc > Date: Mon Aug 3 22:52:52 2015 > New Revision: 243945 > > URL: http://llvm.org/viewvc/llvm-project?rev=243945&view=rev > Log: > [UB] Fix two cases of UB in copy/pasted code from SmallVector. > > We should really stop copying and pasting code around. =/ > > Found by UBSan. > > Modified: > cfe/trunk/include/clang/AST/ASTVector.h > cfe/trunk/include/clang/Analysis/Support/BumpVector.h > > Modified: cfe/trunk/include/clang/AST/ASTVector.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTVector.h?rev=243945&r1=243944&r2=243945&view=diff > ============================================================================== > --- cfe/trunk/include/clang/AST/ASTVector.h (original) > +++ cfe/trunk/include/clang/AST/ASTVector.h Mon Aug 3 22:52:52 2015 > @@ -384,14 +384,15 @@ void ASTVector<T>::grow(const ASTContext > T *NewElts = new (C, llvm::alignOf<T>()) T[NewCapacity]; > > // Copy the elements over. > - if (std::is_class<T>::value) { > - std::uninitialized_copy(Begin, End, NewElts); > - // Destroy the original elements. > - destroy_range(Begin, End); > - } > - else { > - // Use memcpy for PODs (std::uninitialized_copy optimizes to memmove). > - memcpy(NewElts, Begin, CurSize * sizeof(T)); > + if (Begin != End) { > + if (std::is_class<T>::value) { > + std::uninitialized_copy(Begin, End, NewElts); > + // Destroy the original elements. > + destroy_range(Begin, End); > + } else { > + // Use memcpy for PODs (std::uninitialized_copy optimizes to memmove). > + memcpy(NewElts, Begin, CurSize * sizeof(T)); > + } > } > > // ASTContext never frees any memory. > > Modified: cfe/trunk/include/clang/Analysis/Support/BumpVector.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Support/BumpVector.h?rev=243945&r1=243944&r2=243945&view=diff > ============================================================================== > --- cfe/trunk/include/clang/Analysis/Support/BumpVector.h (original) > +++ cfe/trunk/include/clang/Analysis/Support/BumpVector.h Mon Aug 3 22:52:52 > 2015 > @@ -223,14 +223,15 @@ void BumpVector<T>::grow(BumpVectorConte > T *NewElts = C.getAllocator().template Allocate<T>(NewCapacity); > > // Copy the elements over. > - if (std::is_class<T>::value) { > - std::uninitialized_copy(Begin, End, NewElts); > - // Destroy the original elements. > - destroy_range(Begin, End); > - } > - else { > - // Use memcpy for PODs (std::uninitialized_copy optimizes to memmove). > - memcpy(NewElts, Begin, CurSize * sizeof(T)); > + if (Begin != End) { > + if (std::is_class<T>::value) { > + std::uninitialized_copy(Begin, End, NewElts); > + // Destroy the original elements. > + destroy_range(Begin, End); > + } else { > + // Use memcpy for PODs (std::uninitialized_copy optimizes to memmove). > + memcpy(NewElts, Begin, CurSize * sizeof(T)); > + } > } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
