On Tue, Aug 14, 2018 at 03:36:18PM +0100, Lionel Landwerlin wrote: > On 14/08/18 12:55, asimiklit.work wrote: > > Hi Lionel, > > > Hi Andrii, > > > > > > Again sorry, I don't think this is the right fix. > > > I'm sending another patch to fix the parsing of > > > MI_BATCH_BUFFER_START which seems to be the actual issue. > > > > > > Thanks for working on this, > > Thanks for your fast reply. > > I agree that it is not correct patch for this issue but anyway > > "iter_more_groups" function will still work incorrectly > > for unknown instructions when the "iter->group->variable" field is true. > > I guess that this case should be fixed. > > Please let me know if I am incorrect. > > Hey Andrii, > > We shouldn't even get to use the iterator if it's an unknown instruction. > The decoder should just advance dword by dword until it finds something that > makes sense again. > > If we run into that problem, I think we should fix the caller.
In that case, would an unreachable() or assert be a good thing to do? > > > > > Regards, > > Andrii. > > > > On 2018-08-14 1:26 PM, Lionel Landwerlin wrote: > > > Hi Andrii, > > > > > > Again sorry, I don't think this is the right fix. > > > I'm sending another patch to fix the parsing of > > > MI_BATCH_BUFFER_START which seems to be the actual issue. > > > > > > Thanks for working on this, > > > > > > - > > > Lionel > > > > > > On 14/08/18 10:04, asimiklit.w...@gmail.com wrote: > > > > From: Andrii Simiklit <asimiklit.w...@gmail.com> > > > > > > > > The "gen_group_get_length" function can return a negative value > > > > and it can lead to the out of bounds group_iter. > > > > > > > > v2: printing of "unknown command type" was added > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544 > > > > Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com> > > > > --- > > > > src/intel/common/gen_decoder.c | 13 +++++++++++-- > > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/src/intel/common/gen_decoder.c > > > > b/src/intel/common/gen_decoder.c > > > > index ec0a486..b36facf 100644 > > > > --- a/src/intel/common/gen_decoder.c > > > > +++ b/src/intel/common/gen_decoder.c > > > > @@ -770,6 +770,13 @@ gen_group_get_length(struct gen_group > > > > *group, const uint32_t *p) > > > > return -1; > > > > } > > > > } > > > > + default: { > > > > + fprintf(stderr, "Unknown command type %u in '%s::%s'\n", > > > > + type, > > > > + (group->parent && group->parent->name) ? > > > > group->parent->name : "UNKNOWN", > > > > + group->name ? group->name : "UNKNOWN"); > > > > + break; > > > > + } > > > > } > > > > return -1; > > > > @@ -803,8 +810,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; > > > > > > > > > > _______________________________________________ > > 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
