Re: [PATCH 2/3] Check data is GC'ed

Doug Evans
2014-08-22T18:02:30Z

Hi. Comments inline.

Yao Qi writes:
> We see the following fail on arm-none-eabi target,
>
> (gdb) print &var^M
> $1 = (int *) 0x0 <_ftext>^M
> (gdb) FAIL: gdb.dwarf2/dw2-var-zero-addr.exp: print &var
>
> Test dw2-var-zero-addr.exp is intended to test whether GDB can
> correctly know variable var is GC'ed by linker. Currently, the
> heuristic GDB is using is (see add_partial_symbol)
>
> && addr == 0
> && !dwarf2_per_objfile->has_section_at_zero
>
> however, it doesn't work for bare metal targets, on which certain
> section is located at address zero. In this patch, we improve the
> heuristic that if the variable address is zero, and section at address
> zero is executable, we think the variable is GC'ed by linker, because
> there can't be a variable in an executable section. In order to know
> this, we replace flag has_section_at_zero with a pointer
> section_at_zero.
>
> gdb:
>
> 2014-08-20 Yao Qi <yao@codesourcery.com>
>
> * dwarf2read.c (struct dwarf2_per_objfile) <has_section_at_zero>:
> Remove.
> <section_at_zero>: New field. Callers update.
> (add_partial_symbol): Extend the condition to check whether the
> section at zero is executable.
> (new_symbol_full): Check whether the section at zero is
> executable.
> ---
> gdb/dwarf2read.c | 32 +++++++++++++++++++-------------
> 1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index cf2ce76..f5b6341 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -278,9 +278,8 @@ struct dwarf2_per_objfile
> original data was compressed using 'dwz -m'. */
> struct dwz_file *dwz_file;
>
> - /* A flag indicating wether this objfile has a section loaded at a
> - VMA of 0. */
> - int has_section_at_zero;
> + /* The section of this objfile loaded at a VMA of 0. */
> + asection *section_at_zero;

If overlays are in use, could multiple sections be at zero?
[I think so, but I haven't used overlays in awhile.]
Storing a vector of sections_at_zero seems like massive
overkill to solve this problem, maybe there's a simpler solution,
I don't know. But I'm not opposed to it if it's the only
solution we have at the moment.

>
> /* True if we are using the mapped index,
> or we are faking it for OBJF_READNOW's sake. */
> @@ -2186,7 +2185,7 @@ dwarf2_locate_sections (bfd *abfd, asection *sectp, void *vnames)
>
> if ((bfd_get_section_flags (abfd, sectp) & SEC_LOAD)
> && bfd_section_vma (abfd, sectp) == 0)
> - dwarf2_per_objfile->has_section_at_zero = 1;
> + dwarf2_per_objfile->section_at_zero = sectp;
> }
>
> /* A helper function that decides whether a section is empty,
> @@ -6851,7 +6850,13 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
>
> if (pdi->d.locdesc
> && addr == 0
> - && !dwarf2_per_objfile->has_section_at_zero)
> + /* When the address is 0, if the object file doesn't have
> + section at zero or the section at zero is executable,
> + we think address 0 means the corresponding variable is
> + removed by linker, instead of there is a data at address
> + 0. */
> + && (dwarf2_per_objfile->section_at_zero == NULL
> + || dwarf2_per_objfile->section_at_zero->flags & SEC_CODE))

Could there be a DW_TAG_variable in SEC_CODE?
Someone might put one there for a particular reason.

> {
> /* A global or static variable may also have been stripped
> out by the linker if unused, in which case its address
> @@ -7341,8 +7346,8 @@ dwarf2_read_symtab (struct partial_symtab *self,
> = objfile_data (objfile->separate_debug_objfile_backlink,
> dwarf2_objfile_data_key);
>
> - dwarf2_per_objfile->has_section_at_zero
> - = dpo_backlink->has_section_at_zero;
> + dwarf2_per_objfile->section_at_zero
> + = dpo_backlink->section_at_zero;
> }
>
> dwarf2_per_objfile->reading_partial_symbols = 0;
> @@ -11758,7 +11763,7 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
> /* A not-uncommon case of bad debug info.
> Don't pollute the addrmap with bad data. */
> if (range_beginning + baseaddr == 0
> - && !dwarf2_per_objfile->has_section_at_zero)
> + && !dwarf2_per_objfile->section_at_zero)

Convention is to write "ptr == NULL" instead of "!ptr".

> {
> complaint (&symfile_complaints,
> _(".debug_ranges entry has start address of zero"
> @@ -11871,7 +11876,7 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
> labels are not in the output, so the relocs get a value of 0.
> If this is a discarded function, mark the pc bounds as invalid,
> so that GDB will ignore it. */
> - if (low == 0 && !dwarf2_per_objfile->has_section_at_zero)
> + if (low == 0 && !dwarf2_per_objfile->section_at_zero)
> return 0;
>
> *lowpc = low;
> @@ -12095,7 +12100,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
>
> /* A not-uncommon case of bad debug info.
> Don't pollute the addrmap with bad data. */
> - if (start == 0 && !dwarf2_per_objfile->has_section_at_zero)
> + if (start == 0 && !dwarf2_per_objfile->section_at_zero)
> {
> complaint (&symfile_complaints,
> _(".debug_ranges entry has start address of zero"
> @@ -15668,7 +15673,7 @@ read_partial_die (const struct die_reader_specs *reader,
> labels are not in the output, so the relocs get a value of 0.
> If this is a discarded function, mark the pc bounds as invalid,
> so that GDB will ignore it. */
> - if (part_die->lowpc == 0 && !dwarf2_per_objfile->has_section_at_zero)
> + if (part_die->lowpc == 0 && !dwarf2_per_objfile->section_at_zero)
> {
> struct gdbarch *gdbarch = get_objfile_arch (objfile);
>
> @@ -17297,7 +17302,7 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
> pst = cu->per_cu->v.psymtab;
>
> if (address == 0
> - && (!dwarf2_per_objfile->has_section_at_zero
> + && (!dwarf2_per_objfile->section_at_zero
> || (pst != NULL && pst->textlow > address)))
> {
> /* This line table is for a function which has been
> @@ -17870,7 +17875,8 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>
> if (SYMBOL_CLASS (sym) == LOC_STATIC
> && SYMBOL_VALUE_ADDRESS (sym) == 0
> - && !dwarf2_per_objfile->has_section_at_zero)
> + && (!dwarf2_per_objfile->section_at_zero
> + || (dwarf2_per_objfile->section_at_zero->flags & SEC_CODE)))

The SEC_CODE test doesn't feel right.
I understand the intent, but someone might put a variable in .text
for a reason.

> {
> /* When a static variable is eliminated by the linker,
> the corresponding debug information is not stripped
> --
> 1.9.3
>