Re: [Mesa-dev] [PATCH 2/2] gallium/u_vbuf: Protect against overflow with large instance divisors.

2018-03-22 Thread Brian Paul

Reviewed-by: Brian Paul 

On 03/22/2018 04:18 PM, Eric Anholt wrote:

GTF-GLES3.gtf.GL3Tests.instanced_arrays.instanced_arrays_divisor uses -1
as a divisor, so we would overflow to count=0 and upload no data,
triggering the assert below.  We want to upload 1 element in this case,
fixing the test on VC5.

v2: Use some more obvious logic, and explain why we don't use the normal
 round_up().
---
  src/gallium/auxiliary/util/u_vbuf.c | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/util/u_vbuf.c 
b/src/gallium/auxiliary/util/u_vbuf.c
index 95d7990c6ca4..8a680d60a687 100644
--- a/src/gallium/auxiliary/util/u_vbuf.c
+++ b/src/gallium/auxiliary/util/u_vbuf.c
@@ -936,7 +936,16 @@ u_vbuf_upload_buffers(struct u_vbuf *mgr,
   size = mgr->ve->src_format_size[i];
} else if (instance_div) {
   /* Per-instance attrib. */
- unsigned count = (num_instances + instance_div - 1) / instance_div;
+
+ /* Figure out how many instances we'll render given instance_div.  We
+  * can't use the typical div_round_up() pattern because the CTS uses
+  * instance_div = ~0 for a test, which overflows div_round_up()'s
+  * addition.
+  */
+ unsigned count = num_instances / instance_div;
+ if (count * instance_div != num_instances)
+count++;
+
   first += vb->stride * start_instance;
   size = vb->stride * (count - 1) + mgr->ve->src_format_size[i];
} else {



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


[Mesa-dev] [PATCH 2/2] gallium/u_vbuf: Protect against overflow with large instance divisors.

2018-03-22 Thread Eric Anholt
GTF-GLES3.gtf.GL3Tests.instanced_arrays.instanced_arrays_divisor uses -1
as a divisor, so we would overflow to count=0 and upload no data,
triggering the assert below.  We want to upload 1 element in this case,
fixing the test on VC5.

v2: Use some more obvious logic, and explain why we don't use the normal
round_up().
---
 src/gallium/auxiliary/util/u_vbuf.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/util/u_vbuf.c 
b/src/gallium/auxiliary/util/u_vbuf.c
index 95d7990c6ca4..8a680d60a687 100644
--- a/src/gallium/auxiliary/util/u_vbuf.c
+++ b/src/gallium/auxiliary/util/u_vbuf.c
@@ -936,7 +936,16 @@ u_vbuf_upload_buffers(struct u_vbuf *mgr,
  size = mgr->ve->src_format_size[i];
   } else if (instance_div) {
  /* Per-instance attrib. */
- unsigned count = (num_instances + instance_div - 1) / instance_div;
+
+ /* Figure out how many instances we'll render given instance_div.  We
+  * can't use the typical div_round_up() pattern because the CTS uses
+  * instance_div = ~0 for a test, which overflows div_round_up()'s
+  * addition.
+  */
+ unsigned count = num_instances / instance_div;
+ if (count * instance_div != num_instances)
+count++;
+
  first += vb->stride * start_instance;
  size = vb->stride * (count - 1) + mgr->ve->src_format_size[i];
   } else {
-- 
2.16.2

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