[NFC] Address some low hanging fruit UBSan failures (#4833)

* Address some low hanging fruit UBSan failures

Undefined Behavior Sanitizer causes hard failures on runtime conditions that exhibit undefined behavior as defined by C and C++. These issues may not cause immediately identifiable software errors, but the code behavior is dependent on the compiler and runtime implementation. As a result they may become difficult to diagnose or detect errors in the future if the compiler or runtime changes.

This change addresses occurrences of the following undefined behaviors.

* Performing arithmetic operations on null pointers.
* Calling memcpy with a source or destination pointer that is null.
* Calling memset with a destination pointer that is null.
* Assigning an enum a value which is not a valid enum for that enum.
* Reading from unaligned pointers.

After this change, there are 23 remaining undefined behavior failures in the current DXC tests which are less trivial to resolve.

include/dxc/DxilContainer/DxilPipelineStateValidation.h

@@ -668,7 +668,8 @@ DxilPipelineStateValidation::CheckedReaderWriter::IncrementPos(size_t size) {
 template <typename _T> inline bool
 DxilPipelineStateValidation::CheckedReaderWriter::Cast(_T **ppPtr, size_t size) {
   PSV_RETB(CheckBounds(size));
-  *ppPtr = reinterpret_cast<_T*>(Ptr + Offset);
+  if (Mode != RWMode::CalcSize)
+    *ppPtr = reinterpret_cast<_T*>(Ptr + Offset);
   return true;
 }
 template <typename _T>

include/llvm/ADT/BitVector.h

@@ -534,8 +534,10 @@ private:
     // HLSL Change Starts: Use overridable operator new
     // Bits = (BitWord *)std::realloc(Bits, Capacity * sizeof(BitWord));
     BitWord  *newBits = new BitWord[Capacity];
-    std::memcpy(newBits, Bits, NumBitWords(Size) * sizeof(BitWord));
-    delete[] Bits;
+    if (Bits != nullptr) {
+      std::memcpy(newBits, Bits, NumBitWords(Size) * sizeof(BitWord));
+      delete[] Bits;
+    }
     Bits = newBits;
     // HLSL Change Ends
 

lib/DxcSupport/FileIOHelper.cpp

@@ -986,7 +986,7 @@ DxcGetBlobAsUtf8(IDxcBlob *pBlob, IMalloc *pMalloc,
         *pBlobEncoding = internalEncoding;
       }
       return hr;
-    } else {
+    } else if (utf8CharCount > 0) {
       // Copy to new buffer and null-terminate
       if(!utf8NewCopy.Allocate(utf8CharCount + 1))
         return E_OUTOFMEMORY;

lib/DxilCompression/miniz.c

@@ -1102,7 +1102,9 @@ static mz_bool tdefl_compress_lz_codes(tdefl_compressor *d)
         if (flags & 1)
         {
             mz_uint s0, s1, n0, n1, sym, num_extra_bits;
-            mz_uint match_len = pLZ_codes[0], match_dist = *(const mz_uint16 *)(pLZ_codes + 1);
+            mz_uint16 read_val;
+            memcpy((void*)&read_val, pLZ_codes + 1, sizeof(mz_uint16));
+            mz_uint match_len = pLZ_codes[0], match_dist = read_val;
             pLZ_codes += 3;
 
             MZ_ASSERT(d->m_huff_code_sizes[0][s_tdefl_len_sym[match_len]]);
@@ -1147,7 +1149,7 @@ static mz_bool tdefl_compress_lz_codes(tdefl_compressor *d)
         if (pOutput_buf >= d->m_pOutput_buf_end)
             return MZ_FALSE;
 
-        *(mz_uint64 *)pOutput_buf = bit_buffer;
+        memcpy((void*)pOutput_buf, (void*)&bit_buffer, sizeof(bit_buffer));
         pOutput_buf += (bits_in >> 3);
         bit_buffer >>= (bits_in & ~7);
         bits_in &= 7;

lib/DxilRootSignature/DxilRootSignatureSerializer.cpp

@@ -261,7 +261,8 @@ void SerializeRootSignatureTemplate(_In_ const T_ROOT_SIGNATURE_DESC* pRootSigna
   DxilStaticSamplerDesc *pSS;
   unsigned StaticSamplerSize = sizeof(DxilStaticSamplerDesc)*RS.NumStaticSamplers;
   IFT(Serializer.ReserveBlock((void**)&pSS, StaticSamplerSize, &RS.StaticSamplersOffset));
-  memcpy(pSS, pRS->pStaticSamplers, StaticSamplerSize);
+  if (StaticSamplerSize > 0)
+    memcpy(pSS, pRS->pStaticSamplers, StaticSamplerSize);
 
   // Create the result blob.
   CDxcMallocHeapPtr<char> bytes(DxcGetThreadMallocNoRef());
@@ -450,8 +451,8 @@ void DeserializeRootSignatureTemplate(_In_reads_bytes_(SrcDataSizeInBytes) const
   IFTBOOL(((const char*)pInRTS) + s <= pMaxPtr, E_FAIL);
   if (pRootSignature->NumParameters) {
     pRootSignature->pParameters = new T_ROOT_PARAMETER[pRootSignature->NumParameters];
+    memset((void *)pRootSignature->pParameters, 0, pRootSignature->NumParameters * sizeof(T_ROOT_PARAMETER));
   }
-  memset((void *)pRootSignature->pParameters, 0, s);
 
   for(unsigned iRP = 0; iRP < pRootSignature->NumParameters; iRP++) {
     DxilRootParameterType ParameterType = (DxilRootParameterType)pInRTS[iRP].ParameterType;
@@ -510,8 +511,8 @@ void DeserializeRootSignatureTemplate(_In_reads_bytes_(SrcDataSizeInBytes) const
   IFTBOOL(((const char*)pInSS) + s <= pMaxPtr, E_FAIL);
   if (pRootSignature->NumStaticSamplers) {
     pRootSignature->pStaticSamplers = new DxilStaticSamplerDesc[pRootSignature->NumStaticSamplers];
+    memcpy((void*)pRootSignature->pStaticSamplers, pInSS, s);
   }
-  memcpy((void*)pRootSignature->pStaticSamplers, pInSS, s);
 }
 
 _Use_decl_annotations_

lib/Support/SmallVector.cpp

@@ -23,21 +23,14 @@ void SmallVectorBase::grow_pod(void *FirstEl, size_t MinSizeInBytes,
   if (NewCapacityInBytes < MinSizeInBytes)
     NewCapacityInBytes = MinSizeInBytes;
 
-  void *NewElts;
-  if (BeginX == FirstEl) {
-    NewElts = new char[NewCapacityInBytes]; // HLSL Change: Use overridable operator new
-
-    // Copy the elements over.  No need to run dtors on PODs.
-    memcpy(NewElts, this->BeginX, CurSizeBytes);
-  } else {
-    // If this wasn't grown from the inline copy, grow the allocated space.
-    // HLSL Change Begins: Use overridable operator new
-    NewElts = new char[NewCapacityInBytes];
+  // HLSL Change Begin - Use overridable operator new.
+  void *NewElts = new char[NewCapacityInBytes];
+  if (CurSizeBytes > 0)
     memcpy(NewElts, this->BeginX, CurSizeBytes);
+  if (BeginX != nullptr && BeginX != FirstEl)
     delete[] (char*)this->BeginX;
-    // HLSL Change Ends
-  }
   assert(NewElts && "Out of memory");
+  // HLSL Change End - Use overridable operator new.
 
   this->EndX = (char*)NewElts+CurSizeBytes;
   this->BeginX = NewElts;

lib/Support/regmalloc.cpp

@@ -25,6 +25,7 @@ extern "C" {
   }
   void* regex_realloc(void* ptr, size_t oldsize, size_t newsize) {
     void* newptr = regex_malloc(newsize);
+    if (ptr == nullptr) return newptr;
     if (newptr == nullptr) return nullptr;
     std::memcpy(newptr, ptr, std::min(oldsize, newsize));
     regex_free(ptr);

tools/clang/include/clang/AST/OperationKinds.h

@@ -15,6 +15,8 @@
 #ifndef LLVM_CLANG_AST_OPERATIONKINDS_H
 #define LLVM_CLANG_AST_OPERATIONKINDS_H
 
+#include <limits> // HLSL Change
+
 namespace clang {
   
 /// CastKind - The kind of operation required for a conversion.
@@ -46,7 +48,7 @@ enum CastKind {
   /// reinterpret_casts of l-value expressions to reference types.
   ///    bool b; reinterpret_cast<char&>(b) = 'a';
   CK_LValueBitCast,
-  
+
   /// CK_LValueToRValue - A conversion which causes the extraction of
   /// an r-value from the operand gl-value.  The result of an r-value
   /// conversion is always unqualified.
@@ -112,7 +114,7 @@ enum CastKind {
   /// member pointer in base class.
   ///   int A::*mptr = static_cast<int A::*>(&B::member);
   CK_DerivedToBaseMemberPointer,
-    
+
   /// CK_MemberPointerToBoolean - Member pointer to boolean.  A check
   /// against the null member pointer.
   CK_MemberPointerToBoolean,
@@ -134,14 +136,14 @@ enum CastKind {
   /// CK_ConstructorConversion - Conversion by constructor.
   ///    struct A { A(int); }; A a = A(10);
   CK_ConstructorConversion,
-    
+
   /// CK_IntegralToPointer - Integral to pointer.  A special kind of
   /// reinterpreting conversion.  Applies to normal, ObjC, and block
   /// pointers.
   ///    (char*) 0x1001aab0
   ///    reinterpret_cast<int*>(0)
   CK_IntegralToPointer,
-    
+
   /// CK_PointerToIntegral - Pointer to integral.  A special kind of
   /// reinterpreting conversion.  Applies to normal, ObjC, and block
   /// pointers.
@@ -151,17 +153,17 @@ enum CastKind {
   /// CK_PointerToBoolean - Pointer to boolean conversion.  A check
   /// against null.  Applies to normal, ObjC, and block pointers.
   CK_PointerToBoolean,
-    
+
   /// CK_ToVoid - Cast to void, discarding the computed value.
   ///    (void) malloc(2048)
   CK_ToVoid,
-    
+
   /// CK_VectorSplat - A conversion from an arithmetic type to a
   /// vector of that element type.  Fills all elements ("splats") with
   /// the source value.
   ///    __attribute__((ext_vector_type(4))) int v = 5;
   CK_VectorSplat,
-    
+
   /// CK_IntegralCast - A cast between integral types (other than to
   /// boolean).  Variously a bitcast, a truncation, a sign-extension,
   /// or a zero-extension.
@@ -176,7 +178,7 @@ enum CastKind {
   /// CK_IntegralToFloating - Integral to floating point.
   ///    float f = i;
   CK_IntegralToFloating,
-    
+
   /// CK_FloatingToIntegral - Floating point to integral.  Rounds
   /// towards zero, discarding any fractional component.
   ///    (int) f
@@ -185,12 +187,12 @@ enum CastKind {
   /// CK_FloatingToBoolean - Floating point to boolean.
   ///    (bool) f
   CK_FloatingToBoolean,
-    
+
   /// CK_FloatingCast - Casting between floating types of different size.
   ///    (double) f
   ///    (float) ld
   CK_FloatingCast,
-    
+
   /// CK_CPointerToObjCPointerCast - Casting a C pointer kind to an
   /// Objective-C pointer.
   CK_CPointerToObjCPointerCast,
@@ -282,8 +284,8 @@ enum CastKind {
   CK_AtomicToNonAtomic,
   /// \brief Converts from T to _Atomic(T).
   CK_NonAtomicToAtomic,
-  
-  /// \brief Causes a block literal to by copied to the heap and then 
+
+  /// \brief Causes a block literal to by copied to the heap and then
   /// autoreleased.
   ///
   /// This particular cast kind is used for the conversion from a C++11
@@ -299,7 +301,7 @@ enum CastKind {
 
   // Convert a pointer to a different address space.
   CK_AddressSpaceConversion
-  
+
   // HLSL Change Starts
   ,
   CK_FlatConversion,
@@ -318,11 +320,17 @@ enum CastKind {
   CK_HLSLCC_IntegralToFloating,
   CK_HLSLCC_FloatingToIntegral,
   CK_HLSLCC_FloatingToBoolean,
-  CK_HLSLCC_FloatingCast
-  // HLSL Change Ends  
+  CK_HLSLCC_FloatingCast,
+
+  // HLSL Change - Made CK_Invalid an enum case because otherwise it is UB to
+  // assign it to a value of CastKind.
+  CK_Invalid = std::numeric_limits<unsigned int>::max()
 };
 
-static const CastKind CK_Invalid = static_cast<CastKind>(-1);
+static_assert(
+    sizeof(CastKind) == sizeof(unsigned int),
+    "Cast Kind larger than expected. Must increase value of CK_Invalid.");
+// HLSL Change Ends
 
 enum BinaryOperatorKind {
   // Operators listed in order of precedence.