Re: [Mesa-dev] [PATCH v2] swr: Don't crash when encountering a VBO with stride = 0.

2017-06-16 Thread Rowley, Timothy O 
Reviewed-by: Tim Rowley 
>

On Jun 15, 2017, at 11:24 AM, Bruce Cherniak 
> wrote:

The swr driver uses vertex_buffer->stride to determine the number
of elements in a VBO. A recent change to the state-tracker made it
possible for VBO's with stride=0. This resulted in a divide by zero
crash in the driver. The solution is to use the pre-calculated vertex
element stream_pitch in this case.

This patch fixes the crash in a number of piglit and VTK tests introduced
by 17f776c27be266f2.

There are several VTK tests that still crash and need proper handling of
vertex_buffer_index.  This will come in a follow-on patch.

v2: Correctly update all parameters for VBO constants (stride = 0).
   Also fixes the remaining crashes/regressions that v1 did
   not address, without touching vertex_buffer_index.
---
src/gallium/drivers/swr/swr_state.cpp | 25 ++---
1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_state.cpp 
b/src/gallium/drivers/swr/swr_state.cpp
index 08549e51a1..316872581d 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -1247,13 +1247,24 @@ swr_update_derived(struct pipe_context *pipe,

 pitch = vb->stride;
 if (!vb->is_user_buffer) {
-/* VBO
- * size is based on buffer->width0 rather than info.max_index
- * to prevent having to validate VBO on each draw */
-size = vb->buffer.resource->width0;
-elems = size / pitch;
-partial_inbounds = size % pitch;
-min_vertex_index = 0;
+/* VBO */
+if (!pitch) {
+   /* If pitch=0 (ie vb->stride), buffer contains a single
+* constant attribute.  Use the stream_pitch which was
+* calculated during creation of vertex_elements_state for the
+* size of the attribute. */
+   size = ctx->velems->stream_pitch[i];
+   elems = 1;
+   partial_inbounds = 0;
+   min_vertex_index = 0;
+} else {
+   /* size is based on buffer->width0 rather than info.max_index
+* to prevent having to validate VBO on each draw. */
+   size = vb->buffer.resource->width0;
+   elems = size / pitch;
+   partial_inbounds = size % pitch;
+   min_vertex_index = 0;
+}

p_data = swr_resource_data(vb->buffer.resource) + vb->buffer_offset;
 } else {
--
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] swr: Don't crash when encountering a VBO with stride = 0.

2017-06-15 Thread Bruce Cherniak 
The swr driver uses vertex_buffer->stride to determine the number
of elements in a VBO. A recent change to the state-tracker made it
possible for VBO's with stride=0. This resulted in a divide by zero
crash in the driver. The solution is to use the pre-calculated vertex
element stream_pitch in this case.

This patch fixes the crash in a number of piglit and VTK tests introduced
by 17f776c27be266f2.

There are several VTK tests that still crash and need proper handling of
vertex_buffer_index.  This will come in a follow-on patch.

v2: Correctly update all parameters for VBO constants (stride = 0).
Also fixes the remaining crashes/regressions that v1 did
not address, without touching vertex_buffer_index.
---
 src/gallium/drivers/swr/swr_state.cpp | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/swr/swr_state.cpp 
b/src/gallium/drivers/swr/swr_state.cpp
index 08549e51a1..316872581d 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -1247,13 +1247,24 @@ swr_update_derived(struct pipe_context *pipe,
 
  pitch = vb->stride;
  if (!vb->is_user_buffer) {
-/* VBO
- * size is based on buffer->width0 rather than info.max_index
- * to prevent having to validate VBO on each draw */
-size = vb->buffer.resource->width0;
-elems = size / pitch;
-partial_inbounds = size % pitch;
-min_vertex_index = 0;
+/* VBO */
+if (!pitch) {
+   /* If pitch=0 (ie vb->stride), buffer contains a single
+* constant attribute.  Use the stream_pitch which was
+* calculated during creation of vertex_elements_state for the
+* size of the attribute. */
+   size = ctx->velems->stream_pitch[i];
+   elems = 1;
+   partial_inbounds = 0;
+   min_vertex_index = 0;
+} else {
+   /* size is based on buffer->width0 rather than info.max_index
+* to prevent having to validate VBO on each draw. */
+   size = vb->buffer.resource->width0;
+   elems = size / pitch;
+   partial_inbounds = size % pitch;
+   min_vertex_index = 0;
+}
 
 p_data = swr_resource_data(vb->buffer.resource) + 
vb->buffer_offset;
  } else {
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev