[RFA] Fix has_section_at_zero for separate debug files

[RFA] Fix has_section_at_zero for separate debug files

[Tristan Gingold]

Hi,

this patch reconsiders the approach taken in
http://sourceware.org/ml/gdb-patches/2009-06/msg00116.html

The new problem is that has_section_at_zero is now wrongly cleared.  This
happens when the separate debug file is a relocatable file (such as on
Darwin) which may have a function at address 0.  If its backlink (ie the
executable) has no section at 0 (which is the rule), one or more symbols
disappear in the symtab.

Because a separate debug file may be completely unrelated (in terms of
mapping) from its backlink, we shouldn't try to copy the has_section_at_zero.
Instead, we also consider SEC_ALLOC sections to set has_section_at_zero.  This
makes sense because you may perfectly have the bss at zero.

No regression on GNU Linux i386.
I have also checked this patch which a tiny program having a section at 0
and run in qemu.

(I cc: Kevin Buettner, the author of the previous approach).

Tristan.

2010-01-07  Tristan Gingold  <gingold@adacore.com>

	* dwarf2read.c (dwarf2_locate_sections): Also consider SEC_ALLOC
	sections to set has_section_at_zero.
	(dwarf2_psymtab_to_symtab): Do not copy has_section_at_zero from
	backlink for separate debug file.
---
 gdb/dwarf2read.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 7623035..7dbe02f 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1258,7 +1258,7 @@ dwarf2_locate_sections (bfd *abfd, asection *sectp, void *ignore_ptr)
       dwarf2_per_objfile->types.size = bfd_get_section_size (sectp);
     }
 
-  if ((bfd_get_section_flags (abfd, sectp) & SEC_LOAD)
+  if ((bfd_get_section_flags (abfd, sectp) & (SEC_LOAD | SEC_ALLOC))
       && bfd_section_vma (abfd, sectp) == 0)
     dwarf2_per_objfile->has_section_at_zero = 1;
 }
@@ -2994,19 +2994,6 @@ dwarf2_psymtab_to_symtab (struct partial_symtab *pst)
 	  dwarf2_per_objfile = objfile_data (pst->objfile,
 					     dwarf2_objfile_data_key);
 
-	  /* If this psymtab is constructed from a debug-only objfile, the
-	     has_section_at_zero flag will not necessarily be correct.  We
-	     can get the correct value for this flag by looking at the data
-	     associated with the (presumably stripped) associated objfile.  */
-	  if (pst->objfile->separate_debug_objfile_backlink)
-	    {
-	      struct dwarf2_per_objfile *dpo_backlink
-	        = objfile_data (pst->objfile->separate_debug_objfile_backlink,
-		                dwarf2_objfile_data_key);
-	      dwarf2_per_objfile->has_section_at_zero
-		= dpo_backlink->has_section_at_zero;
-	    }
-
 	  psymtab_to_symtab_1 (pst);
 
 	  /* Finish up the debug error message.  */
-- 
1.6.5.rc2

Re: [RFA] Fix has_section_at_zero for separate debug files

[Tom Tromey]

>>>>> "Tristan" == Tristan Gingold <gingold@ACT-Europe.FR> writes:

Tristan> this patch reconsiders the approach taken in
Tristan> http://sourceware.org/ml/gdb-patches/2009-06/msg00116.html

Tristan> The new problem is that has_section_at_zero is now wrongly
Tristan> cleared.  This happens when the separate debug file is a
Tristan> relocatable file (such as on Darwin) which may have a function
Tristan> at address 0.  If its backlink (ie the executable) has no
Tristan> section at 0 (which is the rule), one or more symbols disappear
Tristan> in the symtab.

This seems strange to me.  My mental model is that the separate debug
objfile describes the parent objfile.  To me this implies that on Darwin
you ought to relocate the separate debug objects.

Tristan> Because a separate debug file may be completely unrelated (in
Tristan> terms of mapping) from its backlink, we shouldn't try to copy
Tristan> the has_section_at_zero.  Instead, we also consider SEC_ALLOC
Tristan> sections to set has_section_at_zero.  This makes sense because
Tristan> you may perfectly have the bss at zero.

This also seems a little weird, given that has_section_at_zero is only
used to determine whether PC ranges are valid in some situations.

Tristan> No regression on GNU Linux i386.

I wonder whether this code path is tested there.
I don't know.

Tom

Re: [RFA] Fix has_section_at_zero for separate debug files

[Kevin Buettner]

On Mon, 11 Jan 2010 14:28:48 -0700
Tom Tromey <tromey@redhat.com> wrote:

> Tristan> No regression on GNU Linux i386.
> 
> I wonder whether this code path is tested there.
> I don't know.

I don't know either.  My earlier patch (which seems to be causing
problems on Darwin now) was motivated by some problems that I was
seeing with mep-elf.

I'll test Tristan's patch using a mep-elf build in the next few days
to see if it too solves the problems that I was seeing last year.

Kevin

Re: [RFA] Fix has_section_at_zero for separate debug files

[Tristan Gingold]

On Jan 12, 2010, at 6:00 AM, Kevin Buettner wrote:

