r243945 - [UB] Fix two cases of UB in copy/pasted code from SmallVector.

[Richard Smith]

These all look completely safe; I'm happy with 243945-243950 going to the
branch.

On Wed, Aug 5, 2015 at 11:56 AM, Hans Wennborg <hans at chromium.org> wrote:

> 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 <chandlerc at 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));
> > +    }
> >    }
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150805/50235388/attachment.html>