From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 828 invoked by alias); 13 Jun 2013 17:37:38 -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 819 invoked by uid 89); 13 Jun 2013 17:37:38 -0000 X-Spam-SWARE-Status: No, score=-4.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 13 Jun 2013 17:37:37 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1UnBSq-0006vn-Cm from Maciej_Rozycki@mentor.com for gdb-patches@sourceware.org; Thu, 13 Jun 2013 10:37:36 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Thu, 13 Jun 2013 10:37:36 -0700 Received: from [172.30.64.211] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.2.247.3; Thu, 13 Jun 2013 18:37:34 +0100 Date: Thu, 13 Jun 2013 17:49:00 -0000 From: "Maciej W. Rozycki" To: Yao Qi CC: Subject: Re: [PATCH 1/3] Include asm/ptrace.h in mips-linux-nat.c In-Reply-To: <1369881867-11372-2-git-send-email-yao@codesourcery.com> Message-ID: References: <1369881867-11372-1-git-send-email-yao@codesourcery.com> <1369881867-11372-2-git-send-email-yao@codesourcery.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2013-06/txt/msg00318.txt.bz2 On Thu, 30 May 2013, Yao Qi wrote: > This patch is to include asm/ptrace.h in mips-linux-nat.c and remove > some duplicated structs already defined in asm/ptrace.h. You missed the local definitions of the PTRACE_GET_WATCH_REGS and PTRACE_SET_WATCH_REGS macros that could also be removed, however... > The ptrace support for hardware support was added into kernel in Sep. > 2008 and the mips hardware watchpoint for native GDB was committed > on April 2009. Considering the time order, GDB doesn't have to carry > these local definitions, and should include asm/ptrace.h instead. ... this change actually resulted in a graceless build failure for me: ../../src/gdb/mips-linux-nat.c:599: error: storage size of 'dummy_regs' isn't known etc. for the very reason the system I used for testing has old kernel headers installed (2.6.19 it would seem, according to ). Such a failure is not acceptable from the user's point of view; I think there are three ways to deal with this: 1. Add an autoconf test that checks for the presence of a key definition; I think 'struct pt_watch_regs' is a good candidate. If that test does not succeed, then the configure process fails gracefully stating the minimum released version of kernel headers required. 2. Add the same test, except in the failure case fall back to the internal definitions we already have, wrapped into #ifndef HAVE_STRUCT_PT_WATCH_REGS. 3. Add the same test and disable hardware watchpoint support in the failure case. The user ABI of the Linux kernel has a stability guarantee and your change makes GDB stop building on this otherwise fine system, so I'd lean towards the second choice, moving all the stuff into common/mips-linux-watch.h as per your second patch in this series. I am not entirely sure what the general practice about such changes in GDB has been though, so I'd ask other maintainers for opinion here. Note that likewise the local PTRACE_GET_THREAD_AREA definition could be removed if relying on for watchpoint support as the macro has already been defined in the 2.6.19 version I have on this system that does not have watchpoint definitions yet. For this reason I think it will be better actually if common/mips-linux-watch.h relies on having been included earlier on by the source file including the former header. For the record the system I intended to run native testing on does indeed support hardware watchpoints, although a minimal number of them: hardware watchpoint : yes, count: 2, address/irw mask: [0x0ffc, 0x0ffb] -- i.e. a single data watchpoint and a single execution watchpoint only (as noted by the "irw mask" field, taking the 3 LSBs of the values reported). I expect to have results by the end of tomorrow and will let you know how that has gone. Would general maintainers please comment on the three-way choice and the preferred way to move forward I outlined above? Maciej