PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[H.J. Lu]

bfd_simple_get_relocated_section_content shouldn't apply relocations
on executable and shared library with relocations. This patch uses
the same check in binutils/objdump.c.


H.J.
---
2007-07-09  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/4756
	* simple.c (bfd_simple_get_relocated_section_content): Don't
	apply relocation on executable and shared library.

--- bfd/simple.c.reloc	2007-07-03 10:51:13.000000000 -0700
+++ bfd/simple.c	2007-07-09 11:28:26.000000000 -0700
@@ -162,7 +162,8 @@ bfd_simple_get_relocated_section_content
   int storage_needed;
   void *saved_offsets;
 
-  if (! (sec->flags & SEC_RELOC))
+  if ((abfd->flags & (HAS_RELOC | EXEC_P | DYNAMIC)) != HAS_RELOC
+      || ! (sec->flags & SEC_RELOC))
     {
       bfd_size_type amt = sec->rawsize > sec->size ? sec->rawsize : sec->size;
       bfd_size_type size = sec->rawsize ? sec->rawsize : sec->size;

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[Daniel Jacobowitz]

On Mon, Jul 09, 2007 at 11:43:40AM -0700, H.J. Lu wrote:
> bfd_simple_get_relocated_section_content shouldn't apply relocations
> on executable and shared library with relocations. This patch uses
> the same check in binutils/objdump.c.

Do you see any way to fix this that does not drop support for shared
libraries where debug info has not been relocated?

I don't think so, and I do not object to your patch.  But this
function was originally added for shared libraries, not for ET_REL
objects.  I'm sure you remember the several different ways binutils
treated relocations in shared library debug info over time.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[H.J. Lu]

On Mon, Jul 09, 2007 at 02:56:04PM -0400, Daniel Jacobowitz wrote:
> On Mon, Jul 09, 2007 at 11:43:40AM -0700, H.J. Lu wrote:
> > bfd_simple_get_relocated_section_content shouldn't apply relocations
> > on executable and shared library with relocations. This patch uses
> > the same check in binutils/objdump.c.
> 
> Do you see any way to fix this that does not drop support for shared
> libraries where debug info has not been relocated?

Can you provide a testcase to show the problem? I can take a look.

Thanks.


H.J.

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[Daniel Jacobowitz]

On Mon, Jul 09, 2007 at 12:52:45PM -0700, H.J. Lu wrote:
> On Mon, Jul 09, 2007 at 02:56:04PM -0400, Daniel Jacobowitz wrote:
> > On Mon, Jul 09, 2007 at 11:43:40AM -0700, H.J. Lu wrote:
> > > bfd_simple_get_relocated_section_content shouldn't apply relocations
> > > on executable and shared library with relocations. This patch uses
> > > the same check in binutils/objdump.c.
> > 
> > Do you see any way to fix this that does not drop support for shared
> > libraries where debug info has not been relocated?
> 
> Can you provide a testcase to show the problem? I can take a look.

It is no longer possible to build one with GNU binutils.  If you have
a four or so year old copy for powerpc-linux, it should be easy; but
the last time I tried I didn't have any sufficiently old tools
available.  Sorry, no testcase.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[Nick Clifton]

Hi H.J.

> 2007-07-09  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	PR binutils/4756
> 	* simple.c (bfd_simple_get_relocated_section_content): Don't
> 	apply relocation on executable and shared library.

Approved - please apply - but please could you add the PR number in a 
comment so that future readers of the code will be able to find out why 
shared libraries and executables are excluded.

Cheers
   Nick

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[Alan Modra]

On Mon, Jul 09, 2007 at 11:43:40AM -0700, H.J. Lu wrote:
> the same check in binutils/objdump.c.

Hmm, which needs correcting like objcopy PR 4716 fix.

	* objdump.c (dump_dwarf): Correct setting of is_relocatable.

Index: binutils/objdump.c
===================================================================
RCS file: /cvs/src/src/binutils/objdump.c,v
retrieving revision 1.131
diff -u -p -r1.131 objdump.c
--- binutils/objdump.c	5 Jul 2007 16:54:45 -0000	1.131
+++ binutils/objdump.c	10 Jul 2007 13:46:28 -0000
@@ -2166,8 +2166,7 @@ check_mach_o_dwarf (bfd *abfd)
 static void
 dump_dwarf (bfd *abfd)
 {
-  is_relocatable = ((abfd->flags & (HAS_RELOC | EXEC_P | DYNAMIC))
-		    == HAS_RELOC);
+  is_relocatable = (abfd->flags & (EXEC_P | DYNAMIC)) == 0;
 
   /* FIXME: bfd_get_arch_size may return -1.  We assume that 64bit
      targets will return 64.  */

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[H.J. Lu]

On Tue, Jul 10, 2007 at 11:21:08PM +0930, Alan Modra wrote:
> On Mon, Jul 09, 2007 at 11:43:40AM -0700, H.J. Lu wrote:
> > the same check in binutils/objdump.c.
> 
> Hmm, which needs correcting like objcopy PR 4716 fix.
> 
> 	* objdump.c (dump_dwarf): Correct setting of is_relocatable.
> 
> Index: binutils/objdump.c
> ===================================================================
> RCS file: /cvs/src/src/binutils/objdump.c,v
> retrieving revision 1.131
> diff -u -p -r1.131 objdump.c
> --- binutils/objdump.c	5 Jul 2007 16:54:45 -0000	1.131
> +++ binutils/objdump.c	10 Jul 2007 13:46:28 -0000
> @@ -2166,8 +2166,7 @@ check_mach_o_dwarf (bfd *abfd)
>  static void
>  dump_dwarf (bfd *abfd)
>  {
> -  is_relocatable = ((abfd->flags & (HAS_RELOC | EXEC_P | DYNAMIC))
> -		    == HAS_RELOC);
> +  is_relocatable = (abfd->flags & (EXEC_P | DYNAMIC)) == 0;
>  
>    /* FIXME: bfd_get_arch_size may return -1.  We assume that 64bit
>       targets will return 64.  */
> 

For bfd_simple_get_relocated_section_content,

if ((abfd->flags & (HAS_RELOC | EXEC_P | DYNAMIC)) != HAS_RELOC
    || ! (sec->flags & SEC_RELOC))

is the same as

if ((abfd->flags & (EXEC_P | DYNAMIC)) != 0
    || ! (sec->flags & SEC_RELOC))

But we don't need to check (sec->flags & SEC_RELOC) if
(abfd->flags & HAS_RELOC) == 0.


H.J.

Re: PATCH: PR binutils/4756: addr2line fails on relocatable linux kernel

[Alan Modra]

On Tue, Jul 10, 2007 at 07:13:34AM -0700, H.J. Lu wrote:
> On Tue, Jul 10, 2007 at 11:21:08PM +0930, Alan Modra wrote:
> > On Mon, Jul 09, 2007 at 11:43:40AM -0700, H.J. Lu wrote:
> > > the same check in binutils/objdump.c.
> > 
> > Hmm, which needs correcting like objcopy PR 4716 fix.
> 
> For bfd_simple_get_relocated_section_content,
[snip]

Yes, I wasn't saying you needed to change your simple.c test.

-- 
Alan Modra
Australia Development Lab, IBM