From 83b6c98563ef161e0a2173ffbca6251182750119 Mon Sep 17 00:00:00 2001 From: eladash Date: Sat, 6 Oct 2018 14:54:47 +0300 Subject: [PATCH] rsx: Fix u16 index arrays overflow Force u32 index array destinations to avoid overflows when adding vertex base index. --- rpcs3/Emu/RSX/Capture/rsx_capture.cpp | 10 +++--- rpcs3/Emu/RSX/Common/BufferUtils.cpp | 45 +++++++++++++-------------- rpcs3/Emu/RSX/D3D12/D3D12Buffer.cpp | 4 +-- rpcs3/Emu/RSX/GL/GLVertexBuffers.cpp | 17 +++++----- rpcs3/Emu/RSX/VK/VKVertexBuffers.cpp | 13 ++------ 5 files changed, 39 insertions(+), 50 deletions(-) diff --git a/rpcs3/Emu/RSX/Capture/rsx_capture.cpp b/rpcs3/Emu/RSX/Capture/rsx_capture.cpp index 87d61ee996..eb34bce640 100644 --- a/rpcs3/Emu/RSX/Capture/rsx_capture.cpp +++ b/rpcs3/Emu/RSX/Capture/rsx_capture.cpp @@ -236,12 +236,12 @@ namespace rsx auto fifo = vm::ptr::make(idxAddr); for (u32 i = 0; i < idxCount; ++i) { - u16 index = fifo[i]; - if (is_primitive_restart_enabled && (u32)index == primitive_restart_index) + u32 index = fifo[i]; + if (is_primitive_restart_enabled && index == primitive_restart_index) continue; - index = (u16)get_index_from_base(index, method_registers.vertex_data_base_index()); - min_index = (u16)std::min(index, (u16)min_index); - max_index = (u16)std::max(index, (u16)max_index); + index = get_index_from_base(index, method_registers.vertex_data_base_index()); + min_index = std::min(index, min_index); + max_index = std::max(index, max_index); } break; } diff --git a/rpcs3/Emu/RSX/Common/BufferUtils.cpp b/rpcs3/Emu/RSX/Common/BufferUtils.cpp index ae4d19881b..71caafc79f 100644 --- a/rpcs3/Emu/RSX/Common/BufferUtils.cpp +++ b/rpcs3/Emu/RSX/Common/BufferUtils.cpp @@ -539,10 +539,10 @@ void write_vertex_array_data_to_buffer(gsl::span raw_dst_span, gsl::s namespace { template -std::tuple upload_untouched(gsl::span> src, gsl::span dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) +std::tuple upload_untouched(gsl::span> src, gsl::span dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) { - T min_index = -1; - T max_index = 0; + u32 min_index = -1; + u32 max_index = 0; verify(HERE), (dst.size_bytes() >= src.size_bytes()); @@ -555,27 +555,26 @@ std::tuple upload_untouched(gsl::span> src, gsl::spa if (rsx::method_registers.current_draw_clause.is_disjoint_primitive) continue; - index = -1; + dst[dst_idx++] = -1u; } else { - index = rsx::get_index_from_base(index, base_index); - max_index = std::max(max_index, index); - min_index = std::min(min_index, index); + const u32 new_index = rsx::get_index_from_base((u32)index, base_index); + max_index = std::max(max_index, new_index); + min_index = std::min(min_index, new_index); + dst[dst_idx++] = new_index; } - - dst[dst_idx++] = index; } return std::make_tuple(min_index, max_index, dst_idx); } template -std::tuple expand_indexed_triangle_fan(gsl::span> src, gsl::span dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) +std::tuple expand_indexed_triangle_fan(gsl::span> src, gsl::span dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) { - const T invalid_index = (T)-1; + const u32 invalid_index = -1u; - T min_index = invalid_index; - T max_index = 0; + u32 min_index = invalid_index; + u32 max_index = 0; verify(HERE), (dst.size() >= 3 * (src.size() - 2)); @@ -583,12 +582,12 @@ std::tuple expand_indexed_triangle_fan(gsl::span> sr u32 src_idx = 0; bool needs_anchor = true; - T anchor = invalid_index; - T last_index = invalid_index; + u32 anchor = invalid_index; + u32 last_index = invalid_index; for (size_t src_idx = 0; src_idx < src.size(); ++src_idx) { - T index = src[src_idx]; + u32 index = src[src_idx]; index = rsx::get_index_from_base(index, base_index); if (needs_anchor) @@ -629,20 +628,20 @@ std::tuple expand_indexed_triangle_fan(gsl::span> sr } template -std::tuple expand_indexed_quads(gsl::span> src, gsl::span dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) +std::tuple expand_indexed_quads(gsl::span> src, gsl::span dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) { - T min_index = -1; - T max_index = 0; + u32 min_index = -1; + u32 max_index = 0; verify(HERE), (4 * dst.size_bytes() >= 6 * src.size_bytes()); u32 dst_idx = 0; u8 set_size = 0; - T tmp_indices[4]; + u32 tmp_indices[4]; for (int src_idx = 0; src_idx < src.size(); ++src_idx) { - T index = src[src_idx]; + u32 index = src[src_idx]; index = rsx::get_index_from_base(index, base_index); if (is_primitive_restart_enabled && (u32)src[src_idx] == primitive_restart_index) { @@ -795,7 +794,7 @@ namespace // TODO: Unify indexed and non indexed primitive expansion ? template - std::tuple write_index_array_data_to_buffer_impl(gsl::span dst, + std::tuple write_index_array_data_to_buffer_impl(gsl::span dst, gsl::span> src, rsx::primitive_type draw_mode, bool restart_index_enabled, u32 restart_index, const std::vector > &first_count_arguments, u32 base_index, std::function expands) @@ -833,7 +832,7 @@ std::tuple write_index_array_data_to_buffer(gsl::span switch (type) { case rsx::index_array_type::u16: - return write_index_array_data_to_buffer_impl(as_span_workaround(dst), + return write_index_array_data_to_buffer_impl(as_span_workaround(dst), as_const_span>(src), draw_mode, restart_index_enabled, restart_index, first_count_arguments, base_index, expands); case rsx::index_array_type::u32: return write_index_array_data_to_buffer_impl(as_span_workaround(dst), diff --git a/rpcs3/Emu/RSX/D3D12/D3D12Buffer.cpp b/rpcs3/Emu/RSX/D3D12/D3D12Buffer.cpp index f1225eea4a..d9c24ab073 100644 --- a/rpcs3/Emu/RSX/D3D12/D3D12Buffer.cpp +++ b/rpcs3/Emu/RSX/D3D12/D3D12Buffer.cpp @@ -395,7 +395,7 @@ namespace rsx::index_array_type::u32: rsx::method_registers.index_type(); - size_t index_size = get_index_type_size(indexed_type); + constexpr size_t index_size = sizeof(u32); // Force u32 destination to avoid overflows when adding base // Alloc size_t buffer_size = align(index_count * index_size, 64); @@ -418,7 +418,7 @@ namespace m_buffer_data.unmap(CD3DX12_RANGE(heap_offset, heap_offset + buffer_size)); D3D12_INDEX_BUFFER_VIEW index_buffer_view = { m_buffer_data.get_heap()->GetGPUVirtualAddress() + heap_offset, (UINT)buffer_size, - get_index_type(indexed_type)}; + DXGI_FORMAT_R32_UINT}; // m_timers.buffer_upload_size += buffer_size; command_list->IASetIndexBuffer(&index_buffer_view); diff --git a/rpcs3/Emu/RSX/GL/GLVertexBuffers.cpp b/rpcs3/Emu/RSX/GL/GLVertexBuffers.cpp index 2175303036..0346309e1f 100644 --- a/rpcs3/Emu/RSX/GL/GLVertexBuffers.cpp +++ b/rpcs3/Emu/RSX/GL/GLVertexBuffers.cpp @@ -47,8 +47,7 @@ namespace if (!gl::is_primitive_native(draw_mode)) vertex_draw_count = (u32)get_index_count(draw_mode, ::narrow(vertex_draw_count)); - u32 type_size = ::narrow(get_index_type_size(type)); - u32 block_sz = vertex_draw_count * type_size; + u32 block_sz = vertex_draw_count * sizeof(u32); // Force u32 index size dest to avoid overflows when adding vertex base index gsl::span dst{ reinterpret_cast(ptr), ::narrow(block_sz) }; std::tie(min_index, max_index, vertex_draw_count) = write_index_array_data_to_buffer(dst, raw_index_buffer, @@ -103,7 +102,7 @@ namespace rsx::method_registers.current_draw_clause.first_count_commands, rsx::method_registers.current_draw_clause.primitive, m_index_ring_buffer); - return{ index_count, vertex_count, min_index, 0, std::make_tuple(static_cast(GL_UNSIGNED_SHORT), offset_in_index_buffer) }; + return{ index_count, vertex_count, min_index, 0, std::make_tuple(GL_UNSIGNED_SHORT, offset_in_index_buffer) }; } return{ vertex_count, vertex_count, min_index, 0, std::optional>() }; @@ -116,8 +115,6 @@ namespace rsx::index_array_type type = rsx::method_registers.current_draw_clause.is_immediate_draw? rsx::index_array_type::u32: rsx::method_registers.index_type(); - - u32 type_size = ::narrow(get_index_type_size(type)); const u32 vertex_count = rsx::method_registers.current_draw_clause.get_elements_count(); u32 index_count = vertex_count; @@ -125,7 +122,7 @@ namespace if (!gl::is_primitive_native(rsx::method_registers.current_draw_clause.primitive)) index_count = (u32)get_index_count(rsx::method_registers.current_draw_clause.primitive, vertex_count); - u32 max_size = index_count * type_size; + u32 max_size = index_count * sizeof(u32); auto mapping = m_index_ring_buffer.alloc_from_heap(max_size, 256); void* ptr = mapping.first; u32 offset_in_index_buffer = mapping.second; @@ -137,7 +134,7 @@ namespace if (min_index >= max_index) { //empty set, do not draw - return{ 0, 0, 0, 0, std::make_tuple(get_index_type(type), offset_in_index_buffer) }; + return{ 0, 0, 0, 0, std::make_tuple(GL_UNSIGNED_INT, offset_in_index_buffer) }; } //check for vertex arrays with frequency modifiers @@ -147,13 +144,13 @@ namespace { //Ignore base offsets and return real results //The upload function will optimize the uploaded range anyway - return{ index_count, max_index, 0, 0, std::make_tuple(get_index_type(type), offset_in_index_buffer) }; + return{ index_count, max_index, 0, 0, std::make_tuple(GL_UNSIGNED_INT, offset_in_index_buffer) }; } } //Prefer only reading the vertices that are referenced in the index buffer itself //Offset data source by min_index verts, but also notify the shader to offset the vertexID - return{ index_count, (max_index - min_index + 1), min_index, min_index, std::make_tuple(get_index_type(type), offset_in_index_buffer) }; + return{ index_count, (max_index - min_index + 1), min_index, min_index, std::make_tuple(GL_UNSIGNED_INT, offset_in_index_buffer) }; } vertex_input_state operator()(const rsx::draw_inlined_array& command) @@ -168,7 +165,7 @@ namespace { std::make_pair(0, vertex_count) }, rsx::method_registers.current_draw_clause.primitive, m_index_ring_buffer); - return{ index_count, vertex_count, 0, 0, std::make_tuple(static_cast(GL_UNSIGNED_SHORT), offset_in_index_buffer) }; + return{ index_count, vertex_count, 0, 0, std::make_tuple(GL_UNSIGNED_SHORT, offset_in_index_buffer) }; } return{ vertex_count, vertex_count, 0, 0, std::optional>() }; diff --git a/rpcs3/Emu/RSX/VK/VKVertexBuffers.cpp b/rpcs3/Emu/RSX/VK/VKVertexBuffers.cpp index 1ea2f62296..5a7da386ff 100644 --- a/rpcs3/Emu/RSX/VK/VKVertexBuffers.cpp +++ b/rpcs3/Emu/RSX/VK/VKVertexBuffers.cpp @@ -132,7 +132,7 @@ namespace rsx::index_array_type::u32 : rsx::method_registers.index_type(); - u32 type_size = gsl::narrow(get_index_type_size(index_type)); + constexpr u32 type_size = sizeof(u32); // Force u32 index size dest to avoid overflows when adding vertex base index u32 index_count = rsx::method_registers.current_draw_clause.get_elements_count(); if (primitives_emulated) @@ -177,20 +177,13 @@ namespace if (emulate_restart) { - if (index_type == rsx::index_array_type::u16) - { - index_count = rsx::remove_restart_index((u16*)buf, (u16*)tmp.data(), index_count, (u16)UINT16_MAX); - } - else - { - index_count = rsx::remove_restart_index((u32*)buf, (u32*)tmp.data(), index_count, (u32)UINT32_MAX); - } + index_count = rsx::remove_restart_index((u32*)buf, (u32*)tmp.data(), index_count, (u32)UINT32_MAX); } m_index_buffer_ring_info.unmap(); std::optional> index_info = - std::make_tuple(offset_in_index_buffer, vk::get_index_type(index_type)); + std::make_tuple(offset_in_index_buffer, VK_INDEX_TYPE_UINT32); //check for vertex arrays with frequency modifiers for (auto &block : m_vertex_layout.interleaved_blocks)