PATCH: ld/2338: objdump -d -l doesn't work correctly

PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

_bfd_dwarf2_find_nearest_line doesn't work on relocatable file with
more than one executable sections. This patch checks section to
make sure that the right function name is returned.


H.J.
----
2006-02-13  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/2338
	* dwarf2.c (check_function_name): New function.
	(_bfd_dwarf2_find_nearest_line): Use check_function_name to
	check if function is correct.

--- bfd/dwarf2.c.func	2006-01-20 08:53:55.000000000 -0800
+++ bfd/dwarf2.c	2006-02-14 11:25:51.000000000 -0800
@@ -2179,6 +2179,29 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
+/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
+   SECTION.  */
+
+static bfd_boolean
+check_function_name (asection *section, asymbol **symbols,
+		     const char *func)
+{
+  if (func != NULL && section != NULL)
+    {
+      asymbol **p;
+
+      for (p = symbols; *p != NULL; p++)
+	{
+	  if (((*p)->flags & BSF_FUNCTION) != 0
+	      && (*p)->name != NULL
+	      && strcmp ((*p)->name, func) == 0)
+	    return (*p)->section == section;
+	}
+    }
+
+  return TRUE;
+}
+
 /* The DWARF2 version of find_nearest_line.  Return TRUE if the line
    is found without error.  ADDR_SIZE is the number of bytes in the
    initial .debug_info length field and in the abbreviation offset.
@@ -2300,7 +2323,8 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash))
+					linenumber_ptr, stash)
+	&& check_function_name (section, symbols, *functionname_ptr))
       return TRUE;
 
   /* Read each remaining comp. units checking each as they are read.  */
@@ -2368,7 +2392,9 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash))
+						  stash)
+		  && check_function_name (section, symbols,
+					  *functionname_ptr))
 		return TRUE;
 	    }
 	}

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Andreas Schwab]

"H. J. Lu" <hjl@lucon.org> writes:

> +/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
> +   SECTION.  */

Double negation is not easy to not misunderstand.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 14, 2006 at 11:54:05PM +0100, Andreas Schwab wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> 
> > +/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
> > +   SECTION.  */
> 
> Double negation is not easy to not misunderstand.
> 

I am trying to say that only FALSE return is reliable since TRUE can
return under other conditions. Maybe I should change the function
name to function_name_section_mismatch.

Here is an updated patch to restrict to relocatable files.


H.J.
---
----
2006-02-13  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (check_function_name): New function.
	(_bfd_dwarf2_find_nearest_line): Use check_function_name to
	check if function is correct.

--- bfd/dwarf2.c.func	2006-01-20 08:53:55.000000000 -0800
+++ bfd/dwarf2.c	2006-02-14 13:29:09.000000000 -0800
@@ -2179,6 +2179,34 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
+/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
+   SECTION in ABFD.  */
+
+static bfd_boolean
+check_function_name (bfd *abfd, asection *section, asymbol **symbols,
+		     const char *func)
+{
+  /* Only a relocatable file can have 2 symbols with the same
+     address.  */
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
+      && func != NULL
+      && section != NULL
+      && symbols != NULL)
+    {
+      asymbol **p;
+
+      for (p = symbols; *p != NULL; p++)
+	{
+	  if (((*p)->flags & BSF_FUNCTION) != 0
+	      && (*p)->name != NULL
+	      && strcmp ((*p)->name, func) == 0)
+	    return (*p)->section == section;
+	}
+    }
+
+  return TRUE;
+}
+
 /* The DWARF2 version of find_nearest_line.  Return TRUE if the line
    is found without error.  ADDR_SIZE is the number of bytes in the
    initial .debug_info length field and in the abbreviation offset.
@@ -2300,7 +2328,9 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash))
+					linenumber_ptr, stash)
+	&& check_function_name (abfd, section, symbols,
+				*functionname_ptr))
       return TRUE;
 
   /* Read each remaining comp. units checking each as they are read.  */
@@ -2368,7 +2398,9 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash))
+						  stash)
+		  && check_function_name (abfd, section, symbols,
+					  *functionname_ptr))
 		return TRUE;
 	    }
 	}

RE: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Dave Korn]

On 14 February 2006 23:24, H. J. Lu wrote:

> On Tue, Feb 14, 2006 at 11:54:05PM +0100, Andreas Schwab wrote:
>> "H. J. Lu" <hjl@lucon.org> writes:
>> 
>>> +/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
>>> +   SECTION.  */
>> 
>> Double negation is not easy to not misunderstand.
>> 
> 
> I am trying to say that only FALSE return is reliable since TRUE can
> return under other conditions. Maybe I should change the function
> name to function_name_section_mismatch.

  Or how about wording it the other way round?



/*  Return TRUE if FUNC in symbol table SYMBOLS is relative to SECTION.  */


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

RE: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Dave Korn]

On 15 February 2006 12:30, Dave Korn wrote:

> On 14 February 2006 23:24, H. J. Lu wrote:
> 
>> On Tue, Feb 14, 2006 at 11:54:05PM +0100, Andreas Schwab wrote:
>>> "H. J. Lu" <hjl@lucon.org> writes:
>>> 
>>>> +/* Return FALSE if FUNC in symbol table SYMBOLS is not relative to
>>>> +   SECTION.  */
>>> 
>>> Double negation is not easy to not misunderstand.
>>> 
>> 
>> I am trying to say that only FALSE return is reliable since TRUE can
>> return under other conditions. Maybe I should change the function
>> name to function_name_section_mismatch.
> 
>   Or how about wording it the other way round?

  Or how about _I_ pay more attention to the explanation you already gave?
:-O  This is a strange kind-of-inverted predicate function, it's a bit odd; I
would suggest considering the possibility that making it conceptually clearer
may be worth the cost of logical-negating the return value from strcmp (and
changing the default TRUE to a FALSE at the end, of course).  Here's a more
accurate description than my last attempt, anyway:


/*  Return FALSE if FUNC matches a function symbol from section SECTION
  and is found in symbol table SYMBOL.  */


... perhaps you should move the abfd->flags test for a relocatable file
outside the function, because it's a separate test really, and it's the only
reason for passing in the abfd parameter so you could otherwise omit it?  It
seems strange to see a function called "check_function_name" and the first
thing it's doing is talking about whether two symbols can have the same
address and how it relates to whether the object is relocatable or not; I'd
put the comment and the test as part of the if-conditional at the call sites.




    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Wed, Feb 15, 2006 at 12:52:47PM -0000, Dave Korn wrote:
> 
> 
> /*  Return FALSE if FUNC matches a function symbol from section SECTION
>   and is found in symbol table SYMBOL.  */
> 

I updated the comment. Please take a look.

> 
> ... perhaps you should move the abfd->flags test for a relocatable file
> outside the function, because it's a separate test really, and it's the only
> reason for passing in the abfd parameter so you could otherwise omit it?  It
> seems strange to see a function called "check_function_name" and the first
> thing it's doing is talking about whether two symbols can have the same
> address and how it relates to whether the object is relocatable or not; I'd
> put the comment and the test as part of the if-conditional at the call sites.
> 

check_function_name checks if there is a mismatch. We know it can only
happen in .o file. I think it is appropriate to check it there.


H.J.
----
2006-02-13  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (check_function_name): New function.
	(_bfd_dwarf2_find_nearest_line): Use check_function_name to
	check if function is correct.

--- bfd/dwarf2.c.func	2006-01-20 08:53:55.000000000 -0800
+++ bfd/dwarf2.c	2006-02-15 13:47:20.000000000 -0800
@@ -2179,6 +2179,34 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
+/* Return TRUE if there is no mismatch bewteen function FUNC and
+   section SECTION from symbol table SYMBOLS in ABFD.  */
+
+static bfd_boolean
+check_function_name (bfd *abfd, asection *section, asymbol **symbols,
+		     const char *func)
+{
+  /* Mismatch can only happen when we have 2 functions with the same
+     address. It can only occur in a relocatable file.  */
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
+      && func != NULL
+      && section != NULL
+      && symbols != NULL)
+    {
+      asymbol **p;
+
+      for (p = symbols; *p != NULL; p++)
+	{
+	  if (((*p)->flags & BSF_FUNCTION) != 0
+	      && (*p)->name != NULL
+	      && strcmp ((*p)->name, func) == 0)
+	    return (*p)->section == section;
+	}
+    }
+
+  return TRUE;
+}
+
 /* The DWARF2 version of find_nearest_line.  Return TRUE if the line
    is found without error.  ADDR_SIZE is the number of bytes in the
    initial .debug_info length field and in the abbreviation offset.
@@ -2300,7 +2328,9 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash))
+					linenumber_ptr, stash)
+	&& check_function_name (abfd, section, symbols,
+				*functionname_ptr))
       return TRUE;
 
   /* Read each remaining comp. units checking each as they are read.  */
@@ -2368,7 +2398,9 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash))
+						  stash)
+		  && check_function_name (abfd, section, symbols,
+					  *functionname_ptr))
 		return TRUE;
 	    }
 	}

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Wed, Feb 15, 2006 at 01:50:54PM -0800, H. J. Lu wrote:
> 	PR binutils/2338
> 	* dwarf2.c (check_function_name): New function.
> 	(_bfd_dwarf2_find_nearest_line): Use check_function_name to
> 	check if function is correct.

OK.  A nicer solution might be to change vmas of sections in relocatable
object files so that none overlap, before calling
_bfd_dwarf2_find_nearest_line.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Daniel Jacobowitz]

On Thu, Feb 16, 2006 at 08:50:16AM +1030, Alan Modra wrote:
> On Wed, Feb 15, 2006 at 01:50:54PM -0800, H. J. Lu wrote:
> > 	PR binutils/2338
> > 	* dwarf2.c (check_function_name): New function.
> > 	(_bfd_dwarf2_find_nearest_line): Use check_function_name to
> > 	check if function is correct.
> 
> OK.  A nicer solution might be to change vmas of sections in relocatable
> object files so that none overlap, before calling
> _bfd_dwarf2_find_nearest_line.

There's a bit of nasty code to do this that I added to GDB, for
basically the same reason.  It's in symfile.c:place_section.

-- 
Daniel Jacobowitz
CodeSourcery

RE: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Dave Korn]

On 15 February 2006 21:51, H. J. Lu wrote:


> I updated the comment. Please take a look.

  Oh yes, that's plenty clearer, thank you.

> check_function_name checks if there is a mismatch. We know it can only
> happen in .o file. I think it is appropriate to check it there.

  <shrugs> Fair enough, it's no big deal :)


    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Eric Botcazou]

> > OK.  A nicer solution might be to change vmas of sections in relocatable
> > object files so that none overlap, before calling
> > _bfd_dwarf2_find_nearest_line.

That might indeed be necessary because I think HJ's patch only fixes half of 
the problem in _bfd_dwarf2_find_nearest_line: bogus line numbers are indeed 
not returned anymore but nothing is returned for most of the functions with 
-ffunction-sections for example.

> There's a bit of nasty code to do this that I added to GDB, for
> basically the same reason.  It's in symfile.c:place_section.

I presume it's the following lines:

  /* For relocatable files, all loadable sections will start at zero.
     The zero is meaningless, so try to pick arbitrary addresses such
     that no loadable sections overlap.  This algorithm is quadratic,
     but the number of sections in a single object file is generally
     small.  */
  if ((bfd_get_file_flags (objfile->obfd) & (EXEC_P | DYNAMIC)) == 0)
    {
      struct place_section_arg arg;
      arg.offsets = objfile->section_offsets;
      arg.lowest = 0;
      bfd_map_over_sections (objfile->obfd, place_section, &arg);
    }

Note that the comment seems to be overlooking -ffunction-sections.

-- 
Eric Botcazou

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Daniel Jacobowitz]

On Mon, Feb 20, 2006 at 03:21:21PM +0100, Eric Botcazou wrote:
> > There's a bit of nasty code to do this that I added to GDB, for
> > basically the same reason.  It's in symfile.c:place_section.
> 
> I presume it's the following lines:
> 
>   /* For relocatable files, all loadable sections will start at zero.
>      The zero is meaningless, so try to pick arbitrary addresses such
>      that no loadable sections overlap.  This algorithm is quadratic,
>      but the number of sections in a single object file is generally
>      small.  */
>   if ((bfd_get_file_flags (objfile->obfd) & (EXEC_P | DYNAMIC)) == 0)
>     {
>       struct place_section_arg arg;
>       arg.offsets = objfile->section_offsets;
>       arg.lowest = 0;
>       bfd_map_over_sections (objfile->obfd, place_section, &arg);
>     }
> 
> Note that the comment seems to be overlooking -ffunction-sections.

Well, generally the number of functions in a single source file is
adequately small that there isn't a problem.  If someone wanted to
unquadraticify the algorithm, I wouldn't complain...

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Mon, Feb 20, 2006 at 03:21:21PM +0100, Eric Botcazou wrote:
> > > OK.  A nicer solution might be to change vmas of sections in relocatable
> > > object files so that none overlap, before calling
> > > _bfd_dwarf2_find_nearest_line.
> 
> That might indeed be necessary because I think HJ's patch only fixes half of 
> the problem in _bfd_dwarf2_find_nearest_line: bogus line numbers are indeed 
> not returned anymore but nothing is returned for most of the functions with 
> -ffunction-sections for example.
> 

Are you really sure that it isn't a gcc bug? "objdump -d -l" works
fine with -ffunction-sections for me. Do you have a testcase?



H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Eric Botcazou]

> Are you really sure that it isn't a gcc bug? "objdump -d -l" works
> fine with -ffunction-sections for me. Do you have a testcase?

"objdump -d -l" works fine, but not _bfd_dwarf2_find_nearest_line in the sense 
that it returns nothing for most of the functions (that's the very reason the 
former now works).  And I'm under the impression Alan's suggestion would work 
only if decode_line_info was able to process relocations.

-- 
Eric Botcazou

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Mon, Feb 20, 2006 at 06:25:37PM +0100, Eric Botcazou wrote:
> > Are you really sure that it isn't a gcc bug? "objdump -d -l" works
> > fine with -ffunction-sections for me. Do you have a testcase?
> 
> "objdump -d -l" works fine, but not _bfd_dwarf2_find_nearest_line in the sense 
> that it returns nothing for most of the functions (that's the very reason the 
> former now works).  And I'm under the impression Alan's suggestion would work 
> only if decode_line_info was able to process relocations.

Do you have a binutils testcase to show there is a problem with
_bfd_dwarf2_find_nearest_line and -ffunction-sections? I'd like to
fix it if possible.



H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

> Do you have a binutils testcase to show there is a problem with
> _bfd_dwarf2_find_nearest_line and -ffunction-sections? I'd like to
> fix it if possible.

Attached.  Compile with "-g -o t t.c" and "-g -ffunction-sections -o t t.c".


Before your patch:

t.o: In function `foo1':/home/eric/build/binutils/native/t.c:6: warning: the 
use of `tmpnam' is dangerous, better use `mkstemp'

t.o: In function `foo2':/home/eric/build/binutils/native/t.c:17: warning: the 
use of `tmpnam' is dangerous, better use `mkstemp'

so clearly bogus message with -ffunction-sections.


With your patch:

t.o: In function `foo1':/home/eric/build/binutils/native/t.c:6: warning: the 
use of `tmpnam' is dangerous, better use `mkstemp'

