From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91549 invoked by alias); 8 Aug 2019 16:24:36 -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 91512 invoked by uid 89); 8 Aug 2019 16:24:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.1 spammy=hinder X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 08 Aug 2019 16:24:33 +0000 Received: from pps.filterd (m0098414.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x78GDDFU026988 for ; Thu, 8 Aug 2019 12:24:30 -0400 Received: from e06smtp02.uk.ibm.com (e06smtp02.uk.ibm.com [195.75.94.98]) by mx0b-001b2d01.pphosted.com with ESMTP id 2u8q078kja-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 08 Aug 2019 12:24:29 -0400 Received: from localhost by e06smtp02.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 8 Aug 2019 17:24:27 +0100 Received: from b06avi18878370.portsmouth.uk.ibm.com (9.149.26.194) by e06smtp02.uk.ibm.com (192.168.101.132) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Thu, 8 Aug 2019 17:24:25 +0100 Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06avi18878370.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id x78GOO6T40239532 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 8 Aug 2019 16:24:24 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0F6C652052; Thu, 8 Aug 2019 16:24:24 +0000 (GMT) Received: from oc3748833570.ibm.com (unknown [9.152.214.225]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id ED9AA5204E; Thu, 8 Aug 2019 16:24:23 +0000 (GMT) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id C889CD802EF; Thu, 8 Aug 2019 18:24:23 +0200 (CEST) Subject: Re: [PATCH 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat To: pedromfc@linux.ibm.com (Pedro Franco de Carvalho) Date: Thu, 08 Aug 2019 16:24:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <20190726125012.6503-4-pedromfc@linux.ibm.com> from "Pedro Franco de Carvalho" at Jul 26, 2019 09:50:12 AM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit x-cbid: 19080816-0008-0000-0000-00000306B1C6 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 19080816-0009-0000-0000-00004A24B856 Message-Id: <20190808162423.C889CD802EF@oc3748833570.ibm.com> X-SW-Source: 2019-08/txt/msg00200.txt.bz2 Pedro Franco de Carvalho wrote: > gdb/ChangeLog: > YYYY-MM-DD Pedro Franco de Carvalho > > * ppc-linux-nat.c: Include , , and > . > (PPC_DEBUG_CURRENT_VERSION): Move up define. > (struct arch_lwp_info): New struct. > (class ppc_linux_dreg_interface): New class. > (struct ppc_linux_nat_target) > > > > > > > > : Declare as methods. > : New inner struct. > > : Declare members. > (saved_dabr_value, hwdebug_info, max_slots_number) > (struct hw_break_tuple, struct thread_points, ppc_threads) > (have_ptrace_hwdebug_interface) > (hwdebug_find_thread_points_by_tid) > (hwdebug_insert_point, hwdebug_remove_point): Remove. > (ppc_linux_nat_target::can_use_hw_breakpoint): Use > m_dreg_interface, remove call to PTRACE_SET_DEBUGREG. > (ppc_linux_nat_target::region_ok_for_hw_watchpoint): Add comment, > use m_dreg_interface. > (hwdebug_point_cmp): Change to... > (ppc_linux_nat_target::hwdebug_point_cmp): ...this method. Use > reference arguments instead of pointers. > (ppc_linux_nat_target::ranged_break_num_registers): Use > m_dreg_interface. > (ppc_linux_nat_target::insert_hw_breakpoint): Add comment, use > m_dreg_interface. Call register_hw_breakpoint. > (ppc_linux_nat_target::remove_hw_breakpoint): Add comment, use > m_dreg_interface. Call clear_hw_breakpoint. > (get_trigger_type): Change to... > (ppc_linux_nat_target::get_trigger_type): ...this method. Add > comment. > (ppc_linux_nat_target::insert_mask_watchpoint): Update comment, > use m_dreg_interface. Call register_hw_breakpoint. > (ppc_linux_nat_target::remove_mask_watchpoint): Update comment, > use m_dreg_interface. Call clear_hw_breakpoint. > (can_use_watchpoint_cond_accel): Change to... > (ppc_linux_nat_target::can_use_watchpoint_cond_accel): ...this > method. Update comment, use m_dreg_interface and > m_process_hw_breakpoints. > (calculate_dvc): Change to... > (ppc_linux_nat_target::calculate_dvc): ...this method. Use > m_dreg_interface. > (num_memory_accesses): Change to... > (ppc_linux_nat_target::num_memory_accesses): ...this method. > (check_condition): Change to... > (ppc_linux_nat_target::check_condition): ...this method. > (ppc_linux_nat_target::can_accel_watchpoint_condition): Update > comment, use m_dreg_interface. > (create_watchpoint_request): Change to... > (ppc_linux_nat_target::create_watchpoint_request): ...this > method. Use m_dreg_interface. > (ppc_linux_nat_target::insert_watchpoint): Add comment, use > m_dreg_interface. Call register_hw_breakpoint or register_wp. > (ppc_linux_nat_target::remove_watchpoint): Add comment, use > m_dreg_interface. Call clear_hw_breakpoint or clear_wp. > (ppc_linux_nat_target::low_forget_process) > (ppc_linux_nat_target::low_new_fork) > (ppc_linux_nat_target::low_new_clone) > (ppc_linux_nat_target::low_delete_thread) > (ppc_linux_nat_target::low_prepare_to_resume): New methods. > (ppc_linux_nat_target::low_new_thread): Remove previous logic, > only call mark_thread_stale. > (ppc_linux_nat_target::thread_exit_cb): New method. > (ppc_linux_thread_exit): Remove. > (ppc_linux_nat_target::stopped_data_address): Add comment, use > m_dreg_interface and m_thread_hw_breakpoints. > (ppc_linux_nat_target::stopped_by_watchpoint): Add comment. > (ppc_linux_nat_target::watchpoint_addr_within_range): Use > m_dreg_interface. > (ppc_linux_nat_target::masked_watch_num_registers): Use > m_dreg_interface. > (ppc_linux_nat_target::copy_thread_dreg_state) > (ppc_linux_nat_target::mark_thread_stale) > (ppc_linux_nat_target::mark_debug_registers_changed) > (ppc_linux_nat_target::register_hw_breakpoint) > (ppc_linux_nat_target::clear_hw_breakpoint) > (ppc_linux_nat_target::register_wp) > (ppc_linux_nat_target::clear_wp) > (ppc_linux_nat_target::get_arch_lwp_info): New methods. > (_initialize_ppc_linux_nat): Change the callback function for the > thread_exit observable. This looks generally good to me, just two questions: - As mentioned in the 1/3 patch, why do you need the low_new_clone callback? As I understand it, you'll get low_new_thread called immediatedly afterwards, which will mark the thread as "stale", and once it is scheduled again, all debug regs will be set up from scratch anyway ... - We currently do not support hardware watchpoints in gdbserver, even though we really should. Ideally, the low-level code to handle debug regs should be shared between gdb and gdbserver, as is done e.g. on x86. Now, I'm not saying that handling gdbserver is a pre-req for this patch (fixing GDB first is of course fine!), but I'm wondering if it would make sense, given that you're refactoring a lot of this code anyway, to think about whether this setup would help or hinder a future merge with gdbserver. Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com