[Mesa-dev] [PATCH] gallium: fixed modulo zero crashes in tgsi interpreter

[Brian Paul]

Marius,

As long as you're working on this, would you review 
src/gallium/docs/source/tgsi.rst to check if all the div/mod 
instructions document div/mod by zero behavior?  Thanks.

-Brian


On 06/08/2017 11:10 AM, Roland Scheidegger wrote:
> I don't really know if it makes sense to have different "error values"
> for signed vs. unsigned modulo 0 - maybe the "all bits set" approach
> would do too (the gallivm code does this, because it is actually
> easier). But since it's undefined in any case pretty much everywhere, I
> suppose any value will do (only the 32bit umod really has a requirement
> for all bits set due do d3d10 requirements, but d3d has neither signed
> nor 64bit versions of it).
> And I don't know why it would only crash in geometry shaders...
>
> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>
>
> Am 08.06.2017 um 18:28 schrieb Marius Gräfe:
>> softpipe throws integer division by zero exceptions on windows
>> when using % with integers in a geometry shader.
>> ---
>>   src/gallium/auxiliary/tgsi/tgsi_exec.c | 24 ++++++++++++------------
>>   1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
>> index c41954c..abd2d16 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
>> @@ -866,20 +866,20 @@ static void
>>   micro_u64mod(union tgsi_double_channel *dst,
>>                const union tgsi_double_channel *src)
>>   {
>> -   dst->u64[0] = src[0].u64[0] % src[1].u64[0];
>> -   dst->u64[1] = src[0].u64[1] % src[1].u64[1];
>> -   dst->u64[2] = src[0].u64[2] % src[1].u64[2];
>> -   dst->u64[3] = src[0].u64[3] % src[1].u64[3];
>> +   dst->u64[0] = src[1].u64[0] ? src[0].u64[0] % src[1].u64[0] : UINT64_MAX;
>> +   dst->u64[1] = src[1].u64[1] ? src[0].u64[1] % src[1].u64[1] : UINT64_MAX;
>> +   dst->u64[2] = src[1].u64[2] ? src[0].u64[2] % src[1].u64[2] : UINT64_MAX;
>> +   dst->u64[3] = src[1].u64[3] ? src[0].u64[3] % src[1].u64[3] : UINT64_MAX;
>>   }
>>
>>   static void
>>   micro_i64mod(union tgsi_double_channel *dst,
>>                const union tgsi_double_channel *src)
>>   {
>> -   dst->i64[0] = src[0].i64[0] % src[1].i64[0];
>> -   dst->i64[1] = src[0].i64[1] % src[1].i64[1];
>> -   dst->i64[2] = src[0].i64[2] % src[1].i64[2];
>> -   dst->i64[3] = src[0].i64[3] % src[1].i64[3];
>> +   dst->i64[0] = src[1].i64[0] ? src[0].i64[0] % src[1].i64[0] : INT64_MAX;
>> +   dst->i64[1] = src[1].i64[1] ? src[0].i64[1] % src[1].i64[1] : INT64_MAX;
>> +   dst->i64[2] = src[1].i64[2] ? src[0].i64[2] % src[1].i64[2] : INT64_MAX;
>> +   dst->i64[3] = src[1].i64[3] ? src[0].i64[3] % src[1].i64[3] : INT64_MAX;
>>   }
>>
>>   static void
>> @@ -4653,10 +4653,10 @@ micro_mod(union tgsi_exec_channel *dst,
>>             const union tgsi_exec_channel *src0,
>>             const union tgsi_exec_channel *src1)
>>   {
>> -   dst->i[0] = src0->i[0] % src1->i[0];
>> -   dst->i[1] = src0->i[1] % src1->i[1];
>> -   dst->i[2] = src0->i[2] % src1->i[2];
>> -   dst->i[3] = src0->i[3] % src1->i[3];
>> +   dst->i[0] = src1->i[0] ? src0->i[0] % src1->i[0] : INT_MAX;
>> +   dst->i[1] = src1->i[1] ? src0->i[1] % src1->i[1] : INT_MAX;
>> +   dst->i[2] = src1->i[2] ? src0->i[2] % src1->i[2] : INT_MAX;
>> +   dst->i[3] = src1->i[3] ? src0->i[3] % src1->i[3] : INT_MAX;
>>   }
>>
>>   static void
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>