From patchwork Wed Feb 8 15:16:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 64485 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 3B4013858433 for ; Wed, 8 Feb 2023 15:16:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3B4013858433 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1675869418; bh=tEau6OQCr9tQIfnPBl7DI+eV/Kaqe5AaSjqa8dqncIg=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=kqcekPoqBWtUt+qbZu7cI3zy7TRG6+V+LpDbOAKhvT8onllbPoh4W2Pa4j+9YNKWA I2fNZFMUCLtjzyQv4LRhqoZHfArF6+Gg87tqMnVBBEAAYWpLBANnFAhd8WqUV4HQ73 ZaURPVe4Q+sm7zaPdhYPxZFG76ETHLBvPO9Ch+o4= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 769603858C50 for ; Wed, 8 Feb 2023 15:16:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 769603858C50 Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-207-ITXRuBFqMPKgBm9Izjnn0Q-1; Wed, 08 Feb 2023 10:16:29 -0500 X-MC-Unique: ITXRuBFqMPKgBm9Izjnn0Q-1 Received: by mail-qv1-f71.google.com with SMTP id ly4-20020a0562145c0400b0054d2629a759so9898504qvb.16 for ; Wed, 08 Feb 2023 07:16:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=tEau6OQCr9tQIfnPBl7DI+eV/Kaqe5AaSjqa8dqncIg=; b=oOijIfjBEiIjI5M3a01S7TgMx66bX/k2t8Z5AlvKBR2EOGMvMeYJPiLD+RGTr1ShcY 489KKxZ9C8w29oauhszdyLmiSNVXPjQ038oEGeKmZR/u5rd2G7nM3gxChrPmVK5rFrxW 3uOwFa9jwDWwVDGnpfkRZRgziAN6VFlXrH+s4QeO+x38W4iwI7FfH4BkFajAZzgAC9sy 2wu1DhlgtKM/RxL7vGhtFyKSBzmN4JBnV4QRnxUgRSabJZNcvBFg9KF+RtzwAqXIytpg Cjnu1TNt0mI0AxDuPnMvuuSj3pdrAyHCZl7C8yvsdMs5MayzEI3J4OgzKeUz+GzQzlxy HCUg== X-Gm-Message-State: AO0yUKVvnX39BeIR2StIUaa+Rc6+i6yuN48McqTUPdBC6YgO30fWiJTy JLlDUPHVgNhImwWofDQtQdo8dg0B5mRT0+DBitnwLuEjBqrV1zN/yLhl5opTB3ErsG5gyul8rrd OQqDCPDmPmlLzzpi4tgUaCisF55RrX3OzyakcTwQVxoNU6EWFNdLX/MJNJq85j2Iib6pzEocy7d wtTiU= X-Received: by 2002:ac8:7d83:0:b0:3b9:b731:bf4d with SMTP id c3-20020ac87d83000000b003b9b731bf4dmr14325287qtd.24.1675869387961; Wed, 08 Feb 2023 07:16:27 -0800 (PST) X-Google-Smtp-Source: AK7set8TUc9/ea0RftvdXBwk8U58poOnhAz4DcvhwQfx74qEDT5YJi+HFAeH8yIpzy5k+XP55/XT2w== X-Received: by 2002:ac8:7d83:0:b0:3b9:b731:bf4d with SMTP id c3-20020ac87d83000000b003b9b731bf4dmr14325209qtd.24.1675869387394; Wed, 08 Feb 2023 07:16:27 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id d19-20020ac847d3000000b003b6325dfc4esm11471528qtr.67.2023.02.08.07.16.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Feb 2023 07:16:27 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 1/2] gdb: only allow one of thread or task on breakpoints or watchpoints Date: Wed, 8 Feb 2023 15:16:16 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP 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: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" After this mailing list posting: https://sourceware.org/pipermail/gdb-patches/2023-February/196607.html it seems to me that in practice an Ada task maps 1:1 with a GDB thread, and so it doesn't really make sense to allow uses to give both a thread and a task within a single breakpoint or watchpoint condition. This commit updates GDB so that the user will get an error if both are specified. I've added new tests to cover the CLI as well as the Python and Guile APIs. For the Python and Guile testing, as far as I can tell, this was the first testing for this corner of the APIs, so I ended up adding more than just a single test. For documentation I've added a NEWS entry, but I've not added anything to the docs themselves. Currently we document the commands with a thread-id or task-id as distinct command, e.g.: 'break LOCSPEC task TASKNO' 'break LOCSPEC task TASKNO if ...' 'break LOCSPEC thread THREAD-ID' 'break LOCSPEC thread THREAD-ID if ...' As such, I don't believe there is any indication that combining 'task' and 'thread' would be expected to work; it seems clear to me in the above that those four options are all distinct commands. I think the NEWS entry is enough that if someone is combining these keywords (it's not clear what the expected behaviour would be in this case) then they can figure out that this was a deliberate change in GDB, but for a new user, the manual doesn't suggest combining them is OK, and any future attempt to combine them will give an error. --- gdb/NEWS | 6 +++ gdb/breakpoint.c | 36 ++++++++++++-- gdb/breakpoint.h | 10 ++++ gdb/guile/scm-breakpoint.c | 10 ++++ gdb/python/py-breakpoint.c | 14 ++++++ gdb/testsuite/gdb.ada/tasks.exp | 85 +++++++++++++++++++++++++++++++-- 6 files changed, 153 insertions(+), 8 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 1567cbea9bd..89b9d32817a 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -44,6 +44,12 @@ keyword already gave an error when used multiple times with the watch command, this remains unchanged. +* For both the break and watch commands, it is now invalid to use both + the 'thread' and 'task' keywords within the same command. For + example the following commnds will now give an error: + break foo thread 1 task 1 + watch var thread 2 task 3 + * New commands maintenance print record-instruction [ N ] diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index adf38e7d722..8963b10d516 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1454,12 +1454,16 @@ breakpoint_set_silent (struct breakpoint *b, int silent) gdb::observers::breakpoint_modified.notify (b); } -/* Set the thread for this breakpoint. If THREAD is -1, make the - breakpoint work for any thread. */ +/* See breakpoint.h. */ void breakpoint_set_thread (struct breakpoint *b, int thread) { + /* It is invalid to set the thread field to anything other than -1 (which + means no thread restriction) if a task restriction is already in + place. */ + gdb_assert (thread == -1 || b->task == 0); + int old_thread = b->thread; b->thread = thread; @@ -1467,12 +1471,16 @@ breakpoint_set_thread (struct breakpoint *b, int thread) gdb::observers::breakpoint_modified.notify (b); } -/* Set the task for this breakpoint. If TASK is 0, make the - breakpoint work for any task. */ +/* See breakpoint.h. */ void breakpoint_set_task (struct breakpoint *b, int task) { + /* It is invalid to set the task field to anything other than 0 (which + means no task restriction) if a thread restriction is already in + place. */ + gdb_assert (task == 0 || b->thread == -1); + int old_task = b->task; b->task = task; @@ -8438,6 +8446,8 @@ code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_, gdb_assert (!sals.empty ()); + /* At most one of thread or task can be set on any breakpoint. */ + gdb_assert (thread == -1 || task == 0); thread = thread_; task = task_; @@ -8806,6 +8816,9 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc, if (*thread != -1) error(_("You can specify only one thread.")); + if (*task != 0) + error (_("You can specify only one of thread or task.")); + tok = end_tok + 1; thr = parse_thread_id (tok, &tmptok); if (tok == tmptok) @@ -8820,6 +8833,9 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc, if (*task != 0) error(_("You can specify only one task.")); + if (*thread != -1) + error (_("You can specify only one of thread or task.")); + tok = end_tok + 1; *task = strtol (tok, &tmptok, 0); if (tok == tmptok) @@ -8854,7 +8870,7 @@ find_condition_and_thread_for_sals (const std::vector &sals, for (auto &sal : sals) { gdb::unique_xmalloc_ptr cond; - int thread_id = 0; + int thread_id = -1; int task_id = 0; gdb::unique_xmalloc_ptr remaining; @@ -8869,6 +8885,8 @@ find_condition_and_thread_for_sals (const std::vector &sals, find_condition_and_thread (input, sal.pc, &cond, &thread_id, &task_id, &remaining); *cond_string = std::move (cond); + /* At most one of thread or task can be set. */ + gdb_assert (thread_id == -1 || task_id == 0); *thread = thread_id; *task = task_id; *rest = std::move (remaining); @@ -10089,6 +10107,9 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, if (thread != -1) error(_("You can specify only one thread.")); + if (task != 0) + error (_("You can specify only one of thread or task.")); + /* Extract the thread ID from the next token. */ thr = parse_thread_id (value_start, &endp); @@ -10105,6 +10126,9 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, if (task != 0) error(_("You can specify only one task.")); + if (thread != -1) + error (_("You can specify only one of thread or task.")); + task = strtol (value_start, &tmp, 0); if (tmp == value_start) error (_("Junk after task keyword.")); @@ -10280,6 +10304,8 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, else w.reset (new watchpoint (nullptr, bp_type)); + /* At most one of thread or task can be set on a watchpoint. */ + gdb_assert (thread == -1 || task == 0); w->thread = thread; w->task = task; w->disposition = disp_donttouch; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 352e8468537..5228c38fe02 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1673,8 +1673,18 @@ extern void breakpoint_set_commands (struct breakpoint *b, extern void breakpoint_set_silent (struct breakpoint *b, int silent); +/* Set the thread for this breakpoint. If THREAD is -1, make the + breakpoint work for any thread. Passing a value other than -1 for + THREAD should only be done if b->task is 0; it is not valid to try and + set both a thread and task restriction on a breakpoint. */ + extern void breakpoint_set_thread (struct breakpoint *b, int thread); +/* Set the task for this breakpoint. If TASK is 0, make the breakpoint + work for any task. Passing a value other than 0 for TASK should only be + done if b->thread is -1; it is not valid to try and set both a thread + and task restriction on a breakpoint. */ + extern void breakpoint_set_task (struct breakpoint *b, int task); /* Clear the "inserted" flag in all breakpoints. */ diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c index a7e043d847b..d4f2b7310bd 100644 --- a/gdb/guile/scm-breakpoint.c +++ b/gdb/guile/scm-breakpoint.c @@ -773,6 +773,11 @@ gdbscm_set_breakpoint_thread_x (SCM self, SCM newvalue) gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG2, newvalue, _("invalid thread id")); } + + if (bp_smob->bp->task != 0) + scm_misc_error (FUNC_NAME, + _("cannot set both task and thread attributes"), + SCM_EOL); } else if (gdbscm_is_false (newvalue)) id = -1; @@ -828,6 +833,11 @@ gdbscm_set_breakpoint_task_x (SCM self, SCM newvalue) gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG2, newvalue, _("invalid task id")); } + + if (bp_smob->bp->thread != -1) + scm_misc_error (FUNC_NAME, + _("cannot set both task and thread attributes"), + SCM_EOL); } else if (gdbscm_is_false (newvalue)) id = 0; diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index 1b10ccd5415..52298935242 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -270,6 +270,13 @@ bppy_set_thread (PyObject *self, PyObject *newvalue, void *closure) _("Invalid thread ID.")); return -1; } + + if (self_bp->bp->task != 0) + { + PyErr_SetString (PyExc_RuntimeError, + _("Cannot set both task and thread attributes.")); + return -1; + } } else if (newvalue == Py_None) id = -1; @@ -321,6 +328,13 @@ bppy_set_task (PyObject *self, PyObject *newvalue, void *closure) _("Invalid task ID.")); return -1; } + + if (self_bp->bp->thread != -1) + { + PyErr_SetString (PyExc_RuntimeError, + _("Cannot set both task and thread attributes.")); + return -1; + } } else if (newvalue == Py_None) id = 0; diff --git a/gdb/testsuite/gdb.ada/tasks.exp b/gdb/testsuite/gdb.ada/tasks.exp index 88ef123865b..9d4f396e88e 100644 --- a/gdb/testsuite/gdb.ada/tasks.exp +++ b/gdb/testsuite/gdb.ada/tasks.exp @@ -14,6 +14,8 @@ # along with this program. If not, see . load_lib "ada.exp" +load_lib "gdb-guile.exp" +load_lib "gdb-python.exp" require allow_ada_tests @@ -39,17 +41,35 @@ gdb_test "info tasks" \ "\r\n"] \ "info tasks before inserting breakpoint" +# Confirm that the "info threads" output lines up with the tasks list. +gdb_test "info threads" \ + [multi_line \ + "\\*\\s+1\\s+\[^\r\n\]+\\s\"foo\"\\s\[^\r\n\]+" \ + "\\s+2\\s+\[^\r\n\]+\\s\"task_list\\(1\\)\"\\s\[^\r\n\]+" \ + "\\s+3\\s+\[^\r\n\]+\\s\"task_list\\(2\\)\"\\s\[^\r\n\]+" \ + "\\s+4\\s+\[^\r\n\]+\\s\"task_list\\(3\\)\"\\s\[^\r\n\]+"] + # Check that multiple uses of the 'task' keyword will give an error. gdb_test "break break_me task 1 task 3" "You can specify only one task\\." gdb_test "watch j task 1 task 3" "You can specify only one task\\." +# Check that attempting to combine 'task' and 'thread' gives an error. +gdb_test "break break_me task 1 thread 1" \ + "You can specify only one of thread or task\\." +gdb_test "break break_me thread 1 task 1" \ + "You can specify only one of thread or task\\." +gdb_test "watch j task 1 thread 1" \ + "You can specify only one of thread or task\\." +gdb_test "watch j thread 1 task 1" \ + "You can specify only one of thread or task\\." + # Insert a breakpoint that should stop only if task 1 stops. Since # task 1 never calls break_me, this shouldn't actually ever trigger. # The fact that this breakpoint is created _before_ the next one # matters. GDB used to have a bug where it would report the first # breakpoint in the list that matched the triggered-breakpoint's # address, no matter which task it was specific to. -gdb_test "break break_me task 1" "Breakpoint .* at .*" +gdb_breakpoint "break_me task 1" message gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 1" \ "check info breakpoints for task 1 breakpoint" @@ -57,8 +77,67 @@ gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 1" \ # extract its number. gdb_breakpoint "break_me task 3" message set bp_number [get_integer_valueof "\$bpnum" -1] -if {$bp_number < 0} { - return +gdb_assert { $bp_number > 0 } "check for a valid breakpoint number" + +# Test the Python API for the breakpoint task attribute. +if {[allow_python_tests]} { + gdb_test_no_output "python bp = gdb.breakpoints()\[$bp_number - 1\]" + gdb_test "python print(bp.task)" "3" + gdb_test "python print(bp.thread)" "None" + gdb_test "python bp.thread = 1" \ + [multi_line \ + "RuntimeError: Cannot set both task and thread attributes\\." \ + "Error while executing Python code\\."] \ + "try setting the thread, but expect an error" + gdb_test_no_output "python bp.task = None" + gdb_test_no_output "python bp.thread = 1" + gdb_test "python bp.task = 3" \ + [multi_line \ + "RuntimeError: Cannot set both task and thread attributes\\." \ + "Error while executing Python code\\."] \ + "try setting the task, but expect an error" + + # Reset the breakpoint to the state required for the rest of this + # test. + gdb_test_no_output "python bp.thread = None" + gdb_test_no_output "python bp.task = 3" +} + +# Test the Guile API for the breakpoint task attribute. +if {[allow_guile_tests]} { + gdb_install_guile_utils + gdb_install_guile_module + + gdb_scm_test_silent_cmd "guile (define blist (breakpoints))" \ + "get breakpoint list" + gdb_scm_test_silent_cmd "guile (define bp (list-ref blist (- $bp_number 1)))" \ + "get breakpoint from list" + gdb_test "guile (print (breakpoint-task bp))" "= 3" + gdb_test "guile (print (breakpoint-thread bp))" "= #f" + gdb_test "guile (set-breakpoint-thread! bp 1)" \ + [multi_line \ + "ERROR: In procedure set-breakpoint-thread!:" \ + "In procedure gdbscm_set_breakpoint_thread_x: cannot set both task and thread attributes" \ + "Error while executing Scheme code."] \ + "attempt to set thread, but expect an error" + + gdb_scm_test_silent_cmd "guile (set-breakpoint-task! bp #f)" \ + "clear breakpoint task attribute" + gdb_scm_test_silent_cmd "guile (set-breakpoint-thread! bp 1)" \ + "set breakpoint thread now task is unset" + gdb_test "guile (set-breakpoint-task! bp 1)" \ + [multi_line \ + "ERROR: In procedure set-breakpoint-task!:" \ + "In procedure gdbscm_set_breakpoint_task_x: cannot set both task and thread attributes" \ + "Error while executing Scheme code."] \ + "attempt to set task, but expect an error" + + # Reset the breakpoint to the state required for the rest of this + # test. + gdb_scm_test_silent_cmd "guile (set-breakpoint-thread! bp #f)" \ + "clear breakpoint thread attribute" + gdb_scm_test_silent_cmd "guile (set-breakpoint-task! bp 3)" \ + "restore breakpoint task attribute" } gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 3" \ "check info breakpoints for task 3 breakpoint"