From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18759 invoked by alias); 29 Jan 2008 23:34:03 -0000 Received: (qmail 18731 invoked by uid 22791); 29 Jan 2008 23:33:58 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.29.155) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 29 Jan 2008 23:33:37 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.8/8.13.8) with ESMTP id m0TNXYSE474806 for ; Tue, 29 Jan 2008 23:33:34 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m0TNXYkW1810552 for ; Wed, 30 Jan 2008 00:33:34 +0100 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m0TNXX3L020395 for ; Wed, 30 Jan 2008 00:33:34 +0100 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id m0TNXXxP020392; Wed, 30 Jan 2008 00:33:33 +0100 Message-Id: <200801292333.m0TNXXxP020392@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Wed, 30 Jan 2008 00:33:33 +0100 Subject: Re: [rfc][2/3] gdbserver bi-arch support: core s390x part To: drow@false.org (Daniel Jacobowitz) Date: Tue, 29 Jan 2008 23:35:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <20080129202548.GA15063@caradoc.them.org> from "Daniel Jacobowitz" at Jan 29, 2008 03:25:48 PM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00743.txt.bz2 Daniel Jacobowitz wrote: > This isn't quite good enough. It's already possible for the > architecture to change during a single gdbserver run, so I think we > have to support re-calling arch_setup. Right now you have to run in > extended mode and replace a 32-bit binary with a 64-bit one before > restarting it, but I'll be checking in the support for "set remote > exec-file" shortly if I catch up on patch review :-) > > All it takes is hopefully a global flag that we can reset when > switching to a new inferior. I see. Something along the lines of the new version below? > > - unsigned long pc; > > - collect_register_by_name ("pswa", &pc); > > + if (register_size (0) == 4) > > + { > > + unsigned int pc; > > + collect_register_by_name ("pswa", &pc); > > #ifndef __s390x__ > > - pc &= 0x7fffffff; > > + pc &= 0x7fffffff; > > #endif > > - return pc; > > + return pc; > > + } > > + else > > + { > > + unsigned long pc; > > + collect_register_by_name ("pswa", &pc); > > + return pc; > > + } > > } > > This is harmlessly if dead if gdbserver is 32-bit, right? Yes. Note that there are two different issues: whether we need to clear the high bit depends on the architecture of gdbserver, but the size of the register depends on the architecture of the inferior. To simplify the latter issue, maybe it would be nice if generic code had a "collect_register_as_addr" helper that would check the register's size and convert its contents to CORE_ADDR as appropriate? > > +static void > > +s390_arch_setup (void) > > +{ > > + /* Assume 31-bit inferior process. */ > > + init_registers_s390 (); > > + > > + /* On a 64-bit host, check the low bit of the (31-bit) PSWM > > + -- if this is one, we actually have a 64-bit inferior. */ > > +#ifdef __s390x__ > > + { > > + unsigned int pswm; > > + collect_register_by_name ("pswm", &pswm); > > + if (pswm & 1) > > + init_registers_s390x (); > > + } > > +#endif > > This makes my head hurt quite a lot. You're fetching registers into > the cache before you know their size for sure? Well, it is not so much about "knowing" their size -- on a 64-bit gdbserver, ptrace will always give me 64-bit register values. I can simply choose whether to view those as if they were 32-bit by installing the s390 register map -- which I do. > I think using ptrace directly in this case might be more obvious. I tried this, but couldn't quite figure out which PID to use ... Also, getting all the header files and defines in place in the low target file (which doesn't use ptrace at all right now) seemed a bit tedious -- so I went with the above solution instead. Bye, Ulrich ChangeLog: * configure.srv [s390x-*-linux*]: Set srv_regobj to include both reg-s390.o and reg-s390x.o. * linux-low.c (new_inferior): New global variable. (linux_create_inferior, linux_attach): Set it. (linux_wait_for_process): Call the_low_target.arch_setup after the target has stopped for the first time. (initialize_low): Do not call the_low_target.arch_setup. * linux-s390-low.c (s390_get_pc): Support bi-arch operation. (s390_set_pc): Likewise. (s390_arch_setup): New function. (the_low_target): Use s390_arch_setup as arch_setup routine. * regcache.c (realloc_register_cache): New function. (set_register_cache): Call it for each existing regcache. diff -urNp gdb-orig/gdb/gdbserver/configure.srv gdb-head/gdb/gdbserver/configure.srv --- gdb-orig/gdb/gdbserver/configure.srv 2008-01-29 23:10:39.000000000 +0100 +++ gdb-head/gdb/gdbserver/configure.srv 2008-01-29 23:21:53.000000000 +0100 @@ -138,7 +138,7 @@ case "${target}" in srv_linux_regsets=yes srv_linux_thread_db=yes ;; - s390x-*-linux*) srv_regobj=reg-s390x.o + s390x-*-linux*) srv_regobj="reg-s390.o reg-s390x.o" srv_tgtobj="linux-low.o linux-s390-low.o" srv_linux_usrregs=yes srv_linux_regsets=yes diff -urNp gdb-orig/gdb/gdbserver/linux-low.c gdb-head/gdb/gdbserver/linux-low.c --- gdb-orig/gdb/gdbserver/linux-low.c 2008-01-29 23:21:45.000000000 +0100 +++ gdb-head/gdb/gdbserver/linux-low.c 2008-01-29 23:24:41.000000000 +0100 @@ -107,6 +107,11 @@ static int thread_db_active; static int must_set_ptrace_flags; +/* This flag is true iff we've just created or attached to a new inferior + but it has not stopped yet. As soon as it does, we need to call the + low target's arch_setup callback. */ +static int new_inferior; + static void linux_resume_one_process (struct inferior_list_entry *entry, int step, int signal, siginfo_t *info); static void linux_resume (struct thread_resume *resume_info); @@ -291,6 +296,7 @@ linux_create_inferior (char *program, ch new_process = add_process (pid); add_thread (pid, new_process, pid); must_set_ptrace_flags = 1; + new_inferior = 1; return pid; } @@ -346,6 +352,8 @@ linux_attach (unsigned long pid) process = (struct process_info *) find_inferior_id (&all_processes, pid); process->stop_expected = 0; + new_inferior = 1; + return 0; } @@ -606,6 +614,16 @@ retry: (*childp)->last_status = *wstatp; + /* Architecture-specific setup after inferior is running. + This needs to happen after we have attached to the inferior + and it is stopped for the first time, but before we access + any inferior registers. */ + if (new_inferior) + { + the_low_target.arch_setup (); + new_inferior = 1; + } + if (debug_threads && WIFSTOPPED (*wstatp)) { @@ -2060,7 +2078,6 @@ initialize_low (void) set_target_ops (&linux_target_ops); set_breakpoint_data (the_low_target.breakpoint, the_low_target.breakpoint_len); - the_low_target.arch_setup (); linux_init_signals (); linux_test_for_tracefork (); } diff -urNp gdb-orig/gdb/gdbserver/linux-s390-low.c gdb-head/gdb/gdbserver/linux-s390-low.c --- gdb-orig/gdb/gdbserver/linux-s390-low.c 2008-01-29 23:21:45.000000000 +0100 +++ gdb-head/gdb/gdbserver/linux-s390-low.c 2008-01-29 23:21:53.000000000 +0100 @@ -102,24 +102,61 @@ static const unsigned char s390_breakpoi static CORE_ADDR s390_get_pc () { - unsigned long pc; - collect_register_by_name ("pswa", &pc); + if (register_size (0) == 4) + { + unsigned int pc; + collect_register_by_name ("pswa", &pc); #ifndef __s390x__ - pc &= 0x7fffffff; + pc &= 0x7fffffff; #endif - return pc; + return pc; + } + else + { + unsigned long pc; + collect_register_by_name ("pswa", &pc); + return pc; + } } static void s390_set_pc (CORE_ADDR newpc) { - unsigned long pc = newpc; + if (register_size (0) == 4) + { + unsigned int pc = newpc; #ifndef __s390x__ - pc |= 0x80000000; + pc |= 0x80000000; #endif - supply_register_by_name ("pswa", &pc); + supply_register_by_name ("pswa", &pc); + } + else + { + unsigned long pc = newpc; + supply_register_by_name ("pswa", &pc); + } } + +static void +s390_arch_setup (void) +{ + /* Assume 31-bit inferior process. */ + init_registers_s390 (); + + /* On a 64-bit host, check the low bit of the (31-bit) PSWM + -- if this is one, we actually have a 64-bit inferior. */ +#ifdef __s390x__ + { + unsigned int pswm; + collect_register_by_name ("pswm", &pswm); + if (pswm & 1) + init_registers_s390x (); + } +#endif +} + + static int s390_breakpoint_at (CORE_ADDR pc) { @@ -130,11 +167,7 @@ s390_breakpoint_at (CORE_ADDR pc) struct linux_target_ops the_low_target = { -#ifndef __s390x__ - init_registers_s390, -#else - init_registers_s390x, -#endif + s390_arch_setup, s390_num_regs, s390_regmap, s390_cannot_fetch_register, diff -urNp gdb-orig/gdb/gdbserver/regcache.c gdb-head/gdb/gdbserver/regcache.c --- gdb-orig/gdb/gdbserver/regcache.c 2008-01-29 23:10:39.000000000 +0100 +++ gdb-head/gdb/gdbserver/regcache.c 2008-01-29 23:21:53.000000000 +0100 @@ -121,6 +121,15 @@ free_register_cache (void *regcache_p) free (regcache); } +static void +realloc_register_cache (struct inferior_list_entry *thread_p) +{ + struct thread_info *thread = (struct thread_info *) thread_p; + + free_register_cache (inferior_regcache_data (thread)); + set_inferior_regcache_data (thread, new_register_cache ()); +} + void set_register_cache (struct reg *regs, int n) { @@ -137,6 +146,9 @@ set_register_cache (struct reg *regs, in } register_bytes = offset / 8; + + /* Re-allocate all pre-existing register caches. */ + for_each_inferior (&all_threads, realloc_register_cache); } void -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com