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

[Hans Wennborg]

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));
> +    }
>    }