Re: [patch] workaround gcc46: prologue skip skips too far (PR 12435) #2

[Doug Evans <dje@google.com>]

On Fri, Jul 22, 2011 at 2:58 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> this is an improved patch of a former:
>        [patch] workaround gcc46: prologue skip skips too far (PR 12435)
>        http://sourceware.org/ml/gdb-patches/2011-03/msg01108.html
>        cancel/FYI: Re: [patch] workaround gcc46: prologue skip skips too far (PR 12435)
>        http://sourceware.org/ml/gdb-patches/2011-03/msg01123.html
>
> [...]
>
> I will update the code after FSF gcc gets fixed to minimize the workaround.

I realize this is checked in, but a couple of comments for maybe the
next patch for this.

> [...]
>
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -1902,6 +1902,9 @@ amd64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
>  {
>   struct amd64_frame_cache cache;
>   CORE_ADDR pc;
> +  struct symtab_and_line start_pc_sal, next_sal;
> +  gdb_byte buf[4 + 8 * 7];
> +  int offset, xmmreg;
>
>   amd64_init_frame_cache (&cache);
>   pc = amd64_analyze_prologue (gdbarch, start_pc, 0xffffffffffffffffLL,
> @@ -1909,7 +1912,71 @@ amd64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
>   if (cache.frameless_p)
>     return start_pc;
>
> -  return pc;
> +  /* GCC PR debug/48827 produced false prologue end:
> +     84 c0                test   %al,%al
> +     74 23                je     after
> +     <-- here is 0 lines advance - the false prologue end marker.
> +     0f 29 85 70 ff ff ff movaps %xmm0,-0x90(%rbp)
> +     0f 29 4d 80          movaps %xmm1,-0x80(%rbp)
> +     0f 29 55 90          movaps %xmm2,-0x70(%rbp)
> +     0f 29 5d a0          movaps %xmm3,-0x60(%rbp)
> +     0f 29 65 b0          movaps %xmm4,-0x50(%rbp)
> +     0f 29 6d c0          movaps %xmm5,-0x40(%rbp)
> +     0f 29 75 d0          movaps %xmm6,-0x30(%rbp)
> +     0f 29 7d e0          movaps %xmm7,-0x20(%rbp)
> +     after:  */
> +
> +  if (pc == start_pc)
> +    return pc;
> +
> +  start_pc_sal = find_pc_sect_line (start_pc, NULL, 0);
> +  if (start_pc_sal.symtab == NULL
> +      || !start_pc_sal.symtab->amd64_prologue_line_bug
> +      || start_pc_sal.pc != start_pc || pc >= start_pc_sal.end)
> +    return pc;
> +
> +  next_sal = find_pc_sect_line (start_pc_sal.end, NULL, 0);
> +  if (next_sal.line != start_pc_sal.line)
> +    return pc;
> +
> +  /* START_PC can be from overlayed memory, ignored here.  */
> +  if (target_read_memory (next_sal.pc - 4, buf, sizeof (buf)) != 0)
> +    return pc;
> +
> +  /* test %al,%al */
> +  if (buf[0] != 0x84 || buf[1] != 0xc0)
> +    return pc;
> +  /* je AFTER */
> +  if (buf[2] != 0x74)
> +    return pc;
> +
> +  offset = 4;
> +  for (xmmreg = 0; xmmreg < 8; xmmreg++)
> +    {
> +      /* movaps %xmmreg?,-0x??(%rbp) */
> +      if (buf[offset] != 0x0f || buf[offset + 1] != 0x29
> +          || (buf[offset + 2] & 0b00111111) != (xmmreg << 3 | 0b101))
> +       return pc;
> +
> +      if ((buf[offset + 2] & 0b11000000) == 0b01000000)
> +       {
> +         /* 8-bit displacement.  */
> +         offset += 4;
> +       }
> +      else if ((buf[offset + 2] & 0b11000000) == 0b10000000)
> +       {
> +         /* 32-bit displacement.  */
> +         offset += 7;
> +       }
> +      else
> +       return pc;
> +    }
> +
> +  /* je AFTER */
> +  if (offset - 4 != buf[3])
> +    return pc;
> +
> +  return next_sal.end;
>  }

These kinds of functions tend to grow and grow and become hard to maintain.
IWBN if all this new code is about a specific issue then tuck it away
in its own function.
[Or at least document both the start and end of it.]

> [...]
>
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -784,6 +784,11 @@ struct symtab
>
>   unsigned int epilogue_unwind_valid : 1;
>
> +  /* At least GCC 4.6.0 and 4.6.1 can produce invalid false prologue and marker
> +     on amd64.  This flag is set independently of the symtab arch.  */
> +
> +  unsigned amd64_prologue_line_bug : 1;
> +
>   /* The macro table for this symtab.  Like the blockvector, this
>      may be shared between different symtabs --- and normally is for
>      all the symtabs in a given compilation unit.  */

I can hear the screams of pushback if a random net person wanted to
check in anything architecture specific to the architecture
independent part of gdb. :-)