Changeset 280926 in webkit

Timestamp:

Aug 11, 2021, 1:04:36 PM (5 years ago)

Author:

Kyle Piddington

Message:

Avoid infinite recursion when command buffer creation fails
​https://bugs.webkit.org/show_bug.cgi?id=228978
<rdar://79224824>

Reviewed by Kenneth Russell.

In cases where the MTLCommandBuffer is not a valid metal object,
we can end up in an infinite recursive loop during draw call setup. Refactor setupDraw to take no more than two attempts through the setup function.

Testing: Ran WebGL tests, use case samples. Set up synthetic
repro forcing bail out path, saw WebGL content fail to render
instead of a web process crash.

• src/libANGLE/renderer/metal/ContextMtl.h:
• src/libANGLE/renderer/metal/ContextMtl.mm:

(rx::ContextMtl::setupDraw):
(rx::ContextMtl::setupDrawImpl):

Location:

trunk/Source/ThirdParty/ANGLE

Files:

• ChangeLog (modified) (1 diff)
• src/libANGLE/renderer/metal/ContextMtl.h (modified) (1 diff)
• src/libANGLE/renderer/metal/ContextMtl.mm (modified) (2 diffs)

trunk/Source/ThirdParty/ANGLE/ChangeLog

@@ +1 @@
+2021-08-11  Kyle Piddington  <kpiddington@apple.com>
+
+        Avoid infinite recursion when command buffer creation fails
+        https://bugs.webkit.org/show_bug.cgi?id=228978
+        <rdar://79224824>
+ 
+        Reviewed by Kenneth Russell.
+ 
+        In cases where the MTLCommandBuffer is not a valid metal object,
+        we can end up in an infinite recursive loop during draw call setup. Refactor setupDraw to take no more than two attempts through the setup function. 
+
+        Testing: Ran WebGL tests, use case samples. Set up synthetic 
+        repro forcing bail out path, saw WebGL content fail to render 
+        instead of a web process crash.
+
+        * src/libANGLE/renderer/metal/ContextMtl.h:
+        * src/libANGLE/renderer/metal/ContextMtl.mm:
+        (rx::ContextMtl::setupDraw):
+        (rx::ContextMtl::setupDrawImpl):
+
 2021-08-11  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ContextMtl.h

@@ -382 +382 @@
                             const void *indices,
                             bool transformFeedbackDraw);
+    angle::Result setupDrawImpl(const gl::Context *context,
+                            gl::PrimitiveMode mode,
+                            GLint firstVertex,
+                            GLsizei vertexOrIndexCount,
+                            GLsizei instanceCount,
+                            gl::DrawElementsType indexTypeOrNone,
+                            const void *indices,
+                            bool transformFeedbackDraw);
 
     angle::Result drawTriFanArrays(const gl::Context *context,

trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ContextMtl.mm

@@ -2121 +2121 @@
                                     bool transformFeedbackDraw)
 {
+    ANGLE_TRY(setupDrawImpl(context, mode, firstVertex, vertexOrIndexCount, instances, indexTypeOrNone, indices, transformFeedbackDraw));
+    // Setting up the draw required us to call a command buffer flush, re-run setupDraw with state invaliated to restart the command buffer from the current draw with previously set state
+    if (!mCmdBuffer.valid())
+    {
+        invalidateState(context);
+        ANGLE_TRY(setupDrawImpl(context, mode, firstVertex, vertexOrIndexCount, instances, indexTypeOrNone, indices, transformFeedbackDraw));
+    }
+    // If the command buffer still isn't valid after a second attempt, we have a problem and should stop the draw call to avoid infinite recursion.
+    if(!mCmdBuffer.valid())
+    {
+        return angle::Result::Stop;
+    }
+    return angle::Result::Continue;
+    
+}
+angle::Result ContextMtl::setupDrawImpl(const gl::Context *context,
+                                    gl::PrimitiveMode mode,
+                                    GLint firstVertex,
+                                    GLsizei vertexOrIndexCount,
+                                    GLsizei instances,
+                                    gl::DrawElementsType indexTypeOrNone,
+                                    const void *indices,
+                                    bool transformFeedbackDraw)
+{
     ASSERT(mProgram);
 
@@ -2250 +2274 @@
                                   uniformBuffersDirty, transformFeedbackDraw));
 
-    // Setting up the draw required us to call a command buffer flush, re-run setupDraw with state invaliated to restart the command buffer from the current draw with previously set state
-    if (!mCmdBuffer.valid())
-    {
-        invalidateState(context);
-        ANGLE_TRY(setupDraw(context, mode, firstVertex, vertexOrIndexCount, instances, indexTypeOrNone, indices, transformFeedbackDraw));
-    }
+  
     mDirtyBits.reset();
     return angle::Result::Continue;