[Mesa-dev] [PATCH] etnaviv: Avoid infinite loop in find_frame()

[Christian Gmeiner]

2017-01-30 15:47 GMT+01:00 Rhys Kidd <rhyskidd at gmail.com>:
>
> On Sun, Jan 22, 2017 at 1:49 PM Rhys Kidd <rhyskidd at gmail.com> wrote:
>>
>> On 20 January 2017 at 17:06, Christian Gmeiner
>> <christian.gmeiner at gmail.com> wrote:
>>>
>>> Hi Rhys,
>>>
>>> 2017-01-19 7:02 GMT+01:00 Rhys Kidd <rhyskidd at gmail.com>:
>>> > Use of unsigned loop control variable with '>= 0' would lead to
>>> > infinite loop.
>>> >
>>> > Reported by clang:
>>> >
>>> > etnaviv_compiler.c:1024:39: warning: comparison of unsigned expression
>>> > >= 0 is
>>> >       always true [-Wtautological-compare]
>>> >    for (unsigned sp = c->frame_sp; sp >= 0; sp--)
>>> >                                    ~~ ^  ~
>>> >
>>> > Signed-off-by: Rhys Kidd <rhyskidd at gmail.com>
>>> > ---
>>> >  src/gallium/drivers/etnaviv/etnaviv_compiler.c | 2 +-
>>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>>> >
>>> > I compile tested etnaviv but have not functionally tested on real
>>> > hardware.
>>> >
>>> > diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> > b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> > index 59e1452..f1c6787 100644
>>> > --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> > +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> > @@ -1021,7 +1021,7 @@ label_mark_use(struct etna_compile *c, struct
>>> > etna_compile_label *label)
>>> >  static struct etna_compile_frame *
>>> >  find_frame(struct etna_compile *c, enum etna_compile_frame_type type)
>>> >  {
>>> > -   for (unsigned sp = c->frame_sp; sp >= 0; sp--)
>>> > +   for (unsigned sp = c->frame_sp + 1; sp-- > 0 ; )
>>>
>>> looks quite ugly - I have a better and simpler fix.
>>>
>>> >        if (c->frame_stack[sp].type == type)
>>> >           return &c->frame_stack[sp];
>>> >
>>> > --
>>> > 2.9.3
>>> >
>>>
>>> ------------8<-------------------
>>>
>>> Subject: [PATCH] etnaviv: Avoid infinite loop in find_frame()
>>>
>>> Use of unsigned loop control variable with '>= 0' would lead to infinite
>>> loop.
>>>
>>> Reported by clang:
>>>
>>> etnaviv_compiler.c:1024:39: warning: comparison of unsigned expression
>>> >= 0 is always true [-Wtautological-compare]
>>>    for (unsigned sp = c->frame_sp; sp >= 0; sp--)
>>>                                    ~~ ^  ~
>>>
>>> v2: Simply use the same datatype as c->frame_sp is using.
>>>
>>> Reported-by: Rhys Kidd <rhyskidd at gmail.com>
>>> Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com>
>>> ---
>>>  src/gallium/drivers/etnaviv/etnaviv_compiler.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> index 59e1452..dc9af57 100644
>>> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
>>> @@ -1021,7 +1021,7 @@ label_mark_use(struct etna_compile *c, struct
>>> etna_compile_label *label)
>>>  static struct etna_compile_frame *
>>>  find_frame(struct etna_compile *c, enum etna_compile_frame_type type)
>>>  {
>>> -   for (unsigned sp = c->frame_sp; sp >= 0; sp--)
>>> +   for (int sp = c->frame_sp; sp >= 0; sp--)
>>>        if (c->frame_stack[sp].type == type)
>>>           return &c->frame_stack[sp];
>>>
>>> --
>>> 2.9.3
>>> ------------8<-------------------
>>>
>>> If you are okay with that I will push it in the next days.
>>>
>>
>> Reviewed-by: Rhys Kidd <rhyskidd at gmail.com>
>
>
> Hello Christian,
>
> I don't think this change made it to master as yet.
>

Correct... had some important private stuff to manage the last week. It
will land later the day.

greets
--
Christian Gmeiner, MSc

https://www.youtube.com/user/AloryOFFICIAL
https://soundcloud.com/christian-gmeiner