From patchwork Wed Jul 12 15:47:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Farre X-Patchwork-Id: 72562 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 715D93857734 for ; Wed, 12 Jul 2023 15:48:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 715D93857734 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1689176900; bh=7UWkMnVbTffOj4o+J8uj1ZgJ3vfbdndGmwzPTjItwDQ=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=Qu3zOSGwuifVysIe9ZO5ND2MCgx4L7uWnQ30QRfdIYvJjlg8d8e4NzBIjAaVV82S0 k+kuX+hsAovZFWv0IlJ0C9N3ir4iJ7tVixfhOPPzQ1VNpLfDS8FRqft60roWXeQKmT KZ4UG3XnFAPA5/iEd1OoZY2k8INEVN5L07P9l2LE= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-lf1-x129.google.com (mail-lf1-x129.google.com [IPv6:2a00:1450:4864:20::129]) by sourceware.org (Postfix) with ESMTPS id CD2C43858D20 for ; Wed, 12 Jul 2023 15:47:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD2C43858D20 Received: by mail-lf1-x129.google.com with SMTP id 2adb3069b0e04-4fb7b2e3dacso11585081e87.0 for ; Wed, 12 Jul 2023 08:47:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689176873; x=1691768873; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=7UWkMnVbTffOj4o+J8uj1ZgJ3vfbdndGmwzPTjItwDQ=; b=d+Xz73s5ltsSttILfqXM1lBHDbO1mS3AwNVpRpXbZa/6dFWQWYko0LhALNjGyGa5WD kHDvI89DbbOAvQsTjti4CGmAlPjJk4t3wQfySYZvyB6mzgonZ231wLNFWaAp5s9qQSIJ D14M1ye2u836GQaqqwd4KvEps4bkUZUT1P59EKqFlRehSuLK8AfM0IkhodXKCJDpTJuS TJndFp7SjvE95nu/o0r/QwRIRu4zC3TWdOcbhONoLAvZENOLdNKnvkgAVd06nTGc47VM E/KxulfQ4sgSOKuSbHtnn5Qm3A24Ys+QxfU4hi4kI8LYEHW2a/nd2MBOrI7k33/efZns 6YQA== X-Gm-Message-State: ABy/qLb5o0HO+gcBWg6PbjT5O11IoN3TrxL6oDG2cTIQ2M558NZdLgPt ImmdMhBtwFHY1pKntpZxGT2qvBj59P4= X-Google-Smtp-Source: APBJJlH/uVE0s7ENDO2QwSoClPNr29OkarMXA7bknsc5b9ug83QCNIleB4q/h26WO212ywU9WqE/Fg== X-Received: by 2002:a05:6512:318c:b0:4f9:6c44:1bf3 with SMTP id i12-20020a056512318c00b004f96c441bf3mr19884162lfe.62.1689176872577; Wed, 12 Jul 2023 08:47:52 -0700 (PDT) Received: from localhost.localdomain ([2001:2044:c7:5500:5637:6b43:7745:198c]) by smtp.gmail.com with ESMTPSA id f22-20020ac25336000000b004fba6f38f87sm760723lfh.24.2023.07.12.08.47.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Jul 2023 08:47:51 -0700 (PDT) To: gdb-patches@sourceware.org Cc: tom@tromey.com, Simon Farre Subject: [PATCH v4] gdb/DAP Fix disassemble bug Date: Wed, 12 Jul 2023 17:47:36 +0200 Message-ID: <20230712154736.128897-1-simon.farre.cx@gmail.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, LIKELY_SPAM_BODY, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Farre via Gdb-patches From: Simon Farre Reply-To: Simon Farre Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" v4. Added tests & doc comments to clarify v3. This adds the ability to "disassemble backwards" by retrieving "known sources of truth" by using the gdb.Block data type, Block.start is always a valid position in executable code, no? If not, then gdb.Block and all code that relates to gdb.Block and it's underlying type, must also be invalid, so I assume here that Block.start is always "good", as it were. It would probably be nice to have a test for this if we don't already, but from my dog fooding experience with Midas+VSCode+GDB-DAP, it *seems* to be correct. But looks can be deceiving, I guess. v1. Fixes disassembleRequest The field instructionOffset can be negative. Previous patch made it so that sometimes the request got calculated to 0 instructions, when it meant to retrieve disasm for -50 to 0 (current position). --- gdb/python/lib/gdb/dap/disassemble.py | 61 +++++++++++++++++++++++++-- gdb/testsuite/gdb.dap/disassemble.c | 52 +++++++++++++++++++++++ gdb/testsuite/gdb.dap/disassemble.exp | 58 +++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 gdb/testsuite/gdb.dap/disassemble.c create mode 100644 gdb/testsuite/gdb.dap/disassemble.exp diff --git a/gdb/python/lib/gdb/dap/disassemble.py b/gdb/python/lib/gdb/dap/disassemble.py index bc091eb2c89..ff8cbc9e57a 100644 --- a/gdb/python/lib/gdb/dap/disassemble.py +++ b/gdb/python/lib/gdb/dap/disassemble.py @@ -18,17 +18,72 @@ import gdb from .server import request, capability from .startup import send_gdb_with_response, in_gdb_thread +# Disassemble "backwards" +def _disasm_backwards( + arch: gdb.Architecture, end_pc: int, offset: int, requested_count: int +): + """END_PC is the last PC which we should disassemble to + OFFSET is the offset (in instructions) backwards we want to get + REQUESTED_COUNT is the number of instructions asked for in the DAP request.""" + ins_at_pc = arch.disassemble(start_pc=end_pc)[0] + offset = abs(offset) + # We take an arbitrary jump backwards + # Guessing that an instruction averages at 8 bytes + start = end_pc - 4 * offset + instructions = [] + while len(instructions) < (offset + 1): + block = gdb.current_progspace().block_for_pc(start) + # Fail fast; if we can't find a block backwards + # fill all with "invalid values" + if block is None: + tmp = [] + # fill remaining ins with "invalid values" as per DAP spec + for i in range(0, offset - len(instructions)): + tmp.append({"addr": 0, "asm": "unknown"}) + instructions = tmp + instructions + # we don't have to break out early, as the while loop + # predicate will now return false, but we do so for clarity + break + else: + ins = arch.disassemble(start_pc=block.start, end_pc=end_pc) + instructions = ins + instructions + start = start - 8 * (offset - len(instructions)) + end_pc = block.start + + # Disassembled instructions count is >= OFFSET+1 + diff = len(instructions) - offset + result = instructions[diff : diff + requested_count] + # DAP seem to not want the actual instruction *at* end_pc + # when disassembling backwards + if result[-1]["addr"] == ins_at_pc["addr"]: + result.pop() + result = [instructions[diff - 1]] + result + return result[:requested_count] + @in_gdb_thread -def _disassemble(pc, skip_insns, count): +def _disassemble(pc, instructionOffset, count): try: arch = gdb.selected_frame().architecture() except gdb.error: # Maybe there was no frame. arch = gdb.selected_inferior().architecture() result = [] - total_count = skip_insns + count - for elt in arch.disassemble(pc, count=total_count)[skip_insns:]: + if instructionOffset < 0: + ins = _disasm_backwards(arch, pc, instructionOffset, count) + instructionOffset = 0 + count = count - len(ins) + result = [ + {"address": hex(elt["addr"]), "instruction": elt["asm"]} for elt in ins + ] + # Examples of different requests, once we get here: + # If the request was for offset=-200, count=50, we have those now, + # i.e, instructions -200 to -150. (count = 0, so below does nothing) + # If the request was for -50, 100, we have 50 more ins to disassemble, + # i.e remaining instructions are 0..50 + for elt in arch.disassemble(pc, count=count + instructionOffset)[ + instructionOffset: + ]: result.append( { "address": hex(elt["addr"]), diff --git a/gdb/testsuite/gdb.dap/disassemble.c b/gdb/testsuite/gdb.dap/disassemble.c new file mode 100644 index 00000000000..5fb11fae41c --- /dev/null +++ b/gdb/testsuite/gdb.dap/disassemble.c @@ -0,0 +1,52 @@ +/* Copyright 2022-2023 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +int global_variable = 23; + +void +function_breakpoint_here () +{ + ++global_variable; + ++global_variable; +} + +void +do_not_stop_here () +{ + /* This exists to test that breakpoints are cleared. */ +} + +void +address_breakpoint_here () +{ +} + +int +line_breakpoint_here () +{ + do_not_stop_here (); /* FIRST */ + function_breakpoint_here (); + address_breakpoint_here (); + return 0; /* BREAK */ +} + + +int +main () +{ + return line_breakpoint_here (); +} diff --git a/gdb/testsuite/gdb.dap/disassemble.exp b/gdb/testsuite/gdb.dap/disassemble.exp new file mode 100644 index 00000000000..3e2b2dc20a2 --- /dev/null +++ b/gdb/testsuite/gdb.dap/disassemble.exp @@ -0,0 +1,58 @@ +# Copyright 2022-2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test the stop-at-main extension. + +require allow_dap_tests + +load_lib dap-support.exp + +standard_testfile disassemble.c + +if {[build_executable ${testfile}.exp $testfile $srcfile] == -1} { + return +} + +if {[dap_launch $testfile {} {} 1] == ""} { + return +} + +dap_check_request_and_response "start inferior" configurationDone +# We didn't explicitly set a breakpoint, so if we hit one, it worked. +dap_wait_for_event_and_check "stopped at function breakpoint" stopped \ + "body reason" breakpoint + +set bt [lindex [dap_check_request_and_response "backtrace" stackTrace \ + {o threadId [i 1]}] \ + 0] +set pc [dict get [lindex [dict get $bt body stackFrames] 0] instructionPointerReference] + +set da [dap_check_request_and_response "disassemble 10, \[-5 .. 5\)" disassemble\ + {o memoryReference [s $pc] instructionOffset [i -5] instructionCount [i 10] }] + +set ins_count [ llength [dict get [lindex $da 0] body instructions] ] +gdb_assert { $ins_count == 10 } "expected 10 instructions" + +set da [dap_check_request_and_response "disassemble 11, \[-10 .. 1\)" disassemble\ + {o memoryReference [s $pc] instructionOffset [i -10] instructionCount [i 11] }] + +set ins_count [ llength [dict get [lindex $da 0] body instructions] ] +gdb_assert { $ins_count == 11 } "expected 11 instructions" + +set last_should_be_pc [dict get [lindex [dict get [lindex $da 0] body instructions] 10] address] +gdb_assert { $last_should_be_pc == $pc } "expected 11th ins to be same as pc" + + +dap_shutdown