[cancelled] Re: [patch] bfd/: bfd_elf_bfd_from_remote_memory 32bit &= 0xffffffff

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

cancelling this patch review request.


On Thu, 11 Feb 2010 13:50:52 +0100, Mark Kettenis wrote:
> Please define "garbage".  I suspect that what you really mean is that
> BFD currently returns sign-extended addresses in some cases.

I meant garbage (bits with arbitrary unknown content).

But as I see now fixing few GDB places to always sign-extend the displacement
CORE_ADDR will permit using the current standard 64bit math operators even for
32bit inferiors.  And I can even drop the whole prepared 200KB GDB patch.


> > --- a/gdb/symfile-mem.c
> > +++ b/gdb/symfile-mem.c
> > @@ -72,6 +73,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
> >    bfd_vma loadbase;
> >    struct section_addr_info *sai;
> >    unsigned int i;
> > +  int addr_bit = gdbarch_addr_bit (target_gdbarch);
> >  
> >    if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
> >      error (_("add-symbol-file-from-memory not supported for this target"));
> > @@ -103,6 +105,9 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
> >      if ((bfd_get_section_flags (nbfd, sec) & (SEC_ALLOC|SEC_LOAD)) != 0)
> >        {
> >  	sai->other[i].addr = bfd_get_section_vma (nbfd, sec) + loadbase;
> > +	if (addr_bit < (sizeof (ULONGEST) * HOST_CHAR_BIT))
> > +	  sai->other[i].addr &= ((ULONGEST) 1 << addr_bit) - 1;
> > +
> >  	sai->other[i].name = (char *) bfd_get_section_name (nbfd, sec);
> >  	sai->other[i].sectindex = sec->index;
> >  	++i;
> 
> I'm somewhat worried about this change.  Does this mean that on x86
> Linux executables get loaded at an address that is high enough that we
> section address basically wrap around?

In fact always: As 32bit vDSO is built for (non-randomized) address 0xffffe000
but it gets placed thanks to the randomization on random VMA space:
00d36000-00d37000 r-xp 00000000 00:00 0     [vdso]


> Also, if we go this route, I bet you'll be adding code like this to a
> lot of functions.  It may be better to introduce a function that
> returns the mask directly, say gdbarch_addr_mask() and use that
> unconditionally, like:
> 
>   	sai->other[i].addr = bfd_get_section_vma (nbfd, sec) + loadbase;
>  +	sai->other[i].addr &= gdbarch_addr_mask(gdbarch);

This patch followed the current GDB way of doing it.  My prepared but
hopefully obsoleted now patch was using specifically:
  sai->other[i].addr = addr_add_offset (gdbarch, sai->other[i].addr, loadbase);


Thanks,
Jan