Hi,
I confess I am not 100% on this code and I did educated guessing what it is trying to do; I figured it was trying to insert contents at the current index i; and that ann_count is the size -after- the insert. thus I figured the memmove is to move the half open interval [i, ann_count-1) to the half open interval [i + 1, ann_count). The number of elements in the half open range [i, ann_count - 1) is given by ann_count - i - 1. I tried changing the count from ann_count - i - 1 to ann_count - i + 1 and then the disassembler crashed in annotation on the same shaders that I have had it crash on before. -Kevin -----Original Message----- From: Matt Turner [mailto:matts...@gmail.com] Sent: Monday, November 13, 2017 9:02 PM To: Rogovin, Kevin <kevin.rogo...@intel.com> Cc: mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [PATCH 07/18] intel/compiler: fix for memmove argument on annotating error On Mon, Nov 13, 2017 at 5:17 AM, <kevin.rogo...@intel.com> wrote: > From: Kevin Rogovin <kevin.rogo...@intel.com> > > Without this fix, disassembling of GEN shaders with GPU commands that > the disassembler does not know would result in errors being added to > the annotator which would crash when more than one error was added. > > Signed-off-by: Kevin Rogovin <kevin.rogo...@intel.com> > --- > src/intel/compiler/intel_asm_annotation.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/intel/compiler/intel_asm_annotation.c > b/src/intel/compiler/intel_asm_annotation.c > index b07a545a12..7aa222f04e 100644 > --- a/src/intel/compiler/intel_asm_annotation.c > +++ b/src/intel/compiler/intel_asm_annotation.c > @@ -181,8 +181,9 @@ annotation_insert_error(struct annotation_info > *annotation, unsigned offset, > continue; > > if (offset + sizeof(brw_inst) != next->offset) { > - memmove(next, cur, > - (annotation->ann_count - i + 2) * sizeof(struct > annotation)); > + int count; > + count = annotation->ann_count - i - 1; > + memmove(next, cur, count * sizeof(struct annotation)); I don't see how this can be right. Take for example a case where we have ann_count == 4. We have annotation->ann[0..4] where ann[4] is the end marker... a little surprising. We want to insert an error annotation on an instruction in ann[2] but not at the end, so we need to split ann[2]. cur = 2, next = 3. We need to memmove elements 2, 3, 4 one spot later, and then update ann[2] and ann[3] (which after the memmove is a copy of ann[2]). Count should be ann_count (4) - i (2) + 1 -> 3. The code currently says +2 instead of +1 and that seems like a bug. What do you think? _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev