PATCH: binutils/2096: addr2line fails on linux-kernel

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

PATCH: binutils/2096: addr2line fails on linux-kernel

H.J. Lu-27
There are 2 bugs in dwarf2.c. BFD section's lvm is not very useful
for ELF since ELF section doesn't have it and it is made up by bfd:

[hjl@gnu-3 addr2line]$ readelf -S vmlinux
There are 41 section headers, starting at offset 0x20f06bc:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        c0100000 001000 226f60 00  AX  0   0 16
...
[hjl@gnu-3 addr2line]$ objdump -h  vmlinux

vmlinux:     file format elf32-i386

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         00226f60  c0100000  00100000  00001000  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

dwarf2.c should use vma for ELF.

The section bug is when comp_unit_contains_address return TRUE,
it doesn't necessary mean a compilation unit really contains the
given address. It may happen when "asm" is used to define functions
like arch/i386/kernel/semaphore.c in the Linux 2.6.14 kernel. My
patch checks the line info table to make sure that a compilation
unit really contains the given address.


H.J.

bfd-dwarf-addr-2.patch (1K) Download Attachment
bfd-dwarf-unit-1.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: binutils/2096: addr2line fails on linux-kernel

H.J. Lu-27
On Sat, Dec 31, 2005 at 12:25:44PM -0800, H. J. Lu wrote:
> There are 2 bugs in dwarf2.c. BFD section's lvm is not very useful
> for ELF since ELF section doesn't have it and it is made up by bfd:
>

Could someone comment on this patch

http://sourceware.org/ml/binutils/2005-12/msg00290.html


H.J.
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: binutils/2096: addr2line fails on linux-kernel

Nick Clifton
Hi H.J.

> Could someone comment on this patch
>
> http://sourceware.org/ml/binutils/2005-12/msg00290.html

Sorry - it was in my queue, but I am behind in my replies.  Anyway:


 > 2005-12-31  H.J. Lu  <[hidden email]>
 >
 > PR binutils/2096
 > * dwarf2.c (comp_unit_contains_address): Update comment.
 > (_bfd_dwarf2_find_nearest_line): Return TRUE only if both
 > comp_unit_contains_address and comp_unit_find_nearest_line
 > return TRUE.


This patch is OK, but please could you extend the comment at the start
of the comp_unit_contains_address() function - as I understand it the
reason that we need to consult the line info table is that we cannot
always trust the compilation unit header information.  In particular
compilation units which contain hand coded assembler may be unreliable.
  Yes ?  If so, please could you add a sentence to this effect into the
comment ?


 > 2005-12-31  H.J. Lu  <[hidden email]>
 >
 > PR binutils/2096
 > * dwarf2.c (_bfd_dwarf2_find_nearest_line): Use section's vma,
 > instead of lvm for ELF.

This patch I am not so sure about.  Why would we want to use the LMA (or
LVM) values for non-ELF formats ?  Shouldn't we always be using the VMA
values ?  I am assuming here that the debug info is always going to
relate to the run-time addresses of the executable, not the load-time
addresses.  Can you think of a situation where the load-time addresses
might be used.

Assuming that you agree with me, please could you submit a revised patch
which changes this function over to using VMAs instead of LMAs for all
targets, not just ELF based ones.

Cheers
   Nick
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: binutils/2096: addr2line fails on linux-kernel

H.J. Lu-27
On Mon, Jan 16, 2006 at 04:42:17PM +0000, Nick Clifton wrote:

> Hi H.J.
>
> >Could someone comment on this patch
> >
> >http://sourceware.org/ml/binutils/2005-12/msg00290.html
>
> Sorry - it was in my queue, but I am behind in my replies.  Anyway:
>
>
> > 2005-12-31  H.J. Lu  <[hidden email]>
> >
> > PR binutils/2096
> > * dwarf2.c (comp_unit_contains_address): Update comment.
> > (_bfd_dwarf2_find_nearest_line): Return TRUE only if both
> > comp_unit_contains_address and comp_unit_find_nearest_line
> > return TRUE.
>
>
> This patch is OK, but please could you extend the comment at the start
> of the comp_unit_contains_address() function - as I understand it the
> reason that we need to consult the line info table is that we cannot
> always trust the compilation unit header information.  In particular
> compilation units which contain hand coded assembler may be unreliable.
>  Yes ?  If so, please could you add a sentence to this effect into the
> comment ?
>
>
> > 2005-12-31  H.J. Lu  <[hidden email]>
> >
> > PR binutils/2096
> > * dwarf2.c (_bfd_dwarf2_find_nearest_line): Use section's vma,
> > instead of lvm for ELF.
>
> This patch I am not so sure about.  Why would we want to use the LMA (or
> LVM) values for non-ELF formats ?  Shouldn't we always be using the VMA
> values ?  I am assuming here that the debug info is always going to
> relate to the run-time addresses of the executable, not the load-time
> addresses.  Can you think of a situation where the load-time addresses
> might be used.
>
> Assuming that you agree with me, please could you submit a revised patch
> which changes this function over to using VMAs instead of LMAs for all
> targets, not just ELF based ones.
>

Here is the updated patch. I combined 2 into 1.


H.J.
----
2006-01-16  H.J. Lu  <[hidden email]>

        PR binutils/2096
        * dwarf2.c (comp_unit_contains_address): Update comment.
        (_bfd_dwarf2_find_nearest_line): Use section's vma, instead of
        lvm.  Return TRUE only if both comp_unit_contains_address and
        comp_unit_find_nearest_line return TRUE.
        (_bfd_dwarf2_find_line): Use section's vma, instead of lvm.

--- bfd/dwarf2.c.addr 2005-12-31 08:46:11.000000000 -0800
+++ bfd/dwarf2.c 2006-01-16 10:23:06.000000000 -0800
@@ -2006,7 +2006,11 @@ parse_comp_unit (bfd *abfd,
   return unit;
 }
 
-/* Return TRUE if UNIT contains the address given by ADDR.  */
+/* Return TRUE if UNIT may contain the address given by ADDR.  When
+   there are functions written entirely with inline asm statements, the
+   range info in the compilation unit header may not be correct.  We
+   need to consult the line info table to see if a compilation unit
+   really contains the given address.  */
 
 static bfd_boolean
 comp_unit_contains_address (struct comp_unit *unit, bfd_vma addr)
@@ -2210,9 +2214,9 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
   stash = *pinfo;
   addr = offset;
   if (section->output_section)
-    addr += section->output_section->lma + section->output_offset;
+    addr += section->output_section->vma + section->output_offset;
   else
-    addr += section->lma;
+    addr += section->vma;
   *filename_ptr = NULL;
   *functionname_ptr = NULL;
   *linenumber_ptr = 0;
@@ -2293,16 +2297,16 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
 
   /* Check the previously read comp. units first.  */
   for (each = stash->all_comp_units; each; each = each->next_unit)
-    if (comp_unit_contains_address (each, addr))
-      return comp_unit_find_nearest_line (each, addr, filename_ptr,
-  functionname_ptr, linenumber_ptr,
-  stash);
+    if (comp_unit_contains_address (each, addr)
+ && comp_unit_find_nearest_line (each, addr, filename_ptr,
+ functionname_ptr,
+ linenumber_ptr, stash))
+      return TRUE;
 
   /* Read each remaining comp. units checking each as they are read.  */
   while (stash->info_ptr < stash->info_ptr_end)
     {
       bfd_vma length;
-      bfd_boolean found;
       unsigned int offset_size = addr_size;
       bfd_byte *info_ptr_unit = stash->info_ptr;
 
@@ -2358,25 +2362,14 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd
  unit->high == 0), we need to consult the line info
  table to see if a compilation unit contains the given
  address.  */
-      if (each->arange.high > 0)
- {
-  if (comp_unit_contains_address (each, addr))
-    return comp_unit_find_nearest_line (each, addr,
- filename_ptr,
- functionname_ptr,
- linenumber_ptr,
- stash);
- }
-      else
- {
-  found = comp_unit_find_nearest_line (each, addr,
-       filename_ptr,
-       functionname_ptr,
-       linenumber_ptr,
-       stash);
-  if (found)
-    return TRUE;
- }
+      if ((each->arange.high == 0
+   || comp_unit_contains_address (each, addr))
+  && comp_unit_find_nearest_line (each, addr,
+  filename_ptr,
+  functionname_ptr,
+  linenumber_ptr,
+  stash))
+ return TRUE;
     }
  }
     }
@@ -2419,9 +2412,9 @@ _bfd_dwarf2_find_line (bfd *abfd,
 
   addr = symbol->value;
   if (section->output_section)
-    addr += section->output_section->lma + section->output_offset;
+    addr += section->output_section->vma + section->output_offset;
   else
-    addr += section->lma;
+    addr += section->vma;
 
   *filename_ptr = NULL;
   stash = *pinfo;
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: binutils/2096: addr2line fails on linux-kernel

Andreas Schwab
"H. J. Lu" <[hidden email]> writes:

> PR binutils/2096
> * dwarf2.c (comp_unit_contains_address): Update comment.
> (_bfd_dwarf2_find_nearest_line): Use section's vma, instead of
> lvm.  Return TRUE only if both comp_unit_contains_address and
> comp_unit_find_nearest_line return TRUE.
> (_bfd_dwarf2_find_line): Use section's vma, instead of lvm.

ITYM lma instead of lvm.

Andreas.

--
Andreas Schwab, SuSE Labs, [hidden email]
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."
Reply | Threaded
Open this post in threaded view
|

Re: PATCH: binutils/2096: addr2line fails on linux-kernel

Nick Clifton
In reply to this post by H.J. Lu-27
Hi H. J.

> 2006-01-16  H.J. Lu  <[hidden email]>
>
> PR binutils/2096
> * dwarf2.c (comp_unit_contains_address): Update comment.
> (_bfd_dwarf2_find_nearest_line): Use section's vma, instead of
> lvm.  Return TRUE only if both comp_unit_contains_address and
> comp_unit_find_nearest_line return TRUE.
> (_bfd_dwarf2_find_line): Use section's vma, instead of lvm.

Approved (with Andreas's suggested fix to the ChangeLog) - please apply.

Cheers
   Nick