From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24245 invoked by alias); 24 Feb 2016 20:39:16 -0000 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 Received: (qmail 21371 invoked by uid 89); 24 Feb 2016 20:39:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=corner, recognise, optimisation, speeding X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 24 Feb 2016 20:39:11 +0000 Received: from hhmail02.hh.imgtec.org (unknown [10.100.10.20]) by Websense Email Security Gateway with ESMTPS id 6EFFF6811EEFD; Wed, 24 Feb 2016 20:39:05 +0000 (GMT) Received: from [10.100.200.149] (10.100.200.149) by hhmail02.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server id 14.3.266.1; Wed, 24 Feb 2016 20:39:08 +0000 Date: Wed, 24 Feb 2016 20:39:00 -0000 From: "Maciej W. Rozycki" To: Pedro Alves CC: , "Maciej W. Rozycki" Subject: Re: [PATCH] Handle MIPS Linux SIGTRAP siginfo.si_code values In-Reply-To: <56CDFB9B.3090708@redhat.com> Message-ID: References: <1456332239-24007-1-git-send-email-palves@redhat.com> <56CDFB9B.3090708@redhat.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2016-02/txt/msg00756.txt.bz2 On Wed, 24 Feb 2016, Pedro Alves wrote: > >> + The generic Linux target code should use GDB_ARCH_IS_TRAP_* instead > >> + of TRAP_* to abstract out these peculiarities. */ > >> #if defined __i386__ || defined __x86_64__ > >> # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL) > >> +# define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == TRAP_HWBKPT) > >> #elif defined __powerpc__ > >> # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL || (X) == TRAP_BRKPT) > >> +# define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == TRAP_HWBKPT) > >> +#elif defined __mips__ > >> +# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL) > >> +# define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == SI_KERNEL) > > > > Shall I add the TRAP_BRKPT and TRAP_HWBKPT codes to the MIPS Linux kernel > > then or not? > > The higher order issue was having a way to distinguish the possible > traps, for correctness. Since we found a way, it's no longer a > pressing issue and we could leave it be. > > > If anything, this looks like a performance win to me, at no > > significant cost (any possible kernel overhead will be in the range of a > > couple processor instructions, which is nothing compared to an extra > > ptrace(2) call and all the associated processing). > > Yeah. We usually need several other ptrace calls so it may not > be noticeable. But if you do teach the kernel about TRAP_BRKPT, and want to > avoid the watchpoints check, I think gdb/gdbserver could be made to auto detect > at run time what does the kernel report. E.g,. have gdb fork itself, set a > sw bp at the current PC in the child and continue it. That triggers the bp > immediately. Then the parent can check what is in the child's si_code. We > do similar things already in linux_check_ptrace_features (gdb/nat/linux-ptrace.c). That would be the big-hammer approach. However from my understanding of your code the benefit from such probing would only matter for speeding up random SIGTRAP reporting, a corner case not worth it IMO. Whereas for real breakpoint hits if we report TRAP_BRKPT for software breakpoints and TRAP_HWBKPT for hardware data or instruction breakpoints (the latters once implemented), then the: if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code) && GDB_ARCH_IS_TRAP_HWBKPT (siginfo.si_code)) condition will never be true for legitimate SIGTRAP events and the slow path won't trigger. But then the MIPS variant will need: # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL || (X) == TRAP_BRKPT) # define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == SI_KERNEL || (X) == TRAP_HWBKPT) to actually recognise these events at all in the first place. So we better have it right away or updated kernels will break GDB for a change. But you haven't put it in your proposal despite my earlier suggestion, hence my question whether you want this kernel API correction and the resulting optimisation or not (I do). Given my observations above it looks to me that we only really need the two macros updated with my proposal on the GDB side and a corresponding rather trivial kernel update to have the codes set in the first place, and we can get away without complicating code with an extra run-time probe. Have I missed anything? Maciej