RE: [RFA] i386/amd64 h/w watchpoints in gdbserver

["Pierre Muller" <muller at ics dot u-strasbg dot fr>]

> -----Message d'origine-----
> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Doug Evans
> Envoyé?: Tuesday, June 02, 2009 5:36 PM
> À?: Pierre Muller; Pedro Alves
> Cc?: gdb-patches@sourceware.org
> Objet?: Re: [RFA] i386/amd64 h/w watchpoints in gdbserver
> 
> On Tue, May 19, 2009 at 2:05 AM, Pierre Muller <muller@ics.u-
> strasbg.fr> wrote:
> > ?Doug, I think that your should send a new RFA for an up-to-date
> patch
> > with the bi-arch stuff.
> > Do we have an automated testsuite result comparison script that can
> be
> > used for gdbserver testing?
> 
> I just use --target_board=native-gdbserver.exp and compare
> before/after.
> ref: http://sourceware.org/gdb/wiki/Native_gdbserver_testing
> 
> No regressions on amd64 gdbserver, 32-bit and 64-bit.

  Did you try to remove the line saying that 
gdbserver does not support hardware watchpoints,
does it change anything?
 
> > ?I can test compilation on cygwin,
> > but testing can only be done manually...
> 
> Thanks, can you give this a try?

  I was able to compile without problems
and test gdb.base/watchpoint.exp

and got 60 PASS (same as gdb itself).


This is the board/hext-gdbserver.exp file I am using
Both noargs and nofileio board infos are commented out,
as gdb.base/args.exp and gdb.base/fileio.exp
give the same results for gdbserver as for normal gdb
(after the commenting out in this configuration file).

<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
# gdbserver running native.

load_generic_config "gdbserver"
process_multilib_options ""

# The default compiler for this target.
set_board_info compiler "[find_gcc]"

# This gdbserver can only run a process once per session.
# set_board_info gdb,do_reload_on_run 1

# There's no support for argument-passing (yet).
# set_board_info noargs 1

# Can't do input (or output) in the current gdbserver.
set_board_info gdb,noinferiorio 1

# gdbserver does not intercept target file operations and perform them
# on the host.
# set_board_info gdb,nofileio 1

# Can't do hardware watchpoints, in general.
# set_board_info gdb,no_hardware_watchpoints 1

set_board_info sockethost "localhost:"
#set_board_info use_gdb_stub 1
#set_board_info gdb_stub libgdbstub

# We will be using the standard GDB remote protocol.
set_board_info gdb_protocol "extended-remote"
# Test the copy of gdbserver in the build directory.
set_board_info gdb_server_prog "../gdbserver/gdbserver"

proc ${board}_spawn { board cmd } {
  global board_info

  set baseboard [lindex [split $board "/"] 0]

  set board_info($baseboard,isremote) 0
  set result [remote_spawn $board $cmd]
  set board_info($baseboard,isremote) 1

  return $result
}

proc ${board}_download { board host dest } {
  return $host
}

proc ${board}_file { dest op args } {
    if { $op == "delete" } {
        return 0
    }
    return [eval [list standard_file $dest $op] $args]
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

> > ?Hopefully, Pedro will then be able to review it rapidly.
> 
> There's one aspect of this patch that one might want changing.
> [I'd like to see the change, fwiw.]
> 
> It's ok for higher level routines to refer to global state (within
> reason), but it's less ok for lower level routines to refer to global
> state (modulo constant tables and such that won't change for the
> duration).  The higher level routines should pass down context so the
> lower level routines don't refer to any global state (again, within
> reason).
> 
> Applying that principle here means routines in linux-x86-low.c, for
> example, should not refer to any global state.
> Shall I add parameters to insert_watchpoint, etc. so that, for example,
> x86_insert_watchpoint doesn't have to call current_process?
> 
> Pedro: Note that this patch does defer updating the debug regs until
> just before PTRACE_CONT.
> [win32 version already did a similar thing]

  While I understand your considerations, I would prefer 
that this patch gets approved as is. We can always try to improve it
later.

Pierre