This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [PATCH]: memory region attributes


>>>>> "Fernando" == Fernando Nasser <fnasser@redhat.com> writes:
>> >>>>> "Fernando" == Fernando Nasser <fnasser@redhat.com> writes:
Fernando> You forgot to look for xfer_memory in the gdbtk subdirectory...
Fernando> Insight does not currently build.
>> 
>> I'll have to check out insight before I can address this.  My first
>> thought was what the hell is gdbtk doing with an xfer_memory vector
>> or whatever code.  I'll refrain from making judgement until I see
>> the code.
>> 

Fernando> OK, here is why:
Fernando>
Fernando> Insight has its own mechanism to decide from where it is
Fernando> disassembling from: the target memory or the exec file.
Fernando>
Fernando> The xfer_memory() call is used when it wants to make sure
Fernando> that the memory position to be disassembled comes from the
Fernando> exec file (not from the target memory).
Fernando>
Fernando> The MI does the same.

I've checked out insight and I see the call to xfer_memory() in
gdbtk-cmds.c.  

I've already mentioned that I find that adding a null attribute
pointer argument distasteful.  But perhaps doing that as a short
term temporary workaround to keep insight building is necessary.

First I'd like to mention, and then set aside, the question that
if disassembling from the exec file is useful, shouldn't it also
be available from gdb proper.

But I think the real problem is that xfer_memory(), a target vector
function, is being called directly instead of through target_xfer_memory().

At present, target_xfer_memory() breaks up memory into region sized
chunks, does any target independent processing of memory attributes
and then either calls do_xfer_memory() to perform the transfer or
dcache_xfer_memory() to do the transfer through the cache (depending
on whether the cache attribute is set).  When target_xfer_memory() is
bypassed when xfer_memory() is called directly, we also are bypassing
any machine independent attributes and the dcache.

The dcache is a complicating factor as well.  It may contain data for
the memory region being disassembled, but that was taken from the top-
most target that returned the data, usually something like remote or a
ROM monitor vector.  It may be confusing if some of the data for
disassembly comes from the cache (and thus the target) and some comes
from the exec file.

I can think of a few ways to attack this:

* Just pass 0 as the attrib pointer parameter.

  Pro: Very easy
  Con: Will crash if anyone adds an attribute that xfer_memory ever 
       needs to access.

* Always use the current target for disassembly.

  Pro: Very easy.  Always disassembles what's really on the target.
  Con: May be slow on systems with slow target communications.  The
       dcache may speed this up to an acceptable level though.

* Expose the "default attribute" that is currently local to memattr.c
  and change gdbtk_dis_asm_read_memory() to pass it to xfer_memory().

  Pro: Easy.
       Eliminates confusion caused by cached data and exec file.
  Con: Exposes memattr internals.
       Bypasses memory attributes.

* Change target_xfer_memory() (and all lower functions) to take an
  argument that indicates the target vector (or perhaps the target
  stack) to use.

  Pro: Might be a step that is necessary anyway if we have a single 
       GDB controlling many targets.  I believe Andrew mentioned that
       we might be adding a "target" argument to all vector functions.
       It's not far from that that the high level target_* funcs will
       have to have an argument indicating what target to use.
  Con: More difficult.  A lot of work for something that's only used
       by the GUI now.

Thoughts anyone?

        --jtc

-- 
J.T. Conklin
RedBack Networks

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]