Fix incorrect indexing / out of bounds access in getRestartIndices
Created attachment 432008 [details] Patch
<rdar://problem/79244789>
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment on attachment 432008 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432008&action=review > Source/ThirdParty/ANGLE/ChangeLog:8 > + In the case where a restart index is present in the last element of an index array, we can accidently > + access out of bounds. Fix this by checking boundaries, and fixing other indexing errors Do we need a new test, or did the existing tests pick this up?
(In reply to Dean Jackson from comment #4) > Comment on attachment 432008 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=432008&action=review > > > Source/ThirdParty/ANGLE/ChangeLog:8 > > + In the case where a restart index is present in the last element of an index array, we can accidently > > + access out of bounds. Fix this by checking boundaries, and fixing other indexing errors > > Do we need a new test, or did the existing tests pick this up? The existing tests should hit this path, but would probably only trigger on ASAN builds.
Committed r279213 (239099@main): <https://commits.webkit.org/239099@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432008 [details].
Comment on attachment 432008 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432008&action=review > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:418 > T *bufferData = (T*)(idxBuffer->mapReadOnly(ctx)); do you need unmap? > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:420 > + for(int i = 0; i < numIndices; i++) I think the algorithm creates many 1 length ranges for contents like F,F,F,F IndexRange is inclusive and as such cannot represent zero length ranges. > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:436 > } template<typename T> static void calculateRestartRanges(ContextMtl* ctx, mtl::BufferRef idxBuffer, std::vector<IndexRange> * ranges) { ranges->clear(); T *bufferData = reinterpret_cast<T*>(idxBuffer->mapReadOnly(ctx)); constexpr size_t indexCount = idxBuffer->size() / sizeof(T) constexpr size_t restartMarker = std::numeric_limits<T>::max(); for(size_t i = 0; i < indexCount;) { if (bufferData[i] == restartMarker) { ++i; continue; } size_t restartBegin = i; ++i; while (i < indexCount && bufferData[i] != restartMarker) { ++i; } size_t restartEnd = i - 1; ranges->emplace_back(restartBegin, restartEnd); } idxBuffer->unmap(); } I'd write something like that, but I'm the last person to correct anybody's off-by-ones.