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

[Paul Pluzhnikov <ppluzhnikov at google dot 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