t.o: In function `foo1':t.c:(.text.foo1+0xe): warning: the use of `tmpnam' is 
dangerous, better use `mkstemp'

so no more bogus message but no source location either.

-- 
Eric Botcazou

[-- Attachment #2: t.c --]
[-- Type: text/x-csrc, Size: 172 bytes --]

#include <stdio.h>
#include <stdlib.h>

void foo1(void)
{
  char *s1 = tmpnam ("name");
}

void foo2(void)
{
  int s2 = mkstemp ("name");
}

int main(void)
{
  return 0;
}

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Mon, Feb 20, 2006 at 06:55:15PM +0100, Eric Botcazou wrote:
> > Do you have a binutils testcase to show there is a problem with
> > _bfd_dwarf2_find_nearest_line and -ffunction-sections? I'd like to
> > fix it if possible.
> 
> Attached.  Compile with "-g -o t t.c" and "-g -ffunction-sections -o t t.c".
> 
> 
> Before your patch:
> 
> t.o: In function `foo1':/home/eric/build/binutils/native/t.c:6: warning: the 
> use of `tmpnam' is dangerous, better use `mkstemp'
> 
> t.o: In function `foo2':/home/eric/build/binutils/native/t.c:17: warning: the 
> use of `tmpnam' is dangerous, better use `mkstemp'
> 
> so clearly bogus message with -ffunction-sections.
> 
> 
> With your patch:
> 
> t.o: In function `foo1':/home/eric/build/binutils/native/t.c:6: warning: the 
> use of `tmpnam' is dangerous, better use `mkstemp'
> 
> t.o: In function `foo1':t.c:(.text.foo1+0xe): warning: the use of `tmpnam' is 
> dangerous, better use `mkstemp'
> 
> so no more bogus message but no source location either.
> 

_bfd_dwarf2_find_nearest_line can't tell the line number when more
than one lines have the same address. I will see if I can implement
Dan's approach when I find the time.


H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Eric Botcazou]

> _bfd_dwarf2_find_nearest_line can't tell the line number when more
> than one lines have the same address.

Yes, that was also the underlying problem with objdump -d -l.

> I will see if I can implement Dan's approach when I find the time.

Clearly the way to go.  But since the relocations are not yet resolved in the 
debug info sections, I think you'll have to resolve them manually.

-- 
Eric Botcazou

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Daniel Jacobowitz]

On Tue, Feb 21, 2006 at 12:32:26AM +0100, Eric Botcazou wrote:
> > _bfd_dwarf2_find_nearest_line can't tell the line number when more
> > than one lines have the same address.
> 
> Yes, that was also the underlying problem with objdump -d -l.
> 
> > I will see if I can implement Dan's approach when I find the time.
> 
> Clearly the way to go.  But since the relocations are not yet resolved in the 
> debug info sections, I think you'll have to resolve them manually.

If only GDB had some code for that problem too! :-)  See
symfile_relocate_debug_section, which sets some other offsets
and calls bfd_simple_get_relocated_section_contents.

Note, these two don't always play well together - I just got a bug
report about that.  But in binutils this shouldn't be a big problem.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Eric Botcazou]

> If only GDB had some code for that problem too! :-)  See
> symfile_relocate_debug_section, which sets some other offsets
> and calls bfd_simple_get_relocated_section_contents.

Neat! :-)  I didn't know of bfd_simple_get_relocated_section_contents...

-- 
Eric Botcazou

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 21, 2006 at 12:32:26AM +0100, Eric Botcazou wrote:
> > _bfd_dwarf2_find_nearest_line can't tell the line number when more
> > than one lines have the same address.
> 
> Yes, that was also the underlying problem with objdump -d -l.
> 
> > I will see if I can implement Dan's approach when I find the time.
> 
> Clearly the way to go.  But since the relocations are not yet resolved in the 
> debug info sections, I think you'll have to resolve them manually.
> 

Here is the patch. It seems to work on all testcases I have.


H.J.
----
2006-02-21  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (check_function_name): Removed.
	(place_sections): New.
	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
	on relocatable files.
	(_bfd_dwarf2_find_line): Likewise.

--- bfd/dwarf2.c.rel	2006-02-16 10:08:05.000000000 -0800
+++ bfd/dwarf2.c	2006-02-21 11:40:40.000000000 -0800
@@ -2179,32 +2179,31 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
-/* Return TRUE if there is no mismatch bewteen function FUNC and
-   section SECTION from symbol table SYMBOLS in ABFD.  */
+/* Set unique vmas for loadable sections in ABFD.  */
 
-static bfd_boolean
-check_function_name (bfd *abfd, asection *section, asymbol **symbols,
-		     const char *func)
+static void
+place_sections (bfd *abfd)
 {
-  /* Mismatch can only happen when we have 2 functions with the same
-     address. It can only occur in a relocatable file.  */
-  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
-      && func != NULL
-      && section != NULL
-      && symbols != NULL)
+  asection *sect;
+  bfd_vma current_vma = 0;
+
+  for (sect = abfd->sections; sect != NULL; sect = sect->next)
     {
-      asymbol **p;
+      bfd_size_type sz;
+      bfd_vma vma;
 
-      for (p = symbols; *p != NULL; p++)
-	{
-	  if (((*p)->flags & BSF_FUNCTION) != 0
-	      && (*p)->name != NULL
-	      && strcmp ((*p)->name, func) == 0)
-	    return (*p)->section == section;
-	}
-    }
+      if ((sect->flags & SEC_LOAD) == 0)
+	continue;
 
-  return TRUE;
+      sz = sect->rawsize ? sect->rawsize : sect->size;
+      if (sz == 0)
+	continue;
+
+      vma = sect->vma;
+      if (current_vma != 0)
+	sect->vma = current_vma;
+      current_vma += vma + sz + 1;
+    }
 }
 
 /* The DWARF2 version of find_nearest_line.  Return TRUE if the line
@@ -2240,6 +2239,12 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
   struct comp_unit* each;
 
   stash = *pinfo;
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if (!stash && (abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    place_sections (abfd);
+
   addr = offset;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2328,9 +2333,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash)
-	&& check_function_name (abfd, section, symbols,
-				*functionname_ptr))
+					linenumber_ptr, stash))
       return TRUE;
 
   /* Read each remaining comp. units checking each as they are read.  */
@@ -2398,9 +2401,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash)
-		  && check_function_name (abfd, section, symbols,
-					  *functionname_ptr))
+						  stash))
 		return TRUE;
 	    }
 	}
@@ -2442,6 +2443,13 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
   section = bfd_get_section (symbol);
 
+  stash = *pinfo;
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if (!stash && (abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    place_sections (abfd);
+
   addr = symbol->value;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2449,7 +2457,6 @@ _bfd_dwarf2_find_line (bfd *abfd,
     addr += section->vma;
 
   *filename_ptr = NULL;
-  stash = *pinfo;
   *filename_ptr = NULL;
   *linenumber_ptr = 0;
 

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Daniel Jacobowitz]

On Tue, Feb 21, 2006 at 11:46:09AM -0800, H. J. Lu wrote:
> On Tue, Feb 21, 2006 at 12:32:26AM +0100, Eric Botcazou wrote:
> > > _bfd_dwarf2_find_nearest_line can't tell the line number when more
> > > than one lines have the same address.
> > 
> > Yes, that was also the underlying problem with objdump -d -l.
> > 
> > > I will see if I can implement Dan's approach when I find the time.
> > 
> > Clearly the way to go.  But since the relocations are not yet resolved in the 
> > debug info sections, I think you'll have to resolve them manually.
> > 
> 
> Here is the patch. It seems to work on all testcases I have.

Seems sensible, but do we need to un-set the vmas when we're done? 
Will they get back to the linker somehow?

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 21, 2006 at 03:12:41PM -0500, Daniel Jacobowitz wrote:
> On Tue, Feb 21, 2006 at 11:46:09AM -0800, H. J. Lu wrote:
> > On Tue, Feb 21, 2006 at 12:32:26AM +0100, Eric Botcazou wrote:
> > > > _bfd_dwarf2_find_nearest_line can't tell the line number when more
> > > > than one lines have the same address.
> > > 
> > > Yes, that was also the underlying problem with objdump -d -l.
> > > 
> > > > I will see if I can implement Dan's approach when I find the time.
> > > 
> > > Clearly the way to go.  But since the relocations are not yet resolved in the 
> > > debug info sections, I think you'll have to resolve them manually.
> > > 
> > 
> > Here is the patch. It seems to work on all testcases I have.
> 
> Seems sensible, but do we need to un-set the vmas when we're done? 
> Will they get back to the linker somehow?

I am not sure if it really matters since we already call
bfd_simple_get_relocated_section_contents which it may do more
damage than setting vma.


H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Daniel Jacobowitz]

On Tue, Feb 21, 2006 at 12:23:05PM -0800, H. J. Lu wrote:
> On Tue, Feb 21, 2006 at 03:12:41PM -0500, Daniel Jacobowitz wrote:
> > On Tue, Feb 21, 2006 at 11:46:09AM -0800, H. J. Lu wrote:
> > > On Tue, Feb 21, 2006 at 12:32:26AM +0100, Eric Botcazou wrote:
> > > > > _bfd_dwarf2_find_nearest_line can't tell the line number when more
> > > > > than one lines have the same address.
> > > > 
> > > > Yes, that was also the underlying problem with objdump -d -l.
> > > > 
> > > > > I will see if I can implement Dan's approach when I find the time.
> > > > 
> > > > Clearly the way to go.  But since the relocations are not yet resolved in the 
> > > > debug info sections, I think you'll have to resolve them manually.
> > > > 
> > > 
> > > Here is the patch. It seems to work on all testcases I have.
> > 
> > Seems sensible, but do we need to un-set the vmas when we're done? 
> > Will they get back to the linker somehow?
> 
> I am not sure if it really matters since we already call
> bfd_simple_get_relocated_section_contents which it may do more
> damage than setting vma.

It makes an effort to undo its damage, IIRC.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 21, 2006 at 06:07:07PM -0500, Daniel Jacobowitz wrote:
> On Tue, Feb 21, 2006 at 12:23:05PM -0800, H. J. Lu wrote:
> > On Tue, Feb 21, 2006 at 03:12:41PM -0500, Daniel Jacobowitz wrote:
> > > On Tue, Feb 21, 2006 at 11:46:09AM -0800, H. J. Lu wrote:
> > > > On Tue, Feb 21, 2006 at 12:32:26AM +0100, Eric Botcazou wrote:
> > > > > > _bfd_dwarf2_find_nearest_line can't tell the line number when more
> > > > > > than one lines have the same address.
> > > > > 
> > > > > Yes, that was also the underlying problem with objdump -d -l.
> > > > > 
> > > > > > I will see if I can implement Dan's approach when I find the time.
> > > > > 
> > > > > Clearly the way to go.  But since the relocations are not yet resolved in the 
> > > > > debug info sections, I think you'll have to resolve them manually.
> > > > > 
> > > > 
> > > > Here is the patch. It seems to work on all testcases I have.
> > > 
> > > Seems sensible, but do we need to un-set the vmas when we're done? 
> > > Will they get back to the linker somehow?
> > 
> > I am not sure if it really matters since we already call
> > bfd_simple_get_relocated_section_contents which it may do more
> > damage than setting vma.
> 
> It makes an effort to undo its damage, IIRC.

Good point. Here is an updated patch.


H.J.
----
2006-02-21  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (loadable_section): New struct.
	(dwarf2_debug): Add loadable_section_count and
	loadable_sections.
	(check_function_name): Removed.
	(unset_sections): New.
	(place_sections): New.
	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
	and unset_sections on relocatable files.
	(_bfd_dwarf2_find_line): Likewise.

--- bfd/dwarf2.c.rel	2006-02-16 10:08:05.000000000 -0800
+++ bfd/dwarf2.c	2006-02-21 17:50:21.000000000 -0800
@@ -74,6 +74,13 @@ struct dwarf_block
   bfd_byte *data;
 };
 
+struct loadable_section
+{
+  asection *section;
+  bfd_vma orig_vma;
+  bfd_vma adj_vma;
+};
+
 struct dwarf2_debug
 {
   /* A list of all previously read comp_units.  */
@@ -124,6 +131,12 @@ struct dwarf2_debug
      calling chain for subsequent calls to bfd_find_inliner_info to
      use. */
   struct funcinfo *inliner_chain;
+
+  /* Number of loadable sections.  */
+  unsigned int loadable_section_count;
+
+  /* Array of loadable sections.  */
+  struct loadable_section *loadable_sections;
 };
 
 struct arange
@@ -2179,28 +2192,87 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
-/* Return TRUE if there is no mismatch bewteen function FUNC and
-   section SECTION from symbol table SYMBOLS in ABFD.  */
+/* Unset vmas for loadable sections in STASH.  */
+
+static void
+unset_sections (struct dwarf2_debug *stash)
+{
+  unsigned int i;
+  struct loadable_section *p;
+
+  i = stash->loadable_section_count;
+  p = stash->loadable_sections;
+  for (; i > 0; i--, p++)
+    p->section->vma = p->orig_vma;
+}
+
+/* Set unique vmas for loadable sections in ABFD and save vmas in
+   STASH for unset_sections.  */
 
 static bfd_boolean
-check_function_name (bfd *abfd, asection *section, asymbol **symbols,
-		     const char *func)
+place_sections (bfd *abfd, struct dwarf2_debug *stash)
 {
-  /* Mismatch can only happen when we have 2 functions with the same
-     address. It can only occur in a relocatable file.  */
-  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
-      && func != NULL
-      && section != NULL
-      && symbols != NULL)
-    {
-      asymbol **p;
-
-      for (p = symbols; *p != NULL; p++)
-	{
-	  if (((*p)->flags & BSF_FUNCTION) != 0
-	      && (*p)->name != NULL
-	      && strcmp ((*p)->name, func) == 0)
-	    return (*p)->section == section;
+  struct loadable_section *p;
+  unsigned int i;
+
+  if (stash->loadable_section_count != 0)
+    {
+      i = stash->loadable_section_count;
+      p = stash->loadable_sections;
+      for (; i > 0; i--, p++)
+	p->section->vma = p->adj_vma;
+    }
+  else
+    {
+      asection *sect;
+      bfd_vma current_vma = 0;
+      bfd_size_type amt;
+      struct loadable_section *p;
+
+      i = 0;
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if ((sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  i++;
+	}
+
+      amt = i * sizeof (struct loadable_section);
+      p = (struct loadable_section *) bfd_zalloc (abfd, amt);
+      if (! p)
+	return FALSE;
+
+      stash->loadable_sections = p;
+      stash->loadable_section_count = i;
+
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+	  bfd_vma vma;
+
+	  if ((sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  vma = sect->vma;
+	  p->section = sect;
+	  p->orig_vma = vma;
+	  if (current_vma != 0)
+	    sect->vma = current_vma;
+	  p->adj_vma = sect->vma;
+	  current_vma += vma + sz + 1;
+
+	  p++;
 	}
     }
 
@@ -2239,7 +2311,27 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
   struct comp_unit* each;
 
+  bfd_vma found = FALSE;
+
   stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = offset;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2256,15 +2348,10 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     addr_size = 4;
   BFD_ASSERT (addr_size == 4 || addr_size == 8);
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2273,7 +2360,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2285,7 +2372,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2319,7 +2406,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2328,10 +2415,11 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash)
-	&& check_function_name (abfd, section, symbols,
-				*functionname_ptr))
-      return TRUE;
+					linenumber_ptr, stash))
+      {
+	found = TRUE;
+	goto done;
+      }
 
   /* Read each remaining comp. units checking each as they are read.  */
   while (stash->info_ptr < stash->info_ptr_end)
@@ -2398,15 +2486,20 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash)
-		  && check_function_name (abfd, section, symbols,
-					  *functionname_ptr))
-		return TRUE;
+						  stash))
+		{
+		  found = TRUE;
+		  goto done;
+		}
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 /* The DWARF2 version of find_line.  Return TRUE if the line is found
@@ -2438,10 +2531,29 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
   asection *section;
 
-  bfd_boolean found;
+  bfd_boolean found = FALSE;
 
   section = bfd_get_section (symbol);
 
+  stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if (!stash && (abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = symbol->value;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2449,19 +2561,13 @@ _bfd_dwarf2_find_line (bfd *abfd,
     addr += section->vma;
 
   *filename_ptr = NULL;
-  stash = *pinfo;
   *filename_ptr = NULL;
   *linenumber_ptr = 0;
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2470,7 +2576,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2482,7 +2588,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2516,7 +2622,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2528,7 +2634,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	found = comp_unit_find_line (each, symbol, addr, filename_ptr,
 				     linenumber_ptr, stash);
 	if (found)
-	  return found;
+	  goto done;
       }
 
   /* The DWARF2 spec says that the initial length field, and the
@@ -2605,12 +2711,16 @@ _bfd_dwarf2_find_line (bfd *abfd,
 					       linenumber_ptr,
 					       stash));
 	      if (found)
-		return TRUE;
+		goto done;
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 bfd_boolean

Ping: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 21, 2006 at 05:54:42PM -0800, H. J. Lu wrote:
> 
> Good point. Here is an updated patch.
> 
> 

Can someone take a look at this?

http://sourceware.org/ml/binutils/2006-02/msg00295.html


H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Eric Botcazou]

> Good point. Here is an updated patch.
>
>
> H.J.
> ----
> 2006-02-21  H.J. Lu  <hongjiu.lu@intel.com>
>
> 	PR binutils/2338
> 	* dwarf2.c (loadable_section): New struct.
> 	(dwarf2_debug): Add loadable_section_count and
> 	loadable_sections.
> 	(check_function_name): Removed.
> 	(unset_sections): New.
> 	(place_sections): New.
> 	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
> 	and unset_sections on relocatable files.
> 	(_bfd_dwarf2_find_line): Likewise.

Works For Me(tm).  Thanks.

> -/* Return TRUE if there is no mismatch bewteen function FUNC and
> -   section SECTION from symbol table SYMBOLS in ABFD.  */
> +/* Unset vmas for loadable sections in STASH.  */
> +
> +static void
> +unset_sections (struct dwarf2_debug *stash)
> +{
> +  unsigned int i;
> +  struct loadable_section *p;
> +
> +  i = stash->loadable_section_count;
> +  p = stash->loadable_sections;
> +  for (; i > 0; i--, p++)
> +    p->section->vma = p->orig_vma;
> +}

Minor stylistic nit, but I think the "for (;;)" idiom is a little abused here.

-- 
Eric Botcazou

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Tue, Feb 21, 2006 at 05:54:42PM -0800, H. J. Lu wrote:
> 	PR binutils/2338
> 	* dwarf2.c (loadable_section): New struct.
> 	(dwarf2_debug): Add loadable_section_count and
> 	loadable_sections.
> 	(check_function_name): Removed.
> 	(unset_sections): New.
> 	(place_sections): New.
> 	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
> 	and unset_sections on relocatable files.
> 	(_bfd_dwarf2_find_line): Likewise.

OK, except..

> +	  vma = sect->vma;
> +	  p->section = sect;
> +	  p->orig_vma = vma;
> +	  if (current_vma != 0)
> +	    sect->vma = current_vma;
> +	  p->adj_vma = sect->vma;
> +	  current_vma += vma + sz + 1;

..don't add one here.  This will likely break targets that expect insns
on a multiple of 2 or 4 boudary.  Instead, you ought to heed alignment
when setting the vma, and also round size up according to the alignment.

	  p->section = sect;
	  p->orig_vma = sect->vma;
	  sect->vma = ((current_vma + ~((bfd_vma) -1 << sect->alignment_power))
		       & ((bfd_vma) -1 << sect->alignment_power));
	  p->adj_vma = sect->vma;
	  sz = ((sz + ~((bfd_vma) -1 << sect->alignment_power))
		& ((bfd_vma) -1 << sect->alignment_power));
	  current_vma = sect->vma + sz;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 28, 2006 at 10:07:53AM +1030, Alan Modra wrote:
> On Tue, Feb 21, 2006 at 05:54:42PM -0800, H. J. Lu wrote:
> > 	PR binutils/2338
> > 	* dwarf2.c (loadable_section): New struct.
> > 	(dwarf2_debug): Add loadable_section_count and
> > 	loadable_sections.
> > 	(check_function_name): Removed.
> > 	(unset_sections): New.
> > 	(place_sections): New.
> > 	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
> > 	and unset_sections on relocatable files.
> > 	(_bfd_dwarf2_find_line): Likewise.
> 
> OK, except..
> 
> > +	  vma = sect->vma;
> > +	  p->section = sect;
> > +	  p->orig_vma = vma;
> > +	  if (current_vma != 0)
> > +	    sect->vma = current_vma;
> > +	  p->adj_vma = sect->vma;
> > +	  current_vma += vma + sz + 1;
> 
> ..don't add one here.  This will likely break targets that expect insns
> on a multiple of 2 or 4 boudary.  Instead, you ought to heed alignment
> when setting the vma, and also round size up according to the alignment.
> 
> 	  p->section = sect;
> 	  p->orig_vma = sect->vma;
> 	  sect->vma = ((current_vma + ~((bfd_vma) -1 << sect->alignment_power))
> 		       & ((bfd_vma) -1 << sect->alignment_power));
> 	  p->adj_vma = sect->vma;
> 	  sz = ((sz + ~((bfd_vma) -1 << sect->alignment_power))
> 		& ((bfd_vma) -1 << sect->alignment_power));
> 	  current_vma = sect->vma + sz;
> 

How about this patch? We leave sections with vma != 0 alone. We should
add 1 << sect->alignment_power so that if sect->alignment_power == 0
there is still a gap.


H.J.
---
2006-02-27  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (loadable_section): New struct.
	(dwarf2_debug): Add loadable_section_count and
	loadable_sections.
	(check_function_name): Removed.
	(unset_sections): New.
	(place_sections): New.
	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
	and unset_sections on relocatable files.
	(_bfd_dwarf2_find_line): Likewise.

--- bfd/dwarf2.c.rel	2006-02-16 10:08:05.000000000 -0800
+++ bfd/dwarf2.c	2006-02-27 19:59:56.000000000 -0800
@@ -74,6 +74,12 @@ struct dwarf_block
   bfd_byte *data;
 };
 
+struct loadable_section
+{
+  asection *section;
+  bfd_vma adj_vma;
+};
+
 struct dwarf2_debug
 {
   /* A list of all previously read comp_units.  */
@@ -124,6 +130,12 @@ struct dwarf2_debug
      calling chain for subsequent calls to bfd_find_inliner_info to
      use. */
   struct funcinfo *inliner_chain;
+
+  /* Number of loadable sections.  */
+  unsigned int loadable_section_count;
+
+  /* Array of loadable sections.  */
+  struct loadable_section *loadable_sections;
 };
 
 struct arange
@@ -2179,28 +2191,91 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
-/* Return TRUE if there is no mismatch bewteen function FUNC and
-   section SECTION from symbol table SYMBOLS in ABFD.  */
+/* Unset vmas for loadable sections in STASH.  */
+
+static void
+unset_sections (struct dwarf2_debug *stash)
+{
+  unsigned int i;
+  struct loadable_section *p;
+
+  i = stash->loadable_section_count;
+  p = stash->loadable_sections;
+  for (; i > 0; i--, p++)
+    p->section->vma = 0;
+}
+
+/* Set unique vmas for loadable sections in ABFD and save vmas in
+   STASH for unset_sections.  */
 
 static bfd_boolean
-check_function_name (bfd *abfd, asection *section, asymbol **symbols,
-		     const char *func)
+place_sections (bfd *abfd, struct dwarf2_debug *stash)
 {
-  /* Mismatch can only happen when we have 2 functions with the same
-     address. It can only occur in a relocatable file.  */
-  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
-      && func != NULL
-      && section != NULL
-      && symbols != NULL)
-    {
-      asymbol **p;
-
-      for (p = symbols; *p != NULL; p++)
-	{
-	  if (((*p)->flags & BSF_FUNCTION) != 0
-	      && (*p)->name != NULL
-	      && strcmp ((*p)->name, func) == 0)
-	    return (*p)->section == section;
+  struct loadable_section *p;
+  unsigned int i;
+
+  if (stash->loadable_section_count != 0)
+    {
+      i = stash->loadable_section_count;
+      p = stash->loadable_sections;
+      for (; i > 0; i--, p++)
+	p->section->vma = p->adj_vma;
+    }
+  else
+    {
+      asection *sect;
+      bfd_vma last_vma = 0;
+      bfd_size_type amt;
+      struct loadable_section *p;
+
+      i = 0;
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if (sect->vma != 0 || (sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  i++;
+	}
+
+      amt = i * sizeof (struct loadable_section);
+      p = (struct loadable_section *) bfd_zalloc (abfd, amt);
+      if (! p)
+	return FALSE;
+
+      stash->loadable_sections = p;
+      stash->loadable_section_count = i;
+
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if (sect->vma != 0 || (sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  p->section = sect;
+	  if (last_vma != 0)
+	    {
+	      /* Make a gap between sections and align the new address
+	         to the current section alignment.  */
+	      last_vma = ((last_vma
+			   + ((bfd_vma) 1 << sect->alignment_power))
+			  & ((bfd_vma) -1 << sect->alignment_power));
+	      sect->vma = last_vma;
+	    }
+	  p->adj_vma = sect->vma;
+	  last_vma += sect->vma + sz;
+
+	  p++;
 	}
     }
 
@@ -2239,7 +2314,27 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
   struct comp_unit* each;
 
+  bfd_vma found = FALSE;
+
   stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = offset;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2256,15 +2351,10 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     addr_size = 4;
   BFD_ASSERT (addr_size == 4 || addr_size == 8);
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2273,7 +2363,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2285,7 +2375,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2319,7 +2409,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2328,10 +2418,11 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash)
-	&& check_function_name (abfd, section, symbols,
-				*functionname_ptr))
-      return TRUE;
+					linenumber_ptr, stash))
+      {
+	found = TRUE;
+	goto done;
+      }
 
   /* Read each remaining comp. units checking each as they are read.  */
   while (stash->info_ptr < stash->info_ptr_end)
@@ -2398,15 +2489,20 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash)
-		  && check_function_name (abfd, section, symbols,
-					  *functionname_ptr))
-		return TRUE;
+						  stash))
+		{
+		  found = TRUE;
+		  goto done;
+		}
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 /* The DWARF2 version of find_line.  Return TRUE if the line is found
@@ -2438,10 +2534,29 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
   asection *section;
 
-  bfd_boolean found;
+  bfd_boolean found = FALSE;
 
   section = bfd_get_section (symbol);
 
+  stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if (!stash && (abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = symbol->value;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2449,19 +2564,13 @@ _bfd_dwarf2_find_line (bfd *abfd,
     addr += section->vma;
 
   *filename_ptr = NULL;
-  stash = *pinfo;
   *filename_ptr = NULL;
   *linenumber_ptr = 0;
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2470,7 +2579,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2482,7 +2591,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2516,7 +2625,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2528,7 +2637,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	found = comp_unit_find_line (each, symbol, addr, filename_ptr,
 				     linenumber_ptr, stash);
 	if (found)
-	  return found;
+	  goto done;
       }
 
   /* The DWARF2 spec says that the initial length field, and the
@@ -2605,12 +2714,16 @@ _bfd_dwarf2_find_line (bfd *abfd,
 					       linenumber_ptr,
 					       stash));
 	      if (found)
-		return TRUE;
+		goto done;
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 bfd_boolean

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Mon, Feb 27, 2006 at 08:06:22PM -0800, H. J. Lu wrote:
> We should
> add 1 << sect->alignment_power so that if sect->alignment_power == 0
> there is still a gap.

I don't see that there is any need for a gap.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 28, 2006 at 03:13:41PM +1030, Alan Modra wrote:
> On Mon, Feb 27, 2006 at 08:06:22PM -0800, H. J. Lu wrote:
> > We should
> > add 1 << sect->alignment_power so that if sect->alignment_power == 0
> > there is still a gap.
> 
> I don't see that there is any need for a gap.
> 

If there isn't a gap, "objump -d -l" may not work correctly for the
output from "-ffunction-sections" on x86 and x86-64. That is why I
added "1" in the first patch.


H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Mon, Feb 27, 2006 at 09:21:56PM -0800, H. J. Lu wrote:
> On Tue, Feb 28, 2006 at 03:13:41PM +1030, Alan Modra wrote:
> > On Mon, Feb 27, 2006 at 08:06:22PM -0800, H. J. Lu wrote:
> > > We should
> > > add 1 << sect->alignment_power so that if sect->alignment_power == 0
> > > there is still a gap.
> > 
> > I don't see that there is any need for a gap.
> 
> If there isn't a gap, "objump -d -l" may not work correctly for the
> output from "-ffunction-sections" on x86 and x86-64. That is why I
> added "1" in the first patch.

HJ, "may not work correctly" doesn't say anything about why you really
need a gap.  If you do need a gap, then there is a bug somewhere else.
That should be fixed.  Presumably the same problem, whatever it is,
could affect final linked executables with more than one text section.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 28, 2006 at 05:59:03PM +1030, Alan Modra wrote:
> On Mon, Feb 27, 2006 at 09:21:56PM -0800, H. J. Lu wrote:
> > On Tue, Feb 28, 2006 at 03:13:41PM +1030, Alan Modra wrote:
> > > On Mon, Feb 27, 2006 at 08:06:22PM -0800, H. J. Lu wrote:
> > > > We should
> > > > add 1 << sect->alignment_power so that if sect->alignment_power == 0
> > > > there is still a gap.
> > > 
> > > I don't see that there is any need for a gap.
> > 
> > If there isn't a gap, "objump -d -l" may not work correctly for the
> > output from "-ffunction-sections" on x86 and x86-64. That is why I
> > added "1" in the first patch.
> 
> HJ, "may not work correctly" doesn't say anything about why you really
> need a gap.  If you do need a gap, then there is a bug somewhere else.
> That should be fixed.  Presumably the same problem, whatever it is,
> could affect final linked executables with more than one text section.

decode_line_info in dwarf2.c calls add_line_info for both low_pc
and high_pc. If there is no gap, there may be 2 entries in line info
table with the same address. The line info looks up will fail if
an address matches more than 2 lines.


H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Tue, Feb 28, 2006 at 09:14:41AM -0800, H. J. Lu wrote:
> On Tue, Feb 28, 2006 at 05:59:03PM +1030, Alan Modra wrote:
> > On Mon, Feb 27, 2006 at 09:21:56PM -0800, H. J. Lu wrote:
> > > On Tue, Feb 28, 2006 at 03:13:41PM +1030, Alan Modra wrote:
> > > > On Mon, Feb 27, 2006 at 08:06:22PM -0800, H. J. Lu wrote:
> > > > > We should
> > > > > add 1 << sect->alignment_power so that if sect->alignment_power == 0
> > > > > there is still a gap.
> > > > 
> > > > I don't see that there is any need for a gap.
> > > 
> > > If there isn't a gap, "objump -d -l" may not work correctly for the
> > > output from "-ffunction-sections" on x86 and x86-64. That is why I
> > > added "1" in the first patch.
> > 
> > HJ, "may not work correctly" doesn't say anything about why you really
> > need a gap.  If you do need a gap, then there is a bug somewhere else.
> > That should be fixed.  Presumably the same problem, whatever it is,
> > could affect final linked executables with more than one text section.
> 
> decode_line_info in dwarf2.c calls add_line_info for both low_pc
> and high_pc. If there is no gap, there may be 2 entries in line info
> table with the same address. The line info looks up will fail if
> an address matches more than 2 lines.
> 

add_line_info should sort entries with the same address by line number.
This patch does that.


H.J.
----
2006-02-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (loadable_section): New struct.
	(dwarf2_debug): Add loadable_section_count and
	loadable_sections.
	(add_line_info): Sort entries with the same address by line
	number.
	(check_function_name): Removed.
	(unset_sections): New.
	(place_sections): New.
	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
	and unset_sections on relocatable files.
	(_bfd_dwarf2_find_line): Likewise.

--- bfd/dwarf2.c.rel	2006-02-16 10:08:05.000000000 -0800
+++ bfd/dwarf2.c	2006-02-28 10:21:58.000000000 -0800
@@ -74,6 +74,12 @@ struct dwarf_block
   bfd_byte *data;
 };
 
+struct loadable_section
+{
+  asection *section;
+  bfd_vma adj_vma;
+};
+
 struct dwarf2_debug
 {
   /* A list of all previously read comp_units.  */
@@ -124,6 +130,12 @@ struct dwarf2_debug
      calling chain for subsequent calls to bfd_find_inliner_info to
      use. */
   struct funcinfo *inliner_chain;
+
+  /* Number of loadable sections.  */
+  unsigned int loadable_section_count;
+
+  /* Array of loadable sections.  */
+  struct loadable_section *loadable_sections;
 };
 
 struct arange
@@ -746,8 +758,9 @@ struct varinfo
 
 /* Adds a new entry to the line_info list in the line_info_table, ensuring
    that the list is sorted.  Note that the line_info list is sorted from
-   highest to lowest VMA (with possible duplicates); that is,
-   line_info->prev_line always accesses an equal or smaller VMA.  */
+   highest to lowest VMA (with possible duplicates, which will be sorted
+   by line number); that is, line_info->prev_line always accesses an
+   equal or smaller VMA.  */
 
 static void
 add_line_info (struct line_info_table *table,
@@ -803,6 +816,17 @@ add_line_info (struct line_info_table *t
       {
 	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
 	   list and 2) the head of 'info'.  */
+	if (address == table->lcl_head->prev_line->address)
+	  {
+	    /* If the new adress is the same, we sort entries by line
+	       number.  */
+	    struct line_info* li = table->lcl_head->prev_line;
+
+	    while (address == li->address && li->line > line)
+	      li = li->prev_line;
+
+	    table->lcl_head = li;
+	  }
 	info->prev_line = table->lcl_head->prev_line;
 	table->lcl_head->prev_line = info;
 	break;
@@ -2179,28 +2203,91 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
-/* Return TRUE if there is no mismatch bewteen function FUNC and
-   section SECTION from symbol table SYMBOLS in ABFD.  */
+/* Unset vmas for loadable sections in STASH.  */
+
+static void
+unset_sections (struct dwarf2_debug *stash)
+{
+  unsigned int i;
+  struct loadable_section *p;
+
+  i = stash->loadable_section_count;
+  p = stash->loadable_sections;
+  for (; i > 0; i--, p++)
+    p->section->vma = 0;
+}
+
+/* Set unique vmas for loadable sections in ABFD and save vmas in
+   STASH for unset_sections.  */
 
 static bfd_boolean
-check_function_name (bfd *abfd, asection *section, asymbol **symbols,
-		     const char *func)
+place_sections (bfd *abfd, struct dwarf2_debug *stash)
 {
-  /* Mismatch can only happen when we have 2 functions with the same
-     address. It can only occur in a relocatable file.  */
-  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
-      && func != NULL
-      && section != NULL
-      && symbols != NULL)
-    {
-      asymbol **p;
-
-      for (p = symbols; *p != NULL; p++)
-	{
-	  if (((*p)->flags & BSF_FUNCTION) != 0
-	      && (*p)->name != NULL
-	      && strcmp ((*p)->name, func) == 0)
-	    return (*p)->section == section;
+  struct loadable_section *p;
+  unsigned int i;
+
+  if (stash->loadable_section_count != 0)
+    {
+      i = stash->loadable_section_count;
+      p = stash->loadable_sections;
+      for (; i > 0; i--, p++)
+	p->section->vma = p->adj_vma;
+    }
+  else
+    {
+      asection *sect;
+      bfd_vma last_vma = 0;
+      bfd_size_type amt;
+      struct loadable_section *p;
+
+      i = 0;
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if (sect->vma != 0 || (sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  i++;
+	}
+
+      amt = i * sizeof (struct loadable_section);
+      p = (struct loadable_section *) bfd_zalloc (abfd, amt);
+      if (! p)
+	return FALSE;
+
+      stash->loadable_sections = p;
+      stash->loadable_section_count = i;
+
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if (sect->vma != 0 || (sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  p->section = sect;
+	  if (last_vma != 0)
+	    {
+	      /* Align the new address to the current section
+		 alignment.  */
+	      last_vma = ((last_vma
+			   + ~((bfd_vma) -1 << sect->alignment_power))
+			  & ((bfd_vma) -1 << sect->alignment_power));
+	      sect->vma = last_vma;
+	    }
+	  p->adj_vma = sect->vma;
+	  last_vma += sect->vma + sz;
+
+	  p++;
 	}
     }
 
@@ -2239,7 +2326,27 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
   struct comp_unit* each;
 
+  bfd_vma found = FALSE;
+
   stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = offset;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2256,15 +2363,10 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     addr_size = 4;
   BFD_ASSERT (addr_size == 4 || addr_size == 8);
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2273,7 +2375,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2285,7 +2387,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2319,7 +2421,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2328,10 +2430,11 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash)
-	&& check_function_name (abfd, section, symbols,
-				*functionname_ptr))
-      return TRUE;
+					linenumber_ptr, stash))
+      {
+	found = TRUE;
+	goto done;
+      }
 
   /* Read each remaining comp. units checking each as they are read.  */
   while (stash->info_ptr < stash->info_ptr_end)
@@ -2398,15 +2501,20 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash)
-		  && check_function_name (abfd, section, symbols,
-					  *functionname_ptr))
-		return TRUE;
+						  stash))
+		{
+		  found = TRUE;
+		  goto done;
+		}
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 /* The DWARF2 version of find_line.  Return TRUE if the line is found
@@ -2438,10 +2546,29 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
   asection *section;
 
-  bfd_boolean found;
+  bfd_boolean found = FALSE;
 
   section = bfd_get_section (symbol);
 
+  stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if (!stash && (abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = symbol->value;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2449,19 +2576,13 @@ _bfd_dwarf2_find_line (bfd *abfd,
     addr += section->vma;
 
   *filename_ptr = NULL;
-  stash = *pinfo;
   *filename_ptr = NULL;
   *linenumber_ptr = 0;
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2470,7 +2591,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2482,7 +2603,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2516,7 +2637,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2528,7 +2649,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	found = comp_unit_find_line (each, symbol, addr, filename_ptr,
 				     linenumber_ptr, stash);
 	if (found)
-	  return found;
+	  goto done;
       }
 
   /* The DWARF2 spec says that the initial length field, and the
@@ -2605,12 +2726,16 @@ _bfd_dwarf2_find_line (bfd *abfd,
 					       linenumber_ptr,
 					       stash));
 	      if (found)
-		return TRUE;
+		goto done;
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 bfd_boolean

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Daniel Jacobowitz]

On Tue, Feb 28, 2006 at 10:23:52AM -0800, H. J. Lu wrote:
> On Tue, Feb 28, 2006 at 09:14:41AM -0800, H. J. Lu wrote:
> > > HJ, "may not work correctly" doesn't say anything about why you really
> > > need a gap.  If you do need a gap, then there is a bug somewhere else.
> > > That should be fixed.  Presumably the same problem, whatever it is,
> > > could affect final linked executables with more than one text section.
> > 
> > decode_line_info in dwarf2.c calls add_line_info for both low_pc
> > and high_pc. If there is no gap, there may be 2 entries in line info
> > table with the same address. The line info looks up will fail if
> > an address matches more than 2 lines.
> > 
> 
> add_line_info should sort entries with the same address by line number.
> This patch does that.

Anything related to debug info line mappings that needs to sort by line
number is very suspect.  It sounds like you're hacking around invalid
or misinterpreted data.

Maybe you just need to handle end_sequence specially, possibly again
by sorting it before other entries.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Tue, Feb 28, 2006 at 11:41:38AM -0800, H. J. Lu wrote:
> @@ -803,6 +815,15 @@ add_line_info (struct line_info_table *t
>        {
>  	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
>  	   list and 2) the head of 'info'.  */
> +	if (address == table->lcl_head->prev_line->address
> +	    && end_sequence != 0
> +	    && table->lcl_head->end_sequence != 0
> +	    && table->lcl_head->prev_line->end_sequence == 0)
> +	  {
> +	    /* If the new adress is the same, we need to make sure that
> +	       the order of end_sequence is correct.  */
> +	    table->lcl_head = table->lcl_head->prev_line;
> +	  }
>  	info->prev_line = table->lcl_head->prev_line;
>  	table->lcl_head->prev_line = info;
>  	break;

Convince me that this is correct.  ie. that you have actually done some
analysis and that this is the only place in add_line_info that needs to
do something special with end_sequence.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Fri, Mar 03, 2006 at 01:12:09PM +1030, Alan Modra wrote:
> On Tue, Feb 28, 2006 at 11:41:38AM -0800, H. J. Lu wrote:
> > @@ -803,6 +815,15 @@ add_line_info (struct line_info_table *t
> >        {
> >  	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
> >  	   list and 2) the head of 'info'.  */
> > +	if (address == table->lcl_head->prev_line->address
> > +	    && end_sequence != 0
> > +	    && table->lcl_head->end_sequence != 0
> > +	    && table->lcl_head->prev_line->end_sequence == 0)
> > +	  {
> > +	    /* If the new adress is the same, we need to make sure that
> > +	       the order of end_sequence is correct.  */
> > +	    table->lcl_head = table->lcl_head->prev_line;
> > +	  }
> >  	info->prev_line = table->lcl_head->prev_line;
> >  	table->lcl_head->prev_line = info;
> >  	break;
> 
> Convince me that this is correct.  ie. that you have actually done some
> analysis and that this is the only place in add_line_info that needs to
> do something special with end_sequence.

We can only 2 lines pointing to the same address: one is end_sequence
and the other isn't. That is the only place where it can happen.



H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Thu, Mar 02, 2006 at 09:16:35PM -0800, H. J. Lu wrote:
> On Fri, Mar 03, 2006 at 01:12:09PM +1030, Alan Modra wrote:
> > On Tue, Feb 28, 2006 at 11:41:38AM -0800, H. J. Lu wrote:
> > > @@ -803,6 +815,15 @@ add_line_info (struct line_info_table *t
> > >        {
> > >  	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
> > >  	   list and 2) the head of 'info'.  */
> > > +	if (address == table->lcl_head->prev_line->address
> > > +	    && end_sequence != 0
> > > +	    && table->lcl_head->end_sequence != 0
> > > +	    && table->lcl_head->prev_line->end_sequence == 0)
> > > +	  {
> > > +	    /* If the new adress is the same, we need to make sure that
> > > +	       the order of end_sequence is correct.  */
> > > +	    table->lcl_head = table->lcl_head->prev_line;
> > > +	  }
> > >  	info->prev_line = table->lcl_head->prev_line;
> > >  	table->lcl_head->prev_line = info;
> > >  	break;
> > 
> > Convince me that this is correct.  ie. that you have actually done some
> > analysis and that this is the only place in add_line_info that needs to
> > do something special with end_sequence.
> 
> We can only 2 lines pointing to the same address: one is end_sequence
> and the other isn't. That is the only place where it can happen.

"That is the only place where it can happen" isn't good enough.  You are
just making an assertion.  Prove it.

Yes, I can probably analyse add_line_info myself, and decide whether
your patch is correct.  This requires some work on my part, and, it's
duplicating effort you have already made if you have done the analysis.
Of course, if you haven't properly analysed the function yourself, then
it's likely that your patch is wrong or incomplete.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Fri, Mar 03, 2006 at 06:44:11PM +1030, Alan Modra wrote:
> On Thu, Mar 02, 2006 at 09:16:35PM -0800, H. J. Lu wrote:
> > On Fri, Mar 03, 2006 at 01:12:09PM +1030, Alan Modra wrote:
> > > On Tue, Feb 28, 2006 at 11:41:38AM -0800, H. J. Lu wrote:
> > > > @@ -803,6 +815,15 @@ add_line_info (struct line_info_table *t
> > > >        {
> > > >  	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
> > > >  	   list and 2) the head of 'info'.  */
> > > > +	if (address == table->lcl_head->prev_line->address
> > > > +	    && end_sequence != 0
> > > > +	    && table->lcl_head->end_sequence != 0
> > > > +	    && table->lcl_head->prev_line->end_sequence == 0)
> > > > +	  {
> > > > +	    /* If the new adress is the same, we need to make sure that
> > > > +	       the order of end_sequence is correct.  */
> > > > +	    table->lcl_head = table->lcl_head->prev_line;
> > > > +	  }
> > > >  	info->prev_line = table->lcl_head->prev_line;
> > > >  	table->lcl_head->prev_line = info;
> > > >  	break;
> > > 
> > > Convince me that this is correct.  ie. that you have actually done some
> > > analysis and that this is the only place in add_line_info that needs to
> > > do something special with end_sequence.
> > 
> > We can only 2 lines pointing to the same address: one is end_sequence
> > and the other isn't. That is the only place where it can happen.
> 
> "That is the only place where it can happen" isn't good enough.  You are
> just making an assertion.  Prove it.
> 
> Yes, I can probably analyse add_line_info myself, and decide whether
> your patch is correct.  This requires some work on my part, and, it's
> duplicating effort you have already made if you have done the analysis.
> Of course, if you haven't properly analysed the function yourself, then
> it's likely that your patch is wrong or incomplete.
> 

We need to check end_sequence when we compare line addresses. I added
compare_line_address to do that. Here is the new patch.



H.J.
---
2006-03-03  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2338
	* dwarf2.c (loadable_section): New struct.
	(dwarf2_debug): Add loadable_section_count and
	loadable_sections.
	(compare_line_address): New.
	(add_line_info): Use compare_line_address to compare line
	addresses.
	(check_function_name): Removed.
	(unset_sections): New.
	(place_sections): New.
	(_bfd_dwarf2_find_nearest_line): Updated. Call place_sections
	and unset_sections on relocatable files.
	(_bfd_dwarf2_find_line): Likewise.

--- bfd/dwarf2.c.rel	2006-02-16 10:08:05.000000000 -0800
+++ bfd/dwarf2.c	2006-03-03 09:41:57.000000000 -0800
@@ -74,6 +74,12 @@ struct dwarf_block
   bfd_byte *data;
 };
 
+struct loadable_section
+{
+  asection *section;
+  bfd_vma adj_vma;
+};
+
 struct dwarf2_debug
 {
   /* A list of all previously read comp_units.  */
@@ -124,6 +130,12 @@ struct dwarf2_debug
      calling chain for subsequent calls to bfd_find_inliner_info to
      use. */
   struct funcinfo *inliner_chain;
+
+  /* Number of loadable sections.  */
+  unsigned int loadable_section_count;
+
+  /* Array of loadable sections.  */
+  struct loadable_section *loadable_sections;
 };
 
 struct arange
@@ -744,6 +756,25 @@ struct varinfo
   unsigned int stack: 1;
 };
 
+/* 2 lines can have the same address. But their end_sequence values
+   must be different.  The one with end_sequence != 0 will never be
+   reached.  The one with end_sequence == 0 is real.  */
+
+static bfd_boolean
+compare_line_address (bfd_vma address, int end_sequence,
+		      struct line_info *line)
+{
+  if (address == line->address && end_sequence == line->end_sequence)
+    /* FIXME: Will we ever get here?  */
+    abort ();
+  else if (address > line->address
+	   || (address == line->address
+	       && end_sequence < line->end_sequence))
+    return 1;
+  else
+    return -1;
+}
+
 /* Adds a new entry to the line_info list in the line_info_table, ensuring
    that the list is sorted.  Note that the line_info list is sorted from
    highest to lowest VMA (with possible duplicates); that is,
@@ -777,7 +808,8 @@ add_line_info (struct line_info_table *t
 
   while (1)
     if (!table->last_line
-	|| address >= table->last_line->address)
+	|| compare_line_address (address, end_sequence,
+				 table->last_line) > 0)
       {
 	/* Normal case: add 'info' to the beginning of the list */
 	info->prev_line = table->last_line;
@@ -789,7 +821,8 @@ add_line_info (struct line_info_table *t
 	break;
       }
     else if (!table->lcl_head->prev_line
-	     && table->lcl_head->address > address)
+	     && compare_line_address (address, end_sequence,
+				      table->lcl_head) < 0)
       {
 	/* Abnormal but easy: lcl_head is 1) at the *end* of the line
 	   list and 2) the head of 'info'.  */
@@ -798,8 +831,10 @@ add_line_info (struct line_info_table *t
 	break;
       }
     else if (table->lcl_head->prev_line
-	     && table->lcl_head->address > address
-	     && address >= table->lcl_head->prev_line->address)
+	     && compare_line_address (address, end_sequence,
+				      table->lcl_head) < 0
+	     && compare_line_address (address, end_sequence,
+				      table->lcl_head->prev_line) > 0)
       {
 	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
 	   list and 2) the head of 'info'.  */
@@ -816,7 +851,9 @@ add_line_info (struct line_info_table *t
 
 	while (li1)
 	  {
-	    if (li2->address > address && address >= li1->address)
+	    if (compare_line_address (address, end_sequence, li2) < 0
+		&& compare_line_address (address, end_sequence,
+					 li1) > 0)
 	      break;
 
 	    li2 = li1; /* always non-NULL */
@@ -2179,28 +2216,91 @@ find_debug_info (bfd *abfd, asection *af
   return NULL;
 }
 
-/* Return TRUE if there is no mismatch bewteen function FUNC and
-   section SECTION from symbol table SYMBOLS in ABFD.  */
+/* Unset vmas for loadable sections in STASH.  */
+
+static void
+unset_sections (struct dwarf2_debug *stash)
+{
+  unsigned int i;
+  struct loadable_section *p;
+
+  i = stash->loadable_section_count;
+  p = stash->loadable_sections;
+  for (; i > 0; i--, p++)
+    p->section->vma = 0;
+}
+
+/* Set unique vmas for loadable sections in ABFD and save vmas in
+   STASH for unset_sections.  */
 
 static bfd_boolean
-check_function_name (bfd *abfd, asection *section, asymbol **symbols,
-		     const char *func)
+place_sections (bfd *abfd, struct dwarf2_debug *stash)
 {
-  /* Mismatch can only happen when we have 2 functions with the same
-     address. It can only occur in a relocatable file.  */
-  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
-      && func != NULL
-      && section != NULL
-      && symbols != NULL)
-    {
-      asymbol **p;
-
-      for (p = symbols; *p != NULL; p++)
-	{
-	  if (((*p)->flags & BSF_FUNCTION) != 0
-	      && (*p)->name != NULL
-	      && strcmp ((*p)->name, func) == 0)
-	    return (*p)->section == section;
+  struct loadable_section *p;
+  unsigned int i;
+
+  if (stash->loadable_section_count != 0)
+    {
+      i = stash->loadable_section_count;
+      p = stash->loadable_sections;
+      for (; i > 0; i--, p++)
+	p->section->vma = p->adj_vma;
+    }
+  else
+    {
+      asection *sect;
+      bfd_vma last_vma = 0;
+      bfd_size_type amt;
+      struct loadable_section *p;
+
+      i = 0;
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if (sect->vma != 0 || (sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  i++;
+	}
+
+      amt = i * sizeof (struct loadable_section);
+      p = (struct loadable_section *) bfd_zalloc (abfd, amt);
+      if (! p)
+	return FALSE;
+
+      stash->loadable_sections = p;
+      stash->loadable_section_count = i;
+
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	{
+	  bfd_size_type sz;
+
+	  if (sect->vma != 0 || (sect->flags & SEC_LOAD) == 0)
+	    continue;
+
+	  sz = sect->rawsize ? sect->rawsize : sect->size;
+	  if (sz == 0)
+	    continue;
+
+	  p->section = sect;
+	  if (last_vma != 0)
+	    {
+	      /* Align the new address to the current section
+		 alignment.  */
+	      last_vma = ((last_vma
+			   + ~((bfd_vma) -1 << sect->alignment_power))
+			  & ((bfd_vma) -1 << sect->alignment_power));
+	      sect->vma = last_vma;
+	    }
+	  p->adj_vma = sect->vma;
+	  last_vma += sect->vma + sz;
+
+	  p++;
 	}
     }
 
@@ -2239,7 +2339,27 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
   struct comp_unit* each;
 
+  bfd_vma found = FALSE;
+
   stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = offset;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2256,15 +2376,10 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     addr_size = 4;
   BFD_ASSERT (addr_size == 4 || addr_size == 8);
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2273,7 +2388,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2285,7 +2400,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2319,7 +2434,7 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2328,10 +2443,11 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
     if (comp_unit_contains_address (each, addr)
 	&& comp_unit_find_nearest_line (each, addr, filename_ptr,
 					functionname_ptr,
-					linenumber_ptr, stash)
-	&& check_function_name (abfd, section, symbols,
-				*functionname_ptr))
-      return TRUE;
+					linenumber_ptr, stash))
+      {
+	found = TRUE;
+	goto done;
+      }
 
   /* Read each remaining comp. units checking each as they are read.  */
   while (stash->info_ptr < stash->info_ptr_end)
@@ -2398,15 +2514,20 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 						  filename_ptr,
 						  functionname_ptr,
 						  linenumber_ptr,
-						  stash)
-		  && check_function_name (abfd, section, symbols,
-					  *functionname_ptr))
-		return TRUE;
+						  stash))
+		{
+		  found = TRUE;
+		  goto done;
+		}
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 /* The DWARF2 version of find_line.  Return TRUE if the line is found
@@ -2438,10 +2559,29 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
   asection *section;
 
-  bfd_boolean found;
+  bfd_boolean found = FALSE;
 
   section = bfd_get_section (symbol);
 
+  stash = *pinfo;
+
+  if (! stash)
+    {
+      bfd_size_type amt = sizeof (struct dwarf2_debug);
+
+      stash = bfd_zalloc (abfd, amt);
+      if (! stash)
+	return FALSE;
+    }
+
+  /* In a relocatable file, 2 functions may have the same address.
+     We change the section vma so that they won't overlap.  */
+  if (!stash && (abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    {
+      if (! place_sections (abfd, stash))
+	return FALSE;
+    }
+
   addr = symbol->value;
   if (section->output_section)
     addr += section->output_section->vma + section->output_offset;
@@ -2449,19 +2589,13 @@ _bfd_dwarf2_find_line (bfd *abfd,
     addr += section->vma;
 
   *filename_ptr = NULL;
-  stash = *pinfo;
   *filename_ptr = NULL;
   *linenumber_ptr = 0;
 
-  if (! stash)
+  if (! *pinfo)
     {
       bfd_size_type total_size;
       asection *msec;
-      bfd_size_type amt = sizeof (struct dwarf2_debug);
-
-      stash = bfd_zalloc (abfd, amt);
-      if (! stash)
-	return FALSE;
 
       *pinfo = stash;
 
@@ -2470,7 +2604,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	/* No dwarf2 info.  Note that at this point the stash
 	   has been allocated, but contains zeros, this lets
 	   future calls to this function fail quicker.  */
-	 return FALSE;
+	goto done;
 
       /* There can be more than one DWARF2 info section in a BFD these days.
 	 Read them all in and produce one large stash.  We do this in two
@@ -2482,7 +2616,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
       stash->info_ptr = bfd_alloc (abfd, total_size);
       if (stash->info_ptr == NULL)
-	return FALSE;
+	goto done;
 
       stash->info_ptr_end = stash->info_ptr;
 
@@ -2516,7 +2650,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
   /* A null info_ptr indicates that there is no dwarf2 info
      (or that an error occured while setting up the stash).  */
   if (! stash->info_ptr)
-    return FALSE;
+    goto done;
 
   stash->inliner_chain = NULL;
 
@@ -2528,7 +2662,7 @@ _bfd_dwarf2_find_line (bfd *abfd,
 	found = comp_unit_find_line (each, symbol, addr, filename_ptr,
 				     linenumber_ptr, stash);
 	if (found)
-	  return found;
+	  goto done;
       }
 
   /* The DWARF2 spec says that the initial length field, and the
@@ -2605,12 +2739,16 @@ _bfd_dwarf2_find_line (bfd *abfd,
 					       linenumber_ptr,
 					       stash));
 	      if (found)
-		return TRUE;
+		goto done;
 	    }
 	}
     }
 
-  return FALSE;
+done:
+  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0)
+    unset_sections (stash);
+
+  return found;
 }
 
 bfd_boolean

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Fri, Mar 03, 2006 at 09:46:39AM -0800, H. J. Lu wrote:
> We need to check end_sequence when we compare line addresses.

Much better.

> +/* 2 lines can have the same address. But their end_sequence values
> +   must be different.  The one with end_sequence != 0 will never be
> +   reached.  The one with end_sequence == 0 is real.  */
> +
> +static bfd_boolean
> +compare_line_address (bfd_vma address, int end_sequence,
> +		      struct line_info *line)
> +{
> +  if (address == line->address && end_sequence == line->end_sequence)
> +    /* FIXME: Will we ever get here?  */
> +    abort ();
> +  else if (address > line->address
> +	   || (address == line->address
> +	       && end_sequence < line->end_sequence))
> +    return 1;
> +  else
> +    return -1;
> +}

Other comments in add_line_info indicate that you might need to handle
duplicate line info.  So I believe the compare_line_address function
comment and the abort are incorrect.

I'd rather see something like the following (untested!).  If this works
for you, then this add_line_info patch along with the rest of your patch
is OK to commit.

Index: bfd/dwarf2.c
===================================================================
RCS file: /cvs/src/src/bfd/dwarf2.c,v
retrieving revision 1.84
diff -u -p -r1.84 dwarf2.c
--- bfd/dwarf2.c	15 Feb 2006 22:29:42 -0000	1.84
+++ bfd/dwarf2.c	5 Mar 2006 22:37:36 -0000
@@ -744,6 +744,17 @@ struct varinfo
   unsigned int stack: 1;
 };
 
+/* Return TRUE if NEW_LINE should sort after LINE.  */
+
+static inline bfd_boolean
+new_line_sorts_after (struct line_info *new_line, struct line_info *line)
+{
+  return (new_line->address > line->address
+	  || (new_line->address == line->address
+	      && new_line->end_sequence < line->end_sequence));
+}
+
+
 /* Adds a new entry to the line_info list in the line_info_table, ensuring
    that the list is sorted.  Note that the line_info list is sorted from
    highest to lowest VMA (with possible duplicates); that is,
@@ -760,6 +771,21 @@ add_line_info (struct line_info_table *t
   bfd_size_type amt = sizeof (struct line_info);
   struct line_info* info = bfd_alloc (table->abfd, amt);
 
+  /* Set member data of 'info'.  */
+  info->address = address;
+  info->line = line;
+  info->column = column;
+  info->end_sequence = end_sequence;
+
+  if (filename && filename[0])
+    {
+      info->filename = bfd_alloc (table->abfd, strlen (filename) + 1);
+      if (info->filename)
+	strcpy (info->filename, filename);
+    }
+  else
+    info->filename = NULL;
+
   /* Find the correct location for 'info'.  Normally we will receive
      new line_info data 1) in order and 2) with increasing VMAs.
      However some compilers break the rules (cf. decode_line_info) and
@@ -777,7 +803,7 @@ add_line_info (struct line_info_table *t
 
   while (1)
     if (!table->last_line
-	|| address >= table->last_line->address)
+	|| new_line_sorts_after (info, table->last_line))
       {
 	/* Normal case: add 'info' to the beginning of the list */
 	info->prev_line = table->last_line;
@@ -789,7 +815,7 @@ add_line_info (struct line_info_table *t
 	break;
       }
     else if (!table->lcl_head->prev_line
-	     && table->lcl_head->address > address)
+	     && !new_line_sorts_after (info, table->lcl_head))
       {
 	/* Abnormal but easy: lcl_head is 1) at the *end* of the line
 	   list and 2) the head of 'info'.  */
@@ -798,8 +824,8 @@ add_line_info (struct line_info_table *t
 	break;
       }
     else if (table->lcl_head->prev_line
-	     && table->lcl_head->address > address
-	     && address >= table->lcl_head->prev_line->address)
+	     && !new_line_sorts_after (info, table->lcl_head)
+	     && new_line_sorts_after (info, table->lcl_head->prev_line))
       {
 	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
 	   list and 2) the head of 'info'.  */
@@ -816,7 +842,8 @@ add_line_info (struct line_info_table *t
 
 	while (li1)
 	  {
-	    if (li2->address > address && address >= li1->address)
+	    if (!new_line_sorts_after (info, li2)
+		&& new_line_sorts_after (info, li1))
 	      break;
 
 	    li2 = li1; /* always non-NULL */
@@ -824,21 +851,6 @@ add_line_info (struct line_info_table *t
 	  }
 	table->lcl_head = li2;
       }
-
-  /* Set member data of 'info'.  */
-  info->address = address;
-  info->line = line;
-  info->column = column;
-  info->end_sequence = end_sequence;
-
-  if (filename && filename[0])
-    {
-      info->filename = bfd_alloc (table->abfd, strlen (filename) + 1);
-      if (info->filename)
-	strcpy (info->filename, filename);
-    }
-  else
-    info->filename = NULL;
 }
 
 /* Extract a fully qualified filename from a line info table.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Mon, Mar 06, 2006 at 09:15:36AM +1030, Alan Modra wrote:
> I'd rather see something like the following (untested!).  If this works
> for you, then this add_line_info patch along with the rest of your patch
> is OK to commit.

Hmm, I see there isn't any real need for the outer loop in
add_line_info.  I'll make a follow-up patch after you commit this one.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[H. J. Lu]

On Mon, Mar 06, 2006 at 10:11:30AM +1030, Alan Modra wrote:
> On Mon, Mar 06, 2006 at 09:15:36AM +1030, Alan Modra wrote:
> > I'd rather see something like the following (untested!).  If this works
> > for you, then this add_line_info patch along with the rest of your patch
> > is OK to commit.
> 
> Hmm, I see there isn't any real need for the outer loop in
> add_line_info.  I'll make a follow-up patch after you commit this one.
> 

I checked in the updated patch with your change.

Thanks.


H.J.

Re: PATCH: ld/2338: objdump -d -l doesn't work correctly

[Alan Modra]

On Sun, Mar 05, 2006 at 05:37:29PM -0800, H. J. Lu wrote:
> On Mon, Mar 06, 2006 at 10:11:30AM +1030, Alan Modra wrote:
> > Hmm, I see there isn't any real need for the outer loop in
> > add_line_info.  I'll make a follow-up patch after you commit this one.
> 
> I checked in the updated patch with your change.
> 

OK, here's the outer loop removal.  Fairly obvious, really.  The only
time we do anything but break out of the outer loop immediately is the
"abnormal and hard" case where we search for a new lcl_head.  On
entering the loop again, the new lcl_head must match one of the
"abnormal but easy" cases (which I've merged), so we can simply perform
the "abnormal but easy" actions in the "abnormal and hard" branch.

	* dwarf2.c: Formatting.
	(add_line_info): Remove outer loop.

Index: bfd/dwarf2.c
===================================================================
RCS file: /cvs/src/src/bfd/dwarf2.c,v
retrieving revision 1.85
diff -c -p -r1.85 dwarf2.c
*** bfd/dwarf2.c	6 Mar 2006 01:36:52 -0000	1.85
--- bfd/dwarf2.c	6 Mar 2006 04:48:00 -0000
***************
*** 1,6 ****
  /* DWARF 2 support.
     Copyright 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003,
!    2004, 2005 Free Software Foundation, Inc.
  
     Adapted from gdb/dwarf2read.c by Gavin Koch of Cygnus Solutions
     (gavin@cygnus.com).
--- 1,6 ----
  /* DWARF 2 support.
     Copyright 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003,
!    2004, 2005, 2006 Free Software Foundation, Inc.
  
     Adapted from gdb/dwarf2read.c by Gavin Koch of Cygnus Solutions
     (gavin@cygnus.com).
*************** read_abbrevs (bfd *abfd, bfd_uint64_t of
*** 493,513 ****
  	      amt *= sizeof (struct attr_abbrev);
  	      tmp = bfd_realloc (cur_abbrev->attrs, amt);
  	      if (tmp == NULL)
! 	        {
! 	          size_t i;
  
! 	          for (i = 0; i < ABBREV_HASH_SIZE; i++)
! 	            {
! 	            struct abbrev_info *abbrev = abbrevs[i];
! 
! 	            while (abbrev)
! 	              {
! 	                free (abbrev->attrs);
! 	                abbrev = abbrev->next;
! 	              }
! 	            }
! 	          return NULL;
! 	        }
  	      cur_abbrev->attrs = tmp;
  	    }
  
--- 493,513 ----
  	      amt *= sizeof (struct attr_abbrev);
  	      tmp = bfd_realloc (cur_abbrev->attrs, amt);
  	      if (tmp == NULL)
! 		{
! 		  size_t i;
! 
! 		  for (i = 0; i < ABBREV_HASH_SIZE; i++)
! 		    {
! 		      struct abbrev_info *abbrev = abbrevs[i];
  
! 		      while (abbrev)
! 			{
! 			  free (abbrev->attrs);
! 			  abbrev = abbrev->next;
! 			}
! 		    }
! 		  return NULL;
! 		}
  	      cur_abbrev->attrs = tmp;
  	    }
  
*************** read_abbrevs (bfd *abfd, bfd_uint64_t of
*** 533,539 ****
  	 for the next compile unit) or if the end of the abbreviation
  	 table is reached.  */
        if ((unsigned int) (abbrev_ptr - stash->dwarf_abbrev_buffer)
! 	    >= stash->dwarf_abbrev_size)
  	break;
        abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
        abbrev_ptr += bytes_read;
--- 533,539 ----
  	 for the next compile unit) or if the end of the abbreviation
  	 table is reached.  */
        if ((unsigned int) (abbrev_ptr - stash->dwarf_abbrev_buffer)
! 	  >= stash->dwarf_abbrev_size)
  	break;
        abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
        abbrev_ptr += bytes_read;
*************** add_line_info (struct line_info_table *t
*** 813,868 ****
  
       Note: we may receive duplicate entries from 'decode_line_info'.  */
  
!   while (1)
!     if (!table->last_line
! 	|| new_line_sorts_after (info, table->last_line))
!       {
! 	/* Normal case: add 'info' to the beginning of the list */
! 	info->prev_line = table->last_line;
! 	table->last_line = info;
! 
! 	/* lcl_head: initialize to head a *possible* sequence at the end.  */
! 	if (!table->lcl_head)
! 	  table->lcl_head = info;
! 	break;
!       }
!     else if (!table->lcl_head->prev_line
! 	     && !new_line_sorts_after (info, table->lcl_head))
!       {
! 	/* Abnormal but easy: lcl_head is 1) at the *end* of the line
! 	   list and 2) the head of 'info'.  */
! 	info->prev_line = NULL;
! 	table->lcl_head->prev_line = info;
! 	break;
!       }
!     else if (table->lcl_head->prev_line
! 	     && !new_line_sorts_after (info, table->lcl_head)
! 	     && new_line_sorts_after (info, table->lcl_head->prev_line))
!       {
! 	/* Abnormal but easy: lcl_head is 1) in the *middle* of the line
! 	   list and 2) the head of 'info'.  */
! 	info->prev_line = table->lcl_head->prev_line;
! 	table->lcl_head->prev_line = info;
! 	break;
!       }
!     else
!       {
! 	/* Abnormal and hard: Neither 'last_line' nor 'lcl_head' are valid
! 	   heads for 'info'.  Reset 'lcl_head' and repeat.  */
! 	struct line_info* li2 = table->last_line; /* always non-NULL */
! 	struct line_info* li1 = li2->prev_line;
  
! 	while (li1)
! 	  {
! 	    if (!new_line_sorts_after (info, li2)
! 		&& new_line_sorts_after (info, li1))
! 	      break;
  
! 	    li2 = li1; /* always non-NULL */
! 	    li1 = li1->prev_line;
! 	  }
! 	table->lcl_head = li2;
!       }
  }
  
  /* Extract a fully qualified filename from a line info table.
--- 813,857 ----
  
       Note: we may receive duplicate entries from 'decode_line_info'.  */
  
!   if (!table->last_line
!       || new_line_sorts_after (info, table->last_line))
!     {
!       /* Normal case: add 'info' to the beginning of the list */
!       info->prev_line = table->last_line;
!       table->last_line = info;
! 
!       /* lcl_head: initialize to head a *possible* sequence at the end.  */
!       if (!table->lcl_head)
! 	table->lcl_head = info;
!     }
!   else if (!new_line_sorts_after (info, table->lcl_head)
! 	   && (!table->lcl_head->prev_line
! 	       || new_line_sorts_after (info, table->lcl_head->prev_line)))
!     {
!       /* Abnormal but easy: lcl_head is the head of 'info'.  */
!       info->prev_line = table->lcl_head->prev_line;
!       table->lcl_head->prev_line = info;
!     }
!   else
!     {
!       /* Abnormal and hard: Neither 'last_line' nor 'lcl_head' are valid
! 	 heads for 'info'.  Reset 'lcl_head'.  */
!       struct line_info* li2 = table->last_line; /* always non-NULL */
!       struct line_info* li1 = li2->prev_line;
  
!       while (li1)
! 	{
! 	  if (!new_line_sorts_after (info, li2)
! 	      && new_line_sorts_after (info, li1))
! 	    break;
  
! 	  li2 = li1; /* always non-NULL */
! 	  li1 = li1->prev_line;
! 	}
!       table->lcl_head = li2;
!       info->prev_line = table->lcl_head->prev_line;
!       table->lcl_head->prev_line = info;
!     }
  }
  
  /* Extract a fully qualified filename from a line info table.
*************** decode_line_info (struct comp_unit *unit
*** 1192,1203 ****
  		      amt *= sizeof (struct fileinfo);
  		      tmp = bfd_realloc (table->files, amt);
  		      if (tmp == NULL)
! 		        {
  			  free (table->files);
  			  free (table->dirs);
  			  free (filename);
  			  return NULL;
! 		        }
  		      table->files = tmp;
  		    }
  		  table->files[table->num_files].name = cur_file;
--- 1181,1192 ----
  		      amt *= sizeof (struct fileinfo);
  		      tmp = bfd_realloc (table->files, amt);
  		      if (tmp == NULL)
! 			{
  			  free (table->files);
  			  free (table->dirs);
  			  free (filename);
  			  return NULL;
! 			}
  		      table->files = tmp;
  		    }
  		  table->files[table->num_files].name = cur_file;
*************** read_rangelist (struct comp_unit *unit, 
*** 1609,1615 ****
  	return;
      }
    ranges_ptr = unit->stash->dwarf_ranges_buffer + offset;
!     
    for (;;)
      {
        bfd_vma low_pc;
--- 1598,1604 ----
  	return;
      }
    ranges_ptr = unit->stash->dwarf_ranges_buffer + offset;
! 
    for (;;)
      {
        bfd_vma low_pc;
*************** scan_unit_for_symbols (struct comp_unit 
*** 1826,1832 ****
  						 attr.u.blk->data + 1);
  			}
  		      break;
! 		    
  		    default:
  		      break;
  		    }
--- 1815,1821 ----
  						 attr.u.blk->data + 1);
  			}
  		      break;
! 
  		    default:
  		      break;
  		    }
*************** _bfd_dwarf2_cleanup_debug_info (bfd *abf
*** 2784,2804 ****
        size_t i;
  
        for (i = 0; i < ABBREV_HASH_SIZE; i++)
!         {
!           struct abbrev_info *abbrev = abbrevs[i];
  
!           while (abbrev)
!             {
!               free (abbrev->attrs);
!               abbrev = abbrev->next;
!             }
!         }
  
        if (each->line_table)
!         {
!           free (each->line_table->dirs);
!           free (each->line_table->files);
!         }
      }
  
    free (stash->dwarf_abbrev_buffer);
--- 2773,2793 ----
        size_t i;
  
        for (i = 0; i < ABBREV_HASH_SIZE; i++)
! 	{
! 	  struct abbrev_info *abbrev = abbrevs[i];
  
! 	  while (abbrev)
! 	    {
! 	      free (abbrev->attrs);
! 	      abbrev = abbrev->next;
! 	    }
! 	}
  
        if (each->line_table)
! 	{
! 	  free (each->line_table->dirs);
! 	  free (each->line_table->files);
! 	}
      }
  
    free (stash->dwarf_abbrev_buffer);