Re: [patch] Do not skip prologue for -O2 -g with GCC VTA Re: [patch] Fix for PR gdb/12573

[Paul Pluzhnikov <ppluzhnikov@google.com>]

Ping?

(This fixes a crash we are actually hitting, so an upstream fix is
very desirable.)

Thanks!

On Mon, Mar 28, 2011 at 8:18 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Fri, 18 Mar 2011 18:12:46 +0100, Jan Kratochvil wrote:
>> One of the problems is that GDB tries to skip prologue even for -O2 -g code.
>> There is no such reason as with -O2 -g the debug info is correct for each
>> instructions.  With -O0 -g there are frame-related absolute addresses of
>> autovariables which is the reason GDB needs to skip the prologue to have valid
>> location of such -O0 -g autovariables.
>>
>> -O2 -g code can be detected for a Compilation Unit if there is referenced any
>> location list from that CU (suggested by GCC hackers).  In such case skipping
>> prologues should be disabled.
>
> Implementation attached.  I was told systemtap is using the same idea (I have
> ot checked the systemtap implementation).
>
> It does not fix the artificial testcase of mine and it does not fix the
> problem in general.  But it fixes the binary you (Paul) attached to PR 12573
> and the binary in the Red Hat BZ.  It is a mess GDB now chooses "random"
> addresses in the -O2 -g code and with this clean up the problem does not
> happen in the cases I am aware of.
>
> I am not sure if there should not be also a GCC DW_AT_producer check but
> I hope no compiler produces DW_AT_location as a location list not covering the
> functions prologues.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora15-linux-gnu.
>
> This is not suitable for 7.3 so late.
>
>
> Thanks,
> Jan
>
>
> gdb/
> 2011-03-28  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>        * dwarf2read.c: Include ctype.h.
>        (struct dwarf2_cu): New field has_loclist.
>        (process_full_comp_unit): Set also symtab->locations_valid.  Move the
>        symtab->language code.
>        (var_decode_location): Set cu->has_loclist.
>        * symtab.c (skip_prologue_sal): Return on LOCATIONS_VALID.
>        * symtab.h (struct symtab): Make the primary field a bitfield.  New
>        field locations_valid.
>
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -57,6 +57,7 @@
>  #include "vec.h"
>  #include "c-lang.h"
>  #include "valprint.h"
> +#include <ctype.h>
>
>  #include <fcntl.h>
>  #include "gdb_string.h"
> @@ -395,6 +396,11 @@ struct dwarf2_cu
>      DIEs for namespaces, we don't need to try to infer them
>      from mangled names.  */
>   unsigned int has_namespace_info : 1;
> +
> +  /* This CU references .debug_loc.  It means it has been compiled with
> +     optimizations and debug info together so that GDB may stop skipping the
> +     prologue.  */
> +  unsigned int has_loclist : 1;
>  };
>
>  /* Persistent data held for a compilation unit, even when not
> @@ -4626,13 +4632,15 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu)
>
>   symtab = end_symtab (highpc + baseaddr, objfile, SECT_OFF_TEXT (objfile));
>
> -  /* Set symtab language to language from DW_AT_language.
> -     If the compilation is from a C file generated by language preprocessors,
> -     do not set the language if it was already deduced by start_subfile.  */
> -  if (symtab != NULL
> -      && !(cu->language == language_c && symtab->language != language_c))
> +  if (symtab != NULL)
>     {
> -      symtab->language = cu->language;
> +      /* Set symtab language to language from DW_AT_language.  If the
> +        compilation is from a C file generated by language preprocessors, do
> +        not set the language if it was already deduced by start_subfile.  */
> +      if (!(cu->language == language_c && symtab->language != language_c))
> +       symtab->language = cu->language;
> +
> +      symtab->locations_valid = cu->has_loclist;
>     }
>
>   if (dwarf2_per_objfile->using_index)
> @@ -10839,6 +10847,9 @@ var_decode_location (struct attribute *attr, struct symbol *sym,
>
>   dwarf2_symbol_mark_computed (attr, sym, cu);
>   SYMBOL_CLASS (sym) = LOC_COMPUTED;
> +
> +  if (SYMBOL_COMPUTED_OPS (sym) == &dwarf2_loclist_funcs)
> +    cu->has_loclist = 1;
>  }
>
>  /* Given a pointer to a DWARF information entry, figure out if we need
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2470,6 +2470,9 @@ skip_prologue_sal (struct symtab_and_line *sal)
>   if (sal->explicit_pc)
>     return;
>
> +  if (sal->symtab != NULL && sal->symtab->locations_valid)
> +    return;
> +
>   old_chain = save_current_space_and_thread ();
>   switch_to_program_space_and_thread (sal->pspace);
>
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -762,7 +762,12 @@ struct symtab
>      should be designated the primary, so that the blockvector
>      is relocated exactly once by objfile_relocate.  */
>
> -  int primary;
> +  unsigned int primary : 1;
> +
> +  /* GDB does not need to skip prologues as the variable locations are valid
> +     for all the PC values.  */
> +
> +  unsigned int locations_valid : 1;
>
>   /* The macro table for this symtab.  Like the blockvector, this
>      may be shared between different symtabs --- and normally is for
>



-- 
Paul Pluzhnikov