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

[Hans Wennborg via cfe-commits]

Great, all merged in r244223.

Thanks,
Hans

On Wed, Aug 5, 2015 at 2:58 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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));
>> > +    }
>> >    }
>
>