[2/3] Check data is GC'ed

Message ID 1408609338-17561-2-git-send-email-yao@codesourcery.com
State New, archived
Headers

Commit Message

Yao Qi Aug. 21, 2014, 8:22 a.m. UTC
  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(-)
  

Comments

Doug Evans Aug. 22, 2014, 6:02 p.m. UTC | #1
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
 >
  
Yao Qi Aug. 29, 2014, 1:28 p.m. UTC | #2
Doug Evans <dje@google.com> writes:

>  >        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.

I don't figure out a reason to do so, but it is possible via:

__attribute__((section(".text"))) int foo = 1;

I can't think of other heuristics, so if GDB has a section at address
zero, GDB can't tell address zero in debug information means the
corresponding variable is GC'ed by linker or the variable address is
zero.

How about open a PR about GDB is unable to know whether a variable is
GC'ed by linker if there is a section at address zero? and kfail
dw2-var-zero-addr.exp conditionally?  Something like:

    if [is_address_zero_readable] {
	setup_kfail "gdb/PR" "*-*-*"
    }
    gdb_test "print &var" {No symbol "var" in current context\.}

What do you think?
  

Patch

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;
 
   /* 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))
 	{
 	  /* 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)
 	{
 	  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)))
 		{
 		  /* When a static variable is eliminated by the linker,
 		     the corresponding debug information is not stripped