On Fri, Aug 10, 2018 at 05:37:12PM +0100, Lionel Landwerlin wrote:
> Andrey also opened a bug about this issue :
> https://bugs.freedesktop.org/show_bug.cgi?id=107544
>
> It feels like it should be fixed on master though. get_length() shouldn't
> return -1 for structs anymore.
> We should probably return 1 at end of get_length() so that the decoder
> prints out "unknown instruction".
> That would help spot potential errors and updates needed to genxml.
Yeah, that makes sense. I saw the we were doing the check for length < 0
somewhere else so it seemed reasonable to check for that, considering we
can return -1, but I agree that printing "unknown instruction" would be
better.
> -
> Lionel
>
>
> On 10/08/18 16:48, Rafael Antognolli wrote:
> > On Thu, Aug 09, 2018 at 03:00:30PM +0300, andrey simiklit wrote:
> > > Hi,
> > >
> > > Sorry I missed the main thought here.
> > > The "gen_group_get_length" function returns int
> > > but the "iter_group_offset_bits" function returns uint32_t
> > > So uint32_t(int(-32)) = 0xFFFFFFE0U and it looks like unexpected behavior
> > > for
> > > me:
> > > iter_group_offset_bits(iter, iter->group_iter + 1) < 0xFFFFFFE0U;
> > That's fine, I think the original commit message is good enough to
> > understand this change. Feel free to add this extra bit too if you want,
> > but I don't think it's needed.
> >
> > Reviewed-by: Rafael Antognolli <rafael.antogno...@intel.com>
> >
> > > Regards,
> > > Andrii.
> > >
> > > On Thu, Aug 9, 2018 at 2:35 PM, Andrii Simiklit <asimiklit.w...@gmail.com>
> > > wrote:
> > >
> > > The "gen_group_get_length" function can return a negative value
> > > and it can lead to the out of bounds group_iter.
> > >
> > > Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com>
> > > ---
> > > src/intel/common/gen_decoder.c | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/intel/common/gen_decoder.c
> > > b/src/intel/common/gen_decoder
> > > .c
> > > index ec0a486..f09bd87 100644
> > > --- a/src/intel/common/gen_decoder.c
> > > +++ b/src/intel/common/gen_decoder.c
> > > @@ -803,8 +803,10 @@ static bool
> > > iter_more_groups(const struct gen_field_iterator *iter)
> > > {
> > > if (iter->group->variable) {
> > > - return iter_group_offset_bits(iter, iter->group_iter + 1) <
> > > - (gen_group_get_length(iter->group, iter->p) * 32);
> > > + const int length = gen_group_get_length(iter->group, iter->p);
> > > + return length > 0 &&
> > > + iter_group_offset_bits(iter, iter->group_iter + 1) <
> > > + (length * 32);
> > > } else {
> > > return (iter->group_iter + 1) < iter->group->group_count ||
> > > iter->group->next != NULL;
> > > --
> > > 2.7.4
> > >
> > >
> > >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
