rsx: Fix u16 index arrays overflow

Force u32 index array destinations to avoid overflows when adding vertex base index.
This commit is contained in:
eladash 2018-10-06 14:54:47 +03:00 committed by kd-11
parent e361e0daa6
commit 83b6c98563
5 changed files with 39 additions and 50 deletions

View file

@ -236,12 +236,12 @@ namespace rsx
auto fifo = vm::ptr<u16>::make(idxAddr); auto fifo = vm::ptr<u16>::make(idxAddr);
for (u32 i = 0; i < idxCount; ++i) for (u32 i = 0; i < idxCount; ++i)
{ {
u16 index = fifo[i]; u32 index = fifo[i];
if (is_primitive_restart_enabled && (u32)index == primitive_restart_index) if (is_primitive_restart_enabled && index == primitive_restart_index)
continue; continue;
index = (u16)get_index_from_base(index, method_registers.vertex_data_base_index()); index = get_index_from_base(index, method_registers.vertex_data_base_index());
min_index = (u16)std::min(index, (u16)min_index); min_index = std::min(index, min_index);
max_index = (u16)std::max(index, (u16)max_index); max_index = std::max(index, max_index);
} }
break; break;
} }

View file

@ -539,10 +539,10 @@ void write_vertex_array_data_to_buffer(gsl::span<gsl::byte> raw_dst_span, gsl::s
namespace namespace
{ {
template<typename T> template<typename T>
std::tuple<T, T, u32> upload_untouched(gsl::span<to_be_t<const T>> src, gsl::span<T> dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) std::tuple<u32, u32, u32> upload_untouched(gsl::span<to_be_t<const T>> src, gsl::span<u32> dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index)
{ {
T min_index = -1; u32 min_index = -1;
T max_index = 0; u32 max_index = 0;
verify(HERE), (dst.size_bytes() >= src.size_bytes()); verify(HERE), (dst.size_bytes() >= src.size_bytes());
@ -555,27 +555,26 @@ std::tuple<T, T, u32> upload_untouched(gsl::span<to_be_t<const T>> src, gsl::spa
if (rsx::method_registers.current_draw_clause.is_disjoint_primitive) if (rsx::method_registers.current_draw_clause.is_disjoint_primitive)
continue; continue;
index = -1; dst[dst_idx++] = -1u;
} }
else else
{ {
index = rsx::get_index_from_base(index, base_index); const u32 new_index = rsx::get_index_from_base((u32)index, base_index);
max_index = std::max(max_index, index); max_index = std::max(max_index, new_index);
min_index = std::min(min_index, 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); return std::make_tuple(min_index, max_index, dst_idx);
} }
template<typename T> template<typename T>
std::tuple<T, T, u32> expand_indexed_triangle_fan(gsl::span<to_be_t<const T>> src, gsl::span<T> dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) std::tuple<u32, u32, u32> expand_indexed_triangle_fan(gsl::span<to_be_t<const T>> src, gsl::span<u32> 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; u32 min_index = invalid_index;
T max_index = 0; u32 max_index = 0;
verify(HERE), (dst.size() >= 3 * (src.size() - 2)); verify(HERE), (dst.size() >= 3 * (src.size() - 2));
@ -583,12 +582,12 @@ std::tuple<T, T, u32> expand_indexed_triangle_fan(gsl::span<to_be_t<const T>> sr
u32 src_idx = 0; u32 src_idx = 0;
bool needs_anchor = true; bool needs_anchor = true;
T anchor = invalid_index; u32 anchor = invalid_index;
T last_index = invalid_index; u32 last_index = invalid_index;
for (size_t src_idx = 0; src_idx < src.size(); ++src_idx) 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); index = rsx::get_index_from_base(index, base_index);
if (needs_anchor) if (needs_anchor)
@ -629,20 +628,20 @@ std::tuple<T, T, u32> expand_indexed_triangle_fan(gsl::span<to_be_t<const T>> sr
} }
template<typename T> template<typename T>
std::tuple<T, T, u32> expand_indexed_quads(gsl::span<to_be_t<const T>> src, gsl::span<T> dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index) std::tuple<u32, u32, u32> expand_indexed_quads(gsl::span<to_be_t<const T>> src, gsl::span<u32> dst, bool is_primitive_restart_enabled, u32 primitive_restart_index, u32 base_index)
{ {
T min_index = -1; u32 min_index = -1;
T max_index = 0; u32 max_index = 0;
verify(HERE), (4 * dst.size_bytes() >= 6 * src.size_bytes()); verify(HERE), (4 * dst.size_bytes() >= 6 * src.size_bytes());
u32 dst_idx = 0; u32 dst_idx = 0;
u8 set_size = 0; u8 set_size = 0;
T tmp_indices[4]; u32 tmp_indices[4];
for (int src_idx = 0; src_idx < src.size(); ++src_idx) 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); index = rsx::get_index_from_base(index, base_index);
if (is_primitive_restart_enabled && (u32)src[src_idx] == primitive_restart_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 ? // TODO: Unify indexed and non indexed primitive expansion ?
template<typename T> template<typename T>
std::tuple<T, T, u32> write_index_array_data_to_buffer_impl(gsl::span<T> dst, std::tuple<u32, u32, u32> write_index_array_data_to_buffer_impl(gsl::span<u32> dst,
gsl::span<const be_t<T>> src, gsl::span<const be_t<T>> src,
rsx::primitive_type draw_mode, bool restart_index_enabled, u32 restart_index, const std::vector<std::pair<u32, u32> > &first_count_arguments, rsx::primitive_type draw_mode, bool restart_index_enabled, u32 restart_index, const std::vector<std::pair<u32, u32> > &first_count_arguments,
u32 base_index, std::function<bool(rsx::primitive_type)> expands) u32 base_index, std::function<bool(rsx::primitive_type)> expands)
@ -833,7 +832,7 @@ std::tuple<u32, u32, u32> write_index_array_data_to_buffer(gsl::span<gsl::byte>
switch (type) switch (type)
{ {
case rsx::index_array_type::u16: case rsx::index_array_type::u16:
return write_index_array_data_to_buffer_impl<u16>(as_span_workaround<u16>(dst), return write_index_array_data_to_buffer_impl<u16>(as_span_workaround<u32>(dst),
as_const_span<const be_t<u16>>(src), draw_mode, restart_index_enabled, restart_index, first_count_arguments, base_index, expands); as_const_span<const be_t<u16>>(src), draw_mode, restart_index_enabled, restart_index, first_count_arguments, base_index, expands);
case rsx::index_array_type::u32: case rsx::index_array_type::u32:
return write_index_array_data_to_buffer_impl<u32>(as_span_workaround<u32>(dst), return write_index_array_data_to_buffer_impl<u32>(as_span_workaround<u32>(dst),

View file

@ -395,7 +395,7 @@ namespace
rsx::index_array_type::u32: rsx::index_array_type::u32:
rsx::method_registers.index_type(); 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 // Alloc
size_t buffer_size = align(index_count * index_size, 64); 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)); m_buffer_data.unmap(CD3DX12_RANGE(heap_offset, heap_offset + buffer_size));
D3D12_INDEX_BUFFER_VIEW index_buffer_view = { D3D12_INDEX_BUFFER_VIEW index_buffer_view = {
m_buffer_data.get_heap()->GetGPUVirtualAddress() + heap_offset, (UINT)buffer_size, 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; // m_timers.buffer_upload_size += buffer_size;
command_list->IASetIndexBuffer(&index_buffer_view); command_list->IASetIndexBuffer(&index_buffer_view);

View file

@ -47,8 +47,7 @@ namespace
if (!gl::is_primitive_native(draw_mode)) if (!gl::is_primitive_native(draw_mode))
vertex_draw_count = (u32)get_index_count(draw_mode, ::narrow<int>(vertex_draw_count)); vertex_draw_count = (u32)get_index_count(draw_mode, ::narrow<int>(vertex_draw_count));
u32 type_size = ::narrow<u32>(get_index_type_size(type)); u32 block_sz = vertex_draw_count * sizeof(u32); // Force u32 index size dest to avoid overflows when adding vertex base index
u32 block_sz = vertex_draw_count * type_size;
gsl::span<gsl::byte> dst{ reinterpret_cast<gsl::byte*>(ptr), ::narrow<u32>(block_sz) }; gsl::span<gsl::byte> dst{ reinterpret_cast<gsl::byte*>(ptr), ::narrow<u32>(block_sz) };
std::tie(min_index, max_index, vertex_draw_count) = write_index_array_data_to_buffer(dst, raw_index_buffer, 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.first_count_commands,
rsx::method_registers.current_draw_clause.primitive, m_index_ring_buffer); rsx::method_registers.current_draw_clause.primitive, m_index_ring_buffer);
return{ index_count, vertex_count, min_index, 0, std::make_tuple(static_cast<GLenum>(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<std::tuple<GLenum, u32>>() }; return{ vertex_count, vertex_count, min_index, 0, std::optional<std::tuple<GLenum, u32>>() };
@ -116,8 +115,6 @@ namespace
rsx::index_array_type type = rsx::method_registers.current_draw_clause.is_immediate_draw? rsx::index_array_type type = rsx::method_registers.current_draw_clause.is_immediate_draw?
rsx::index_array_type::u32: rsx::index_array_type::u32:
rsx::method_registers.index_type(); rsx::method_registers.index_type();
u32 type_size = ::narrow<u32>(get_index_type_size(type));
const u32 vertex_count = rsx::method_registers.current_draw_clause.get_elements_count(); const u32 vertex_count = rsx::method_registers.current_draw_clause.get_elements_count();
u32 index_count = vertex_count; u32 index_count = vertex_count;
@ -125,7 +122,7 @@ namespace
if (!gl::is_primitive_native(rsx::method_registers.current_draw_clause.primitive)) 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); 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); auto mapping = m_index_ring_buffer.alloc_from_heap(max_size, 256);
void* ptr = mapping.first; void* ptr = mapping.first;
u32 offset_in_index_buffer = mapping.second; u32 offset_in_index_buffer = mapping.second;
@ -137,7 +134,7 @@ namespace
if (min_index >= max_index) if (min_index >= max_index)
{ {
//empty set, do not draw //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 //check for vertex arrays with frequency modifiers
@ -147,13 +144,13 @@ namespace
{ {
//Ignore base offsets and return real results //Ignore base offsets and return real results
//The upload function will optimize the uploaded range anyway //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 //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 //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) vertex_input_state operator()(const rsx::draw_inlined_array& command)
@ -168,7 +165,7 @@ namespace
{ std::make_pair(0, vertex_count) }, { std::make_pair(0, vertex_count) },
rsx::method_registers.current_draw_clause.primitive, m_index_ring_buffer); rsx::method_registers.current_draw_clause.primitive, m_index_ring_buffer);
return{ index_count, vertex_count, 0, 0, std::make_tuple(static_cast<GLenum>(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<std::tuple<GLenum, u32>>() }; return{ vertex_count, vertex_count, 0, 0, std::optional<std::tuple<GLenum, u32>>() };

View file

@ -132,7 +132,7 @@ namespace
rsx::index_array_type::u32 : rsx::index_array_type::u32 :
rsx::method_registers.index_type(); rsx::method_registers.index_type();
u32 type_size = gsl::narrow<u32>(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(); u32 index_count = rsx::method_registers.current_draw_clause.get_elements_count();
if (primitives_emulated) if (primitives_emulated)
@ -177,20 +177,13 @@ namespace
if (emulate_restart) if (emulate_restart)
{ {
if (index_type == rsx::index_array_type::u16) index_count = rsx::remove_restart_index((u32*)buf, (u32*)tmp.data(), index_count, (u32)UINT32_MAX);
{
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);
}
} }
m_index_buffer_ring_info.unmap(); m_index_buffer_ring_info.unmap();
std::optional<std::tuple<VkDeviceSize, VkIndexType>> index_info = std::optional<std::tuple<VkDeviceSize, VkIndexType>> 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 //check for vertex arrays with frequency modifiers
for (auto &block : m_vertex_layout.interleaved_blocks) for (auto &block : m_vertex_layout.interleaved_blocks)