> On Mon, 11 Jan 2010 14:28:48 -0700
> Tom Tromey <tromey@redhat.com> wrote:
> 
>> Tristan> No regression on GNU Linux i386.
>> 
>> I wonder whether this code path is tested there.
>> I don't know.
> 
> I don't know either.  My earlier patch (which seems to be causing
> problems on Darwin now) was motivated by some problems that I was
> seeing with mep-elf.
> 
> I'll test Tristan's patch using a mep-elf build in the next few days
> to see if it too solves the problems that I was seeing last year.

Thanks for doing that.

Tristan.

Re: [RFA] Fix has_section_at_zero for separate debug files

[Joel Brobecker]

Hi Kevin,

I'm going through the (growing) backlog of patches written by AdaCore,
and notice that this one got stuck. Would you mind doing the testing
you offered below? Once we have the result, I can start pinging Tristan
and whoever can review the code to get things going again...

I attached the initial email, to make it easier to get the patch
again.  Hopefully, it still applies - otherwise, I'll nudge Tristan
into producing an updated version...

Thanks!

On Mon, Jan 11, 2010 at 10:00:43PM -0700, Kevin Buettner wrote:
> On Mon, 11 Jan 2010 14:28:48 -0700
> Tom Tromey <tromey@redhat.com> wrote:
> 
> > Tristan> No regression on GNU Linux i386.
> > 
> > I wonder whether this code path is tested there.
> > I don't know.
> 
> I don't know either.  My earlier patch (which seems to be causing
> problems on Darwin now) was motivated by some problems that I was
> seeing with mep-elf.
> 
> I'll test Tristan's patch using a mep-elf build in the next few days
> to see if it too solves the problems that I was seeing last year.
> 
> Kevin

-- 
Joel

Re: [RFA] Fix has_section_at_zero for separate debug files

[Tristan Gingold]

On Jan 11, 2010, at 10:28 PM, Tom Tromey wrote:

>>>>>> "Tristan" == Tristan Gingold <gingold@ACT-Europe.FR> writes:
> 
> Tristan> this patch reconsiders the approach taken in
> Tristan> http://sourceware.org/ml/gdb-patches/2009-06/msg00116.html
> 
> Tristan> The new problem is that has_section_at_zero is now wrongly
> Tristan> cleared.  This happens when the separate debug file is a
> Tristan> relocatable file (such as on Darwin) which may have a function
> Tristan> at address 0.  If its backlink (ie the executable) has no
> Tristan> section at 0 (which is the rule), one or more symbols disappear
> Tristan> in the symtab.
> 
> This seems strange to me.  My mental model is that the separate debug
> objfile describes the parent objfile.

That's correct.

>  To me this implies that on Darwin
> you ought to relocate the separate debug objects.

Currently we don't relocate it (ie we don't apply relocs), we simply set offsets.  But the code doesn't
tests offsets.
Of course not relocating is somewhat wrong, because of common symbols.  But that's how it works now, and
I am working on using relocations.

> Tristan> Because a separate debug file may be completely unrelated (in
> Tristan> terms of mapping) from its backlink, we shouldn't try to copy
> Tristan> the has_section_at_zero.  Instead, we also consider SEC_ALLOC
> Tristan> sections to set has_section_at_zero.  This makes sense because
> Tristan> you may perfectly have the bss at zero.
> 
> This also seems a little weird, given that has_section_at_zero is only
> used to determine whether PC ranges are valid in some situations.

Argh, you're right.  I thought it was also used to check wether variables exists or not.

> Tristan> No regression on GNU Linux i386.
> 
> I wonder whether this code path is tested there.
> I don't know.

I think so.  I tested the little example with gdb unpatched, with the code removed in the patch (failure)
and with the full patch.

Tristan.

Re: [RFA] Fix has_section_at_zero for separate debug files

[Kevin Buettner]

On Thu, 7 Jan 2010 14:09:18 +0100
Tristan Gingold <gingold@ACT-Europe.FR> wrote:

> 2010-01-07  Tristan Gingold  <gingold@adacore.com>
> 
> 	* dwarf2read.c (dwarf2_locate_sections): Also consider SEC_ALLOC
> 	sections to set has_section_at_zero.
> 	(dwarf2_psymtab_to_symtab): Do not copy has_section_at_zero from
> 	backlink for separate debug file.

Tristan's patch causes no regressions for the gdb.base/sepdebug.exp
test, for which I had used a different approach for fixing problems
for that I was seeing when testing a mep-elf toolchain.

Tristan's patch reverts my earlier work and uses a different approach
to ensure that `has_section_at_zero' is set correctly.  I endorse this
approach and think that this patch should be committed.

Kevin

Re: [RFA] Fix has_section_at_zero for separate debug files

[Joel Brobecker]

> > 2010-01-07  Tristan Gingold  <gingold@adacore.com>
> > 
> > 	* dwarf2read.c (dwarf2_locate_sections): Also consider SEC_ALLOC
> > 	sections to set has_section_at_zero.
> > 	(dwarf2_psymtab_to_symtab): Do not copy has_section_at_zero from
> > 	backlink for separate debug file.
> 
> Tristan's patch causes no regressions for the gdb.base/sepdebug.exp
> test, for which I had used a different approach for fixing problems
> for that I was seeing when testing a mep-elf toolchain.

Just a quick status on this patch. I spoke to Tristan yesterday, and
we think that there were some things that needed to be adjusted. But
we'll need some time to remember all the details again, and Tristan
has found a new toy (VMS), so....

-- 
Joel