From 74e25673d242ed1fd105bfc0dca139ec39f6d938 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 22 Nov 2017 14:34:15 +0000 Subject: [PATCH] Fix some integer sign extension/potential overflow issues. * Reported by Coverity Scan. In all cases, should not be a problem, but with an upcast happening anyway we might as well ensure calculation happens at a higher precision. --- .../Windows/PipelineState/D3D11PipelineStateViewer.cpp | 4 ++-- .../Windows/PipelineState/D3D12PipelineStateViewer.cpp | 2 +- renderdoc/driver/d3d11/d3d11_device.cpp | 2 +- renderdoc/driver/d3d12/d3d12_debug.cpp | 2 +- renderdoc/driver/d3d12/d3d12_device.cpp | 4 ++-- renderdoc/driver/gl/gl_driver.cpp | 2 +- renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp | 7 +++---- renderdoc/driver/vulkan/vk_core.cpp | 2 +- renderdoc/driver/vulkan/vk_debug.cpp | 2 +- renderdoc/driver/vulkan/vk_manager.cpp | 9 +++++---- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp index 7101a8201..acc36a5b0 100644 --- a/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/D3D11PipelineStateViewer.cpp @@ -1837,8 +1837,8 @@ void D3D11PipelineStateViewer::resource_itemActivated(RDTreeWidgetItem *item, in if(view.res.Resource != ResourceId()) { - offs = view.res.FirstElement * view.res.ElementSize; - size = view.res.NumElements * view.res.ElementSize; + offs = uint64_t(view.res.FirstElement) * view.res.ElementSize; + size = uint64_t(view.res.NumElements) * view.res.ElementSize; } else { diff --git a/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp b/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp index a9d8fb5f1..cf6d1441d 100644 --- a/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp +++ b/qrenderdoc/Windows/PipelineState/D3D12PipelineStateViewer.cpp @@ -1733,7 +1733,7 @@ void D3D12PipelineStateViewer::resource_itemActivated(RDTreeWidgetItem *item, in if(view.res.Resource != ResourceId()) { offs = view.res.FirstElement * view.res.ElementSize; - size = view.res.NumElements * view.res.ElementSize; + size = uint64_t(view.res.NumElements) * view.res.ElementSize; } else { diff --git a/renderdoc/driver/d3d11/d3d11_device.cpp b/renderdoc/driver/d3d11/d3d11_device.cpp index 54f60a3ca..c663d5399 100644 --- a/renderdoc/driver/d3d11/d3d11_device.cpp +++ b/renderdoc/driver/d3d11/d3d11_device.cpp @@ -1484,7 +1484,7 @@ bool WrappedID3D11Device::EndFrameCapture(void *dev, void *wnd) thwidth &= ~0x7; // align down to multiple of 8 thheight = uint16_t(float(thwidth) / aspect); - thpixels = new byte[3 * thwidth * thheight]; + thpixels = new byte[3U * thwidth * thheight]; float widthf = float(desc.Width); float heightf = float(desc.Height); diff --git a/renderdoc/driver/d3d12/d3d12_debug.cpp b/renderdoc/driver/d3d12/d3d12_debug.cpp index ee109e31b..c80a57e5f 100644 --- a/renderdoc/driver/d3d12/d3d12_debug.cpp +++ b/renderdoc/driver/d3d12/d3d12_debug.cpp @@ -3982,7 +3982,7 @@ void D3D12DebugManager::InitPostVSBuffers(uint32_t eventID) ID3D12Resource *idxBuf = NULL; bool recreate = false; - uint64_t outputSize = stride * drawcall->numIndices * drawcall->numInstances; + uint64_t outputSize = uint64_t(drawcall->numIndices) * drawcall->numInstances * stride; if(m_SOBufferSize < outputSize) { diff --git a/renderdoc/driver/d3d12/d3d12_device.cpp b/renderdoc/driver/d3d12/d3d12_device.cpp index 692341eaa..75703e388 100644 --- a/renderdoc/driver/d3d12/d3d12_device.cpp +++ b/renderdoc/driver/d3d12/d3d12_device.cpp @@ -975,7 +975,7 @@ bool WrappedID3D12Device::Serialise_WriteToSubresource(SerialiserType &ser, ID3D UINT numSlicesBeforeLast = pDstBox->back - pDstBox->front - 1; UINT numRows = pDstBox->bottom - pDstBox->top; - dataSize = SrcDepthPitch * numSlicesBeforeLast + SrcRowPitch * numRows; + dataSize = SrcDepthPitch * uint64_t(numSlicesBeforeLast) + SrcRowPitch * uint64_t(numRows); } else { @@ -1445,7 +1445,7 @@ bool WrappedID3D12Device::EndFrameCapture(void *dev, void *wnd) thwidth &= ~0x7; // align down to multiple of 8 thheight = uint16_t(float(thwidth) / aspect); - thpixels = new byte[3 * thwidth * thheight]; + thpixels = new byte[3U * thwidth * thheight]; float widthf = float(desc.Width); float heightf = float(desc.Height); diff --git a/renderdoc/driver/gl/gl_driver.cpp b/renderdoc/driver/gl/gl_driver.cpp index 55d4a94b1..a9d856e56 100644 --- a/renderdoc/driver/gl/gl_driver.cpp +++ b/renderdoc/driver/gl/gl_driver.cpp @@ -2691,7 +2691,7 @@ WrappedOpenGL::BackbufferImage *WrappedOpenGL::SaveBackbufferImage() thheight = uint16_t(float(thwidth) / aspect); byte *src = thpixels; - byte *dst = thpixels = new byte[3 * thwidth * thheight]; + byte *dst = thpixels = new byte[3U * thwidth * thheight]; for(uint32_t y = 0; y < thheight; y++) { diff --git a/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp index 0460e6c26..4ce158882 100644 --- a/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_draw_funcs.cpp @@ -729,8 +729,7 @@ WrappedOpenGL::ClientMemoryData *WrappedOpenGL::CopyClientMemoryArrays(GLint fir const void *mmIndices = indices; if(indexType != eGL_NONE) { - uint32_t IdxSize = GetIdxSize(indexType); - idxlen = GLsizeiptr(IdxSize * count); + idxlen = GLsizeiptr(count) * GetIdxSize(indexType); m_Real.glGetIntegerv(eGL_ELEMENT_ARRAY_BUFFER_BINDING, &idxbuf); if(idxbuf == 0) @@ -2568,7 +2567,7 @@ bool WrappedOpenGL::Serialise_glMultiDrawElementsIndirect(SerialiserType &ser, G m_Real.glGetBufferSubData(eGL_DRAW_INDIRECT_BUFFER, offs, sizeof(params), ¶ms); m_Real.glDrawElementsInstancedBaseVertexBaseInstance( - mode, params.count, type, (const void *)ptrdiff_t(params.firstIndex * IdxSize), + mode, params.count, type, (const void *)(ptrdiff_t(params.firstIndex) * IdxSize), params.instanceCount, params.baseVertex, params.baseInstance); } @@ -2916,7 +2915,7 @@ bool WrappedOpenGL::Serialise_glMultiDrawElementsIndirectCountARB(SerialiserType m_Real.glGetBufferSubData(eGL_DRAW_INDIRECT_BUFFER, offs, sizeof(params), ¶ms); m_Real.glDrawElementsInstancedBaseVertexBaseInstance( - mode, params.count, type, (const void *)ptrdiff_t(params.firstIndex * IdxSize), + mode, params.count, type, (const void *)(ptrdiff_t(params.firstIndex) * IdxSize), params.instanceCount, params.baseVertex, params.baseInstance); } diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 87da0e7fa..c52fe2cd1 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -1147,7 +1147,7 @@ bool WrappedVulkan::EndFrameCapture(void *dev, void *wnd) thwidth &= ~0x7; // align down to multiple of 8 thheight = uint16_t(float(thwidth) / aspect); - thpixels = new byte[3 * thwidth * thheight]; + thpixels = new byte[3U * thwidth * thheight]; uint32_t stride = fmt.compByteWidth * fmt.compCount; diff --git a/renderdoc/driver/vulkan/vk_debug.cpp b/renderdoc/driver/vulkan/vk_debug.cpp index a21fa1392..3a52f610c 100644 --- a/renderdoc/driver/vulkan/vk_debug.cpp +++ b/renderdoc/driver/vulkan/vk_debug.cpp @@ -8045,7 +8045,7 @@ void VulkanDebugManager::InitPostVSBuffers(uint32_t eventID) { // fetch ibuffer GetBufferData(state.ibuffer.buf, state.ibuffer.offs + drawcall->indexOffset * idxsize, - drawcall->numIndices * idxsize, idxdata); + uint64_t(drawcall->numIndices) * idxsize, idxdata); // figure out what the maximum index could be, so we can clamp our index buffer to something // sane diff --git a/renderdoc/driver/vulkan/vk_manager.cpp b/renderdoc/driver/vulkan/vk_manager.cpp index 47502c3cd..51ab51da9 100644 --- a/renderdoc/driver/vulkan/vk_manager.cpp +++ b/renderdoc/driver/vulkan/vk_manager.cpp @@ -368,11 +368,12 @@ void VulkanResourceManager::MarkSparseMapReferenced(SparseMapping *sparse) MarkResourceFrameReferenced(GetResID(sparse->opaquemappings[i].memory), eFrameRef_Read); for(int a = 0; a < NUM_VK_IMAGE_ASPECTS; a++) - for(VkDeviceSize i = 0; - sparse->pages[a] && - i < VkDeviceSize(sparse->imgdim.width * sparse->imgdim.height * sparse->imgdim.depth); - i++) + { + VkDeviceSize totalSize = + VkDeviceSize(sparse->imgdim.width) * sparse->imgdim.height * sparse->imgdim.depth; + for(VkDeviceSize i = 0; sparse->pages[a] && i < totalSize; i++) MarkResourceFrameReferenced(GetResID(sparse->pages[a][i].first), eFrameRef_Read); + } } void VulkanResourceManager::ApplyBarriers(vector > &states,