[Mesa-dev] [PATCH 07/18] intel/compiler: fix for memmove argument on annotating error

[Matt Turner]

On Mon, Nov 13, 2017 at 5:17 AM,  <kevin.rogovin at intel.com> wrote:
> From: Kevin Rogovin <kevin.rogovin at 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.rogovin at 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?