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

Jan Kratochvil
2010-02-11T13:30:46Z

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