From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7548 invoked by alias); 8 Nov 2007 20:51:31 -0000 Received: (qmail 7232 invoked by uid 22791); 8 Nov 2007 20:51:27 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate8.de.ibm.com (HELO mtagate8.de.ibm.com) (195.212.29.157) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 08 Nov 2007 20:51:18 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate8.de.ibm.com (8.13.8/8.13.8) with ESMTP id lA8KpF2x136532 for ; Thu, 8 Nov 2007 20:51:15 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.6) with ESMTP id lA8KpFEI2351306 for ; Thu, 8 Nov 2007 21:51:15 +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 lA8KpEfw016252 for ; Thu, 8 Nov 2007 21:51:15 +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 lA8KpEL8016249; Thu, 8 Nov 2007 21:51:14 +0100 Message-Id: <200711082051.lA8KpEL8016249@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 8 Nov 2007 21:51:14 +0100 Subject: Re: [patch] nto target update To: ARistovski@qnx.com (Aleksandar Ristovski) Date: Thu, 08 Nov 2007 20:51:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, RMansfield@qnx.com (Ryan Mansfield), Ulrich.Weigand@de.ibm.com (Ulrich Weigand) In-Reply-To: <3518719F06577C4F85DA618E3C37AB910CE5E2E8@exch.ott.qnx.com> from "Aleksandar Ristovski" at Nov 08, 2007 10:47:52 AM 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: 2007-11/txt/msg00177.txt.bz2 Aleksandar Ristovski wrote: > Nto target has been maintained in our private branch and hasn't been > synchronized with mainstream for a while. > As a result, nto target does not compile. This patch brings it up-to date. Thank you very much for working on getting nto support in GDB mainline up to date. Unfortunately, this patch introduces a number of coding style violations that I'd ask you to fix before this can be applied. Also, the patch introduces a number of functions and macros that are apparently not used anywhere, neither in the patch nor in the existing nto code. Is this something to support some other out-of-mainline code? It would be better if we only add functions as they are actually needed in mainline. > 2007-11-07 Aleksandar Ristovski > > Update for nto target. > > * i386-nto-tdep.c: Update. > * nto-procfs.c: Update. > * nto-tdep.c: Update. > * nto-tdep.h: Update. Please provide a complete ChangeLog entry, listing every change you made to those files. See section 6.8 in http://www.gnu.org/prep/standards/standards.html on how those logs should look like. Some remarks on coding style and other issues: - sp = extract_unsigned_integer (buf, 4); + CORE_ADDR ptrctx; - return sp + I386_NTO_SIGCONTEXT_OFFSET; + /* we store __ucontext_t addr in EDI register */ + ptrctx = frame_unwind_register_unsigned (next_frame, + I386_EDI_REGNUM); + ptrctx += 24 /* context pointer is at this offset */; Comments should be full sentences starting with a capital letter and ending with a full stop (followed by two spaces): /* We store the __ucontext_t address in the EDI register. */ + int sigwaitres; ofunc = (void (*)()) signal (SIGINT, nto_interrupt); - sigwaitinfo (&set, &info); + sigwaitres = sigwaitinfo (&set, &info); + if (sigwaitres == -1) + { + internal_error (__FILE__, __LINE__ - 3, "sigwaitres failed with errno: %d\n", errno); + } Is this really an internal_error, or more something along the lines of perror_with_name? If it is an internal error, I don't think you should use __LINE__ - 3 here ... + pvoid = (void*)&run.fault; Space between "void" and "*". + + psigset = (sigset_t *) pvoid; Why is this double cast necessary in the first place? +#include "gdb_assert.h" +#include "gdbcmd.h" Whenever you add include files to a file, please update the dependencies in the Makefile.in. +#define link_map so_map This doesn't appear to be used anywhere. @@ -154,7 +155,12 @@ nto_find_and_open_solib (char *solib, un if (ret >= 0) *temp_pathname = gdb_realpath (arch_path); else - **temp_pathname = '\0'; + { + if (*temp_pathname) + **temp_pathname = '\0'; + else + *temp_pathname = ""; + } Should this not just always set *temp_pathname to NULL? +#if defined (__MINGW32__) +#define PATH_SEP ";" +#else +#define PATH_SEP ":" +#endif You should use the existing DIRNAME_SEPARATOR. + sprintf (buf, "set solib-search-path %s/%s" PATH_SEP "%s/%s", arch_path, "lib", arch_path, "usr/lib"); Lines should not exceed 80 characters in length. Also, it would be better to avoid using sprintf on fixed-size buffers. Use xstrprintf instead. +struct link_map_offsets * +nto_generic_svr4_fetch_link_map_offsets (void) Unused. +#ifndef offsetof +#define offsetof(TYPE, MEMBER) ((unsigned long) &((TYPE *)0)->MEMBER) +#endif +#define fieldsize(TYPE, MEMBER) (sizeof (((TYPE *)0)->MEMBER)) Unused. + if (NULL == buf) + { + return 0; + } GDB coding style would prefer not using braces for a single statement. + return extract_typed_address (so->lm_info->lm + lmo->l_addr_offset, + builtin_type_void_data_ptr); Indentation looks odd. Please use tabs to correspond to 8 spaces. - sec->addr = nto_truncate_ptr (sec->addr + LM_ADDR (so) - vaddr); - sec->endaddr = nto_truncate_ptr (sec->endaddr + LM_ADDR (so) - vaddr); + sec->addr = nto_truncate_ptr (sec->addr + LM_ADDR_FROM_LINK_MAP (so) - vaddr); + sec->endaddr = nto_truncate_ptr (sec->endaddr + LM_ADDR_FROM_LINK_MAP (so) - vaddr); Lines too long. +char * +nto_target_extra_thread_info (struct thread_info *ti) +{ + if (ti && ti->private && ti->private->name[0]) + return ti->private->name; + return ""; +} + +#ifndef __QNXNTO__ + +#define NTO_SIGHUP 1 /* hangup */ +#define NTO_SIGINT 2 /* interrupt */ +#define NTO_SIGQUIT 3 /* quit */ +#define NTO_SIGILL 4 /* illegal instruction (not reset when caught) */ +#define NTO_SIGTRAP 5 /* trace trap (not reset when caught) */ +#define NTO_SIGIOT 6 /* IOT instruction */ +#define NTO_SIGABRT 6 /* used by abort */ +#define NTO_SIGEMT 7 /* EMT instruction */ +#define NTO_SIGDEADLK 7 /* Mutex deadlock */ +#define NTO_SIGFPE 8 /* floating point exception */ +#define NTO_SIGKILL 9 /* kill (cannot be caught or ignored) */ +#define NTO_SIGBUS 10 /* bus error */ +#define NTO_SIGSEGV 11 /* segmentation violation */ +#define NTO_SIGSYS 12 /* bad argument to system call */ +#define NTO_SIGPIPE 13 /* write on pipe with no reader */ +#define NTO_SIGALRM 14 /* real-time alarm clock */ +#define NTO_SIGTERM 15 /* software termination signal from kill */ +#define NTO_SIGUSR1 16 /* user defined signal 1 */ +#define NTO_SIGUSR2 17 /* user defined signal 2 */ +#define NTO_SIGCHLD 18 /* death of child */ +#define NTO_SIGPWR 19 /* power-fail restart */ +#define NTO_SIGWINCH 20 /* window change */ +#define NTO_SIGURG 21 /* urgent condition on I/O channel */ +#define NTO_SIGPOLL 22 /* System V name for NTO_SIGIO */ +#define NTO_SIGIO NTO_SIGPOLL +#define NTO_SIGSTOP 23 /* sendable stop signal not from tty */ +#define NTO_SIGTSTP 24 /* stop signal from tty */ +#define NTO_SIGCONT 25 /* continue a stopped process */ +#define NTO_SIGTTIN 26 /* attempted background tty read */ +#define NTO_SIGTTOU 27 /* attempted background tty write */ +#define NTO_SIGVTALRM 28 /* virtual timer expired */ +#define NTO_SIGPROF 29 /* profileing timer expired */ +#define NTO_SIGXCPU 30 /* exceded cpu limit */ +#define NTO_SIGXFSZ 31 /* exceded file size limit */ + +static struct + { + int nto_sig; + enum target_signal gdb_sig; + } +sig_map[] = +{ + {1, TARGET_SIGNAL_HUP}, + {2, TARGET_SIGNAL_INT}, + {3, TARGET_SIGNAL_QUIT}, + {4, TARGET_SIGNAL_ILL}, + {5, TARGET_SIGNAL_TRAP}, + {6, TARGET_SIGNAL_ABRT}, + {7, TARGET_SIGNAL_EMT}, + {8, TARGET_SIGNAL_FPE}, + {9, TARGET_SIGNAL_KILL}, + {10, TARGET_SIGNAL_BUS}, + {11, TARGET_SIGNAL_SEGV}, + {12, TARGET_SIGNAL_SYS}, + {13, TARGET_SIGNAL_PIPE}, + {14, TARGET_SIGNAL_ALRM}, + {15, TARGET_SIGNAL_TERM}, + {16, TARGET_SIGNAL_USR1}, + {17, TARGET_SIGNAL_USR2}, + {18, TARGET_SIGNAL_CHLD}, + {19, TARGET_SIGNAL_PWR}, + {20, TARGET_SIGNAL_WINCH}, + {21, TARGET_SIGNAL_URG}, + {22, TARGET_SIGNAL_POLL}, + {23, TARGET_SIGNAL_STOP}, + {24, TARGET_SIGNAL_TSTP}, + {25, TARGET_SIGNAL_CONT}, + {26, TARGET_SIGNAL_TTIN}, + {27, TARGET_SIGNAL_TTOU}, + {28, TARGET_SIGNAL_VTALRM}, + {29, TARGET_SIGNAL_PROF}, + {30, TARGET_SIGNAL_XCPU}, + {31, TARGET_SIGNAL_XFSZ} +}; +#endif // ndef __QNXNTO__ + +/* Convert nto signal to gdb signal. */ +enum target_signal +target_signal_from_nto(int sig) +{ +#ifndef __QNXNTO__ + switch(sig) + { + case 0: return 0; break; + case NTO_SIGHUP: return TARGET_SIGNAL_HUP; break; + case NTO_SIGINT: return TARGET_SIGNAL_INT; break; + case NTO_SIGQUIT: return TARGET_SIGNAL_QUIT; break; + case NTO_SIGILL: return TARGET_SIGNAL_ILL; break; + case NTO_SIGTRAP: return TARGET_SIGNAL_TRAP; break; + case NTO_SIGABRT: return TARGET_SIGNAL_ABRT; break; + case NTO_SIGEMT: return TARGET_SIGNAL_EMT; break; + case NTO_SIGFPE: return TARGET_SIGNAL_FPE; break; + case NTO_SIGKILL: return TARGET_SIGNAL_KILL; break; + case NTO_SIGBUS: return TARGET_SIGNAL_BUS; break; + case NTO_SIGSEGV: return TARGET_SIGNAL_SEGV; break; + case NTO_SIGSYS: return TARGET_SIGNAL_SYS; break; + case NTO_SIGPIPE: return TARGET_SIGNAL_PIPE; break; + case NTO_SIGALRM: return TARGET_SIGNAL_ALRM; break; + case NTO_SIGTERM: return TARGET_SIGNAL_TERM; break; + case NTO_SIGUSR1: return TARGET_SIGNAL_USR1; break; + case NTO_SIGUSR2: return TARGET_SIGNAL_USR2; break; + case NTO_SIGCHLD: return TARGET_SIGNAL_CHLD; break; + case NTO_SIGPWR: return TARGET_SIGNAL_PWR; break; + case NTO_SIGWINCH: return TARGET_SIGNAL_WINCH; break; + case NTO_SIGURG: return TARGET_SIGNAL_URG; break; + case NTO_SIGPOLL: return TARGET_SIGNAL_POLL; break; + case NTO_SIGSTOP: return TARGET_SIGNAL_STOP; break; + case NTO_SIGTSTP: return TARGET_SIGNAL_TSTP; break; + case NTO_SIGCONT: return TARGET_SIGNAL_CONT; break; + case NTO_SIGTTIN: return TARGET_SIGNAL_TTIN; break; + case NTO_SIGTTOU: return TARGET_SIGNAL_TTOU; break; + case NTO_SIGVTALRM: return TARGET_SIGNAL_VTALRM; break; + case NTO_SIGPROF: return TARGET_SIGNAL_PROF; break; + case NTO_SIGXCPU: return TARGET_SIGNAL_XCPU; break; + case NTO_SIGXFSZ: return TARGET_SIGNAL_XFSZ; break; + default: break; + } +#endif /* __QNXNTO__ */ + return target_signal_from_host(sig); +} + + +/* Convert gdb signal to nto signal. */ +int +target_signal_to_nto(enum target_signal sig) +{ +#ifndef __QNXNTO__ + switch(sig) + { + case 0: return 0; break; + case TARGET_SIGNAL_HUP: return NTO_SIGHUP; break; + case TARGET_SIGNAL_INT: return NTO_SIGINT; break; + case TARGET_SIGNAL_QUIT: return NTO_SIGQUIT; break; + case TARGET_SIGNAL_ILL: return NTO_SIGILL; break; + case TARGET_SIGNAL_TRAP: return NTO_SIGTRAP; break; + case TARGET_SIGNAL_ABRT: return NTO_SIGABRT; break; + case TARGET_SIGNAL_EMT: return NTO_SIGEMT; break; + case TARGET_SIGNAL_FPE: return NTO_SIGFPE; break; + case TARGET_SIGNAL_KILL: return NTO_SIGKILL; break; + case TARGET_SIGNAL_BUS: return NTO_SIGBUS; break; + case TARGET_SIGNAL_SEGV: return NTO_SIGSEGV; break; + case TARGET_SIGNAL_SYS: return NTO_SIGSYS; break; + case TARGET_SIGNAL_PIPE: return NTO_SIGPIPE; break; + case TARGET_SIGNAL_ALRM: return NTO_SIGALRM; break; + case TARGET_SIGNAL_TERM: return NTO_SIGTERM; break; + case TARGET_SIGNAL_USR1: return NTO_SIGUSR1; break; + case TARGET_SIGNAL_USR2: return NTO_SIGUSR2; break; + case TARGET_SIGNAL_CHLD: return NTO_SIGCHLD; break; + case TARGET_SIGNAL_PWR: return NTO_SIGPWR; break; + case TARGET_SIGNAL_WINCH: return NTO_SIGWINCH; break; + case TARGET_SIGNAL_URG: return NTO_SIGURG; break; + case TARGET_SIGNAL_IO: return NTO_SIGIO; break; + case TARGET_SIGNAL_POLL: return NTO_SIGPOLL; break; + case TARGET_SIGNAL_STOP: return NTO_SIGSTOP; break; + case TARGET_SIGNAL_TSTP: return NTO_SIGTSTP; break; + case TARGET_SIGNAL_CONT: return NTO_SIGCONT; break; + case TARGET_SIGNAL_TTIN: return NTO_SIGTTIN; break; + case TARGET_SIGNAL_TTOU: return NTO_SIGTTOU; break; + case TARGET_SIGNAL_VTALRM: return NTO_SIGVTALRM; break; + case TARGET_SIGNAL_PROF: return NTO_SIGPROF; break; + case TARGET_SIGNAL_XCPU: return NTO_SIGXCPU; break; + case TARGET_SIGNAL_XFSZ: return NTO_SIGXFSZ; break; + default: break; + } +#endif /* __QNXNTO__ */ + return target_signal_to_host(sig); +} + Unless I'm missing something, all this seems unused. +static void +show_nto_debug (struct ui_file *file, int from_tty, + struct cmd_list_element *c, const char *value) +{ + fprintf_filtered (file, _("QNX NTO debug level is %d.\n"), nto_internal_debugging); Line too long. +} + +static int +nto_print_tidinfo_callback (struct thread_info *tp, void *data) +{ + printf_filtered("%c%d\t%d\t%d\n", ptid_equal (tp->ptid, inferior_ptid) ? '*' : ' ', tp->private->tid, tp->private->state, tp->private->flags ); Line much too long. Also, please use a space before the '(' in a function call. + return 0; +} + +static void +nto_info_tidinfo_command (char *args, int from_tty) +{ + nto_trace (0) ("%s (args=%s, from_tty=%d)\n", __func__, args, from_tty); + + target_find_new_threads (); + printf_filtered("Threads for pid %d (%s)\nTid:\tState:\tFlags:\n", ptid_get_pid (inferior_ptid), get_exec_file (0)); Likewise. +#define nto_trace(level) \ + if ((nto_internal_debugging & 0xFF) <= level) {} else \ + printf_unfiltered I'm not sure this kind of tracing macro belongs in mainline; but I won't object to it either if you feel you need it ... +/* register supply helper macros*/ +#define NTO_ALL_REGS (-1) +#define RAW_SUPPLY_IF_NEEDED(regcache, whichreg, dataptr) \ + {if (!(NTO_ALL_REGS == regno || regno == (whichreg))) {} \ + else regcache_raw_supply (regcache, whichreg, dataptr); } Apparently unused. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com