From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x642.google.com (mail-ej1-x642.google.com [IPv6:2a00:1450:4864:20::642]) by sourceware.org (Postfix) with ESMTPS id C2D003851C3C for ; Wed, 10 Jun 2020 17:47:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C2D003851C3C Received: by mail-ej1-x642.google.com with SMTP id y13so3585178eju.2 for ; Wed, 10 Jun 2020 10:47:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-description:content-disposition; bh=lFuaPutHRHFWI/oPonMOYiKh0q+JuCsFVz3HxBIOwho=; b=MBzvOaNzOjOnzcCaRKJhV/eQML2t832q7InnLcquW2KTV+oTv5JWVmqYqYbjlzNvGS Y++hRQHjJ3dJU3ExHK6oMJ3u4EkHgWVw009ta58atqqrTu4zDb+/ve+UgHziu7KC5Sdy hKY0TIOS7G62SIBJQIriHoLy8NhoitDh3Lirh809C+bnvoCSAjsrWDN9tDy1r72Yxl74 CsK0wa8k2nvKjbvpShXUWRAikfRCF/7wWdXxw16WQqB/QpbgDQSng7vo+EvAuvMl78JC xk3Ja+tk+6PlEov7EV7vZatsaqt57lv1cm1mz0jdkE93fSU4GcZrCPA5Kx00tTm2JtXh 6gaQ== X-Gm-Message-State: AOAM530Tp9ats8JBytYIS5tG30KJKtIa+SlgX+wsJ1hSpDBEj/3LPl5h K4qg1IHVRX2Q4pOTn6bud7TSN3MHGHtYLw== X-Google-Smtp-Source: ABdhPJyzPQMTpDJ+chUwF0XsJXx1K7lD+yswI9HNKW0luXd5RagTRn/3hq/Bq9wIQ0DEc+IMAb+SRQ== X-Received: by 2002:a17:906:f74a:: with SMTP id jp10mr4538791ejb.43.1591811224464; Wed, 10 Jun 2020 10:47:04 -0700 (PDT) Received: from gmail.com ([2a03:1b20:3:f011::6d]) by smtp.gmail.com with ESMTPSA id n9sm330185ejk.21.2020.06.10.10.47.02 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 10 Jun 2020 10:47:03 -0700 (PDT) Date: Wed, 10 Jun 2020 19:47:02 +0200 From: Shahab Vahedi To: gdb@sourceware.org Cc: Tankut Baris Aktemur , Luis Machado , Anton Kolesov , Shahab Vahedi Subject: Why enforcing sw_breakpoint_from_kind() implementation in GDBserver targets Message-ID: <20200610174702.GA3486@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Description: mail Content-Disposition: inline X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Jun 2020 17:47:07 -0000 Hello, I believe that the "sw_breakpoint_from_kind()" method declared in gdbserver/target.h should not be a pure virtual method. Please read on for further details. Recently, I have rebased our code base of ARC GDB [1] onto the master of binutils-gdb [2]. During this process, I noticed that "linux target ops" have become methods [3]. I noticed that the proto type of "sw_breakpoint_from_kind()" has become [4]: -------------------------[ gdbserver/target.h ]------------------------- ... class process_stratum_target { public: ... virtual const gdb_byte * sw_breakpoint_from_kind (int kind, int *size) = 0; ... }; ... ------------------------------------------------------------------------ This means "sw_breakpoint_from_kind()" is a pure virtual method now. Any class inheriting "process_stratum_target" or its derivatives ("linux_process_target" in particular) MUST implement this method. Else, the compilation will fail. In ARC's case, the GDBserver code did not have this method implemented [5]: ---------------------[ gdbserver/linux-arc-low.cc ]--------------------- ... struct linux_target_ops the_low_target = { arc_arch_setup, arc_regs_info, arc_cannot_fetch_register, arc_cannot_store_register, NULL, /* fetch_register */ linux_get_pc_32bit, linux_set_pc_32bit, NULL, /* breakpoint_kind_from_pc */ NULL, /* sw_breakpoint_from_kind */ NULL, /* get_next_pcs */ 0, /* decr_pc_after_break */ arc_breakpoint_at, }; ... ------------------------------------------------------------------------ Now, it has to provide the implementation for "sw_breakpoint_from_kind". That is not the whole story. I also noticed that this piece of newly implemented code never gets executed. This makes sense because we have a setup that looks like below (both entities are running inside GNU/Linux): ,------------------------------------------------. | ARC GDB client on x86 machine (cross debugger) | `------------------------------------------------' /\ || remote debugging || \/ ,------------------------------------------------. | ARC GDBserver on ARC board (native server) | `------------------------------------------------' It is the "ARC GDB client" that inserts the breakpoint. That has always been the case. Else, it would be impossible to break while debugging with GDBserver in older implementation (before rebase). It is worth mentioning the "ARC GDB client" does have the "sw_breakpoint_from_kind" implemented []. Last but not least, one nitpick: Even though I have added the implementation of "sw_breakpoint_from_kind", I have never done so for "breakpoint_kind_from_pc" or "breakpoint_kind_from_current_state". These last two are supposed to provide the "kind" that will be the input parameter for "sw_breakpoint_from_kind". Therefore, even if the new piece of "sw_breakpoint_from_kind" would be executed, that would be problematic. I'm not sure what can be done about this but I think _if_ "sw_breakpoint_from_kind" should be mandatory, so are "breakpoint_kind_from_pc" and "breakpoint_kind_from_current_state". Cheers, Shahab [1] ARC GDB repository https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb [2] Head of the master at the time of rebase commit: cebd6b8ac1c5a2a847a50e3efe932ff2d0867b3e IFUNC: Update IFUNC resolver check with DT_TEXTREL [3] Commits for converting "linux target ops" to methods 0dd7b52ede3 gdbserver/linux-low: delete 'linux_target_ops' and 'the_low_target' fc5ecdb6304 gdbserver/linux-low: turn 'get_ipa_tdesc_idx' into a method 9eedd27d42c gdbserver/linux-low: turn 'get_syscall_trapinfo' into a method b31cdfa69f4 gdbserver/linux-low: turn 'supports_hardware_single_step' into a method 9cfd8715514 gdbserver/linux-low: turn 'supports_range_stepping' into a method ab64c99982e gdbserver/linux-low: turn 'emit_ops' into a method 809a0c354b9 gdbserver/linux-low: turn fast tracepoint ops into methods 13e567af27e gdbserver/linux-low: turn 'get_thread_area' into a method 47f70aa7685 gdbserver/linux-low: turn 'supports_tracepoints' into a method a5b5da9258d gdbserver/linux-low: turn 'process_qsupported' into a method d7599cc0826 gdbserver/linux-low: turn 'prepare_to_resume' into a method fd000fb3dfd gdbserver/linux-low: turn process/thread addition/deletion ops into methods cb63de7ca80 gdbserver/linux-low: turn 'siginfo_fixup' into a method b35db73327c gdbserver/linux-low: turn '{collect, supply}_ptrace_register' into methods ac1bbaca106 gdbserver/linux-low: turn watchpoint ops into methods 9db9aa232ac gdbserver/linux-low: turn 'insert_point' and 'remove_point' into methods 007c9b975dc gdbserver/linux-low: turn 'supports_z_point_type' into a method d7146cda56c gdbserver/linux-low: turn 'breakpoint_at' into a method d4807ea231e gdbserver/linux-low: turn the 'decr_pc_after_break' field into a method 7582c77c1d2 gdbserver/linux-low: turn 'supports_software_single_step' and 'get_next_pcs' into methods 3ca4edb6617 gdbserver/linux-low: turn 'sw_breakpoint_from_kind' into a method 06250e4e67c gdbserver/linux-low: turn 'breakpoint_kind_from_{pc, current_state}' into methods bf9ae9d8c37 gdbserver/linux-low: turn 'get_pc' and 'set_pc' into methods df95181f00d gdbserver/linux-low: turn some more static functions into private methods bd70b1f240b gdbserver/linux-low: turn 'fetch_register' into a method daca57a7de5 gdbserver/linux-low: turn 'cannot_{fetch/store}_register' into methods aa8d21c9bb4 gdbserver/linux-low: turn 'regs_info' into a method 797bcff595c gdbserver/linux-low: turn 'arch_setup' into a method ef0478f6112 gdbserver/linux-low: start turning linux target ops into methods [4] gdbserver: turn breakpoint kind-related target ops into methods https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=d367006fb7cf [5] ARC's GDB server code before rebase; not providing sw_breakpoint_from_kind() https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb/blob/arc-2020.03/gdbserver/linux-arc-low.cc#L311 [6] ARC's GDB client having the "sw_breakpoint_from_kind" method https://github.com/foss-for-synopsys-dwc-arc-processors/binutils-gdb/blob/arc-2020.03/gdb/arc-linux-tdep.c#L319