Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Ulrich Weigand" <uweigand@de.ibm.com>
To: rth@redhat.com (Richard Henderson)
Cc: palves@redhat.com (Pedro Alves), gdb-patches@gcc.gnu.org
Subject: Re: [PATCH 0/3] Support for x86 segments as address classes
Date: Wed, 18 Nov 2015 18:18:00 -0000	[thread overview]
Message-ID: <20151118181830.6FC3E8509@oc7340732750.ibm.com> (raw)
In-Reply-To: <563B646D.7000805@redhat.com> from "Richard Henderson" at Nov 05, 2015 03:15:09 PM

Pedro Alves wrote:

> get_current_frame / get_current_regcache ?  If you can pass it down
> through a couple layers, it's of course better.
> 
> Though I'm not sufficiently familiar with the address class
> mechanisms be to sure I'm not pointing you in the
> wrong direction.  Ulrich, can you help?

Sorry for the late reply, I must haved missed this earlier ...

I'm a bit sceptical about using get_current_regcache in the
gdbarch_address_to_pointer routines.  Those routines are called
from the generic unpack_long / pack_long routines, which in
turn are called from all over the source base.

It is at least not immediately obvious to me that all those
call sites will have an appropriate "current" regcache set.
(It is probably the case most of the time, though ...)

I had a similar issue on the SPU, where I'm using those
routines to encode the SPU ID into the GDB CORE_ADDR.
For a while, I was attempting to get the SPU ID to use
from the "current" regcache as well, but then decided on
another approach: create a separate gdbarch structure for
each SPU ID in use, and take the SPU ID from gdbarch->tdep
in gdbarch_address_to_pointer.

This worked because I was able to already identify the
appropriate SPU ID whenever creating an gdbarch (from
the objfile, or for the current thread).  This method
likely will not work for the x86 case.


Richard Henderson wrote:

> Breakpoint 2, test () at z.c:10
> 10	  return *f + *g;
> (gdb) ptype f
> type = int * @__seg_fs
> (gdb) ptype g
> type = int * @__seg_gs
> (gdb) info loc
> f = 0x7ffff7fd9710
> g = 0x10
> 
> It seems to me that the "real" value of both f and g are 0x10.  Instead, we 
> display the converted value, displaying f with a surprising value.

Yes, that's another problem.  GDB currently always does a pointer-to-address
conversion, both if it then wants to use the address for accessing memory,
and when it simply wants to *print* the pointer value.  In the latter case,
the address is printed via paddress.

Again, I worked around this issue on the SPU using a hack: paddress always
truncates the output to the size of an address on the platform, which I
defined a 32 on the SPU.  The extra bits to encode the SPU ID within a
GDB address are placed in the high 32 bits, which means a sequence of
pointer-to-address conversion followed by paddress actually prints the
original *pointer* value.

To clean this up, it seems GDB code should be clearer in when to use
*GDB address* values (e.g. to access memory), and when to use *target
pointer* values (e.g. for printing).  This probably means auditing a
lot of code ...
 
> It seems to me that it would be better to do the conversion when we want to 
> dereference, such as "print *f", but not otherwise.  However, I'm not really 
> sure how that would interact with the other users of address classes throughout 
> gdb.

That might a workaround for the x86 case.  I'd still encode address classes
into the types, but leave pointer-to-address / address-to-pointer conversion
as no-ops, and instead modify e.g. value_ind (pointer indirection).  There
is even already a callback to support type-specific value_ind implementations.

However, I'm not completely sure this will cover all relevant cases where
you really want an address.   What about "x/x f", for example?  Perhaps
the correct place to hook is value_as_address?  This already handles the
gdbarch_integer_to_address callback today ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com


  reply	other threads:[~2015-11-18 18:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-03 13:43 Richard Henderson
2015-11-03 13:43 ` [PATCH 2/3] Use register cache for x86_64 ps_get_thread_area Richard Henderson
2015-11-03 13:44 ` [PATCH 3/3] Segment support for x86_64, part 1 Richard Henderson
2015-11-03 13:44 ` [PATCH 1/3] Add amd64 registers fs_base and gs_base Richard Henderson
2015-11-05 11:21 ` [PATCH 0/3] Support for x86 segments as address classes Pedro Alves
2015-11-05 14:15   ` Richard Henderson
2015-11-18 18:18     ` Ulrich Weigand [this message]
2015-11-05 14:18 ` [PATCH 4/3] Segment support for x86_64, part 2 Richard Henderson
2015-11-27 14:34 ` [PATCH 0/3] Support for x86 segments as address classes Gary Benson
2016-04-19 14:23 ` Tedeschi, Walfred
2016-04-19 14:44   ` Richard Henderson
2016-04-19 14:47     ` Walfred Tedeschi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151118181830.6FC3E8509@oc7340732750.ibm.com \
    --to=uweigand@de.ibm.com \
    --cc=gdb-patches@gcc.gnu.org \
    --cc=palves@redhat.com \
    --cc=rth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox