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

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Thu, 11 Feb 2010 12:57:30 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> CORE_ADDR can be 64bit while gdb (bfd) can handle 32bit inferiors
> (either when gdb host is e.g. x86_64 or when --enable-64-bit-bfd is
> in use).  Currently gdb (bfd) leaves garbage at the bits 32..63 and
> clear it only at the last moment when required (such as when calling
> ptrace or printing it to the user).

Please define "garbage".  I suspect that what you really mean is that
BFD currently returns sign-extended addresses in some cases.  I'll
assume that's what you really mean in the remainder of this reply.

> There has been a conclusion gdb (bfd) should rather keep the bits cleared (so
> that for example addr1 == addr2 is easily possible):
> 
> http://sourceware.org/ml/gdb-patches/2010-01/msg00497.html
> # Wouldn't it be more natural to force a canonical representation at
> # the time the address is *determined* in the first place?

The question is what this canonical representation should be.  It's
not obvious that the solution you propose (clearing the high bits) is
the right one.  Especiallly when you do arithmetic on addresses, sign
extension may actually be the proper thing to do.

> http://sourceware.org/ml/gdb-patches/2006-09/msg00197.html
> # If we have garbage in the high bits, that's a problem already.

Yup, if the high bits are really garbage.

> Asking if bfd will follow this policy; otherwise the result from
> bfd_elf_bfd_from_remote_memory can be also only masked in gdb/.

Is bfd_elf_bfd_from_remote_memory really the only place in BFD that
returns a sign-extended address?

> I had to fixup gdb/ as currently it worked thanksfully due to bfd/ returning
> the right magic bits 32..63.
> (gdb) p/x loadbase
> $1 = 0xffffffff0033b000
> (There will follow a gdb/-only patch to handle 64->32 more systematically.)

Essentially you'll have to make sure all arithmetic on addresses is
done modulo 2^n with n being the number of bits in the address space.

> --- 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?

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);

Cheers,

Mark