From patchwork Mon Feb 20 14:13:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 65265 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 9CA4C383E29B for ; Mon, 20 Feb 2023 14:14:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9CA4C383E29B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1676902475; bh=oDFQ8Ob4JVBP3IcfvMzedSzqnHaA+TxMbYKIHbdrBAU=; 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=vtgeCGPkhsAue5wCrw9Yn+8U0N/CDrsZFXfrRQaz3LDqU13YtC5NCN0IdpMD1RkUI WsQbF4FEYQtUirJ6WqOD7lLFPggBIfIZn6kkUjMtsAsdggeiWKtCnryUX3wTx3V/Oi 8jbwYRQpZ9ZMp5xeERsI+ukGKw/UXUtwcFpxakoo= 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.133.124]) by sourceware.org (Postfix) with ESMTPS id EBC02383FDCD for ; Mon, 20 Feb 2023 14:14:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EBC02383FDCD Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-664-oQ3tnubbNKyso4HLzGqvng-1; Mon, 20 Feb 2023 09:13:59 -0500 X-MC-Unique: oQ3tnubbNKyso4HLzGqvng-1 Received: by mail-wm1-f70.google.com with SMTP id l20-20020a05600c1d1400b003e10d3e1c23so555919wms.1 for ; Mon, 20 Feb 2023 06:13:59 -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=oDFQ8Ob4JVBP3IcfvMzedSzqnHaA+TxMbYKIHbdrBAU=; b=yMgBc9Y1t+eq7Oe6fRRBtrtciD3EQ4d7augpetBElMBQhVv+7bLdSHKo0mg4GngLME hK975/nJFllPyzzSxNK3l4XTg+edyJSSe6W0pa7r673X+m7XuzCFGwlBKAfBSgEQ2HQU xxOmADlZJ/7aodxDWfZsNzgk6Mm1BlA+mtO83dnXFDWIdO91FfMkD2wi7O5qFuhE8haR On7igj0ItC747nEcHxwBLOzRR8UooceQuBypMJaiGTy++gxsVuhFWHG0U9ptE3YFDmYB HU2Yrbqp0lIWGd++bqYO+hrr45IwhSSqFNF4NpQHlcMn6GNryYAzWC/PuGnmkECAQZsS GM7g== X-Gm-Message-State: AO0yUKXUZSPYnvoW+uA6c8EynIzUggpp4kykakwx1Jm/IBoOU7B7obwd 6wSwy7hfPUKwxhNBdKAIyne2IX0OB36wwiQ0a0yP5pk0pecEG72v25WdZlbM0Mcd7GVCLdj8h6G rgXH9nbvJRl7D57JzWPk90Kg5XN4L87Z9XFGII7S4Mobh53rN1116c8RwmzUVu0LDiyUrp11fzi WDt98= X-Received: by 2002:a5d:5963:0:b0:2c5:5edb:7068 with SMTP id e35-20020a5d5963000000b002c55edb7068mr2444777wri.60.1676902438264; Mon, 20 Feb 2023 06:13:58 -0800 (PST) X-Google-Smtp-Source: AK7set8BWNrdsNn5rodzzIKR0yLOedGgj9snyz7A4YPXsm6+HZE7qX1ekIPWnI7WS4/7K9kCivy+3w== X-Received: by 2002:a5d:5963:0:b0:2c5:5edb:7068 with SMTP id e35-20020a5d5963000000b002c55edb7068mr2444751wri.60.1676902437831; Mon, 20 Feb 2023 06:13:57 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id j8-20020a5d6048000000b002c553e061fdsm4215160wrt.112.2023.02.20.06.13.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Feb 2023 06:13:57 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/8] gdb: don't duplicate 'thread' field in MI breakpoint output Date: Mon, 20 Feb 2023 14:13:47 +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.6 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" When creating a thread-specific breakpoint with a single location, the 'thread' field would be repeated in the MI output. This can be seen in two existing tests gdb.mi/mi-nsmoribund.exp and gdb.mi/mi-pending.exp, e.g.: (gdb) -break-insert -p 1 bar ^done,bkpt={number="1",type="breakpoint",disp="keep", enabled="y", addr="0x000000000040110a",func="bar", file="/tmp/mi-thread-specific-bp.c", fullname="/tmp/mi-thread-specific-bp.c", line="32",thread-groups=["i1"], thread="1",thread="1", <================ DUPLICATION! times="0",original-location="bar"} I know we need to be careful when adjusting MI output, but I'm hopeful in this case, as the field is duplicated, and the field contents are always identical, that we might get away with removing one of the duplicates. The change in GDB is a fairly trivial condition change. We did have a couple of tests that contained the duplicate fields in their expected output, but given there was no comment pointing out this oddity either in the GDB code, or in the test, I suspect this was more a case of copying whatever output GDB produced and using that as the expected results. I've updated these tests to remove the duplication. I've update lib/mi-support.exp to provide support for building breakpoint patterns that contain the thread field, and I've made use of this in a new test I've added that is just about creating thread-specific breakpoints and checking the results. The two tests I mentioned above as being updated could also use the new lib/mi-support.exp functionality, but I'm going to do that in a later patch, this was it is clear what changes I'm actually proposing to make to the expected output. As I said, I hope that frontends will be able to handle this change, but I still think its worth adding a NEWS entry, that way, if someone runs into problems, there's a chance they can figure out what's going on. This should not impact CLI output at all. Reviewed-By: Eli Zaretskii --- gdb/NEWS | 8 +++ gdb/breakpoint.c | 2 +- gdb/testsuite/gdb.mi/mi-nsmoribund.exp | 2 +- gdb/testsuite/gdb.mi/mi-pending.exp | 2 +- gdb/testsuite/gdb.mi/mi-thread-specific-bp.c | 44 +++++++++++++++++ .../gdb.mi/mi-thread-specific-bp.exp | 49 +++++++++++++++++++ gdb/testsuite/lib/mi-support.exp | 32 ++++++++---- 7 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.c create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp diff --git a/gdb/NEWS b/gdb/NEWS index 75cd11b204e..bea604d7e75 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -83,6 +83,14 @@ maintenance info frame-unwinders ** mi now reports 'no-history' as a stop reason when hitting the end of the reverse execution history. +** When creating a thread-specific breakpoint using the '-p' option, + the -break-insert command would report the 'thread' field twice in + the reply. The content of both fields was always identical. This + has now been fixed; the 'thread' field will be reported just once + for thread-specific breakpoints, or not at all for breakpoints + without a thread restriction. The same is also true for the 'task' + field of an Ada task-specific breakpoint. + *** Changes in GDB 13 * MI version 1 is deprecated, and will be removed in GDB 14. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 902119c7d2c..f3121c8d6eb 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -6480,7 +6480,7 @@ print_one_breakpoint_location (struct breakpoint *b, For the CLI output, the thread/task information is printed on a separate line, see the 'stop only in thread' and 'stop only in task' output below. */ - if (!header_of_multiple && uiout->is_mi_like_p ()) + if (part_of_multiple && uiout->is_mi_like_p ()) { if (b->thread != -1) uiout->field_signed ("thread", b->thread); diff --git a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp index 103aa45d7e1..55450e4621a 100644 --- a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp +++ b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp @@ -74,7 +74,7 @@ mi_delete_breakpoints # Recreate the same breakpoint, but this time, specific to thread 5. mi_gdb_test "234-break-insert -p 5 $srcfile:$bkpt_line" \ - "234\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\".*\",func=\"thread_function\",file=\".*\",fullname=\".*\",line=\".*\",thread-groups=\\\[\".*\"\\\],thread=\"5\",thread=\"5\",times=\"0\",original-location=\".*\"\}" \ + "234\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\".*\",func=\"thread_function\",file=\".*\",fullname=\".*\",line=\".*\",thread-groups=\\\[\".*\"\\\],thread=\"5\",times=\"0\",original-location=\".*\"\}" \ "thread specific breakpoint at thread_function" # Resume all threads. Only thread 5 should report a stop. diff --git a/gdb/testsuite/gdb.mi/mi-pending.exp b/gdb/testsuite/gdb.mi/mi-pending.exp index 153efdf45ce..cd1301c21e3 100644 --- a/gdb/testsuite/gdb.mi/mi-pending.exp +++ b/gdb/testsuite/gdb.mi/mi-pending.exp @@ -98,7 +98,7 @@ mi_gdb_test "-break-delete 3" "\\^done" "delete breakpoint 3" # Set pending breakpoint with a thread via MI. mi_gdb_test "-break-insert -p 2 -f pendfunc3" \ - ".*\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"\",pending=\"pendfunc3\",thread=\"2\",thread=\"2\",times=\"0\",original-location=\"pendfunc3\"\}"\ + ".*\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"\",pending=\"pendfunc3\",thread=\"2\",times=\"0\",original-location=\"pendfunc3\"\}"\ "MI pending breakpoint on pendfunc3" mi_send_resuming_command "exec-continue" "continuing execution to thread condition" diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.c b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.c new file mode 100644 index 00000000000..8c87f01f14b --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.c @@ -0,0 +1,44 @@ +/* This testcase is part of GDB, the GNU debugger. + + 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 . */ + +volatile int global_var = 0; + +__attribute__((__always_inline__)) static inline void +foo (void) +{ + int i; + + for (i = 0; i < 10; ++i) + global_var = i; +} + +__attribute__((__noinline__)) static void +bar (void) +{ + global_var = 0; + foo (); +} + +int +main (void) +{ + global_var = 0; + foo (); + bar (); + foo (); + return 0; +} diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp new file mode 100644 index 00000000000..4586fa44bbc --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp @@ -0,0 +1,49 @@ +# Copyright 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 . + +# This test is for creating thread-specific breakpoint using the MI, +# and checking the results from GDB. + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +if {[mi_clean_restart]} { + return +} + +standard_testfile + +if [build_executable ${testfile}.exp ${binfile} ${srcfile}] { + return -1 +} + +if {[mi_clean_restart $binfile]} { + return -1 +} + +mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \ + -thread "1" + +proc make_loc {num} { + return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"] +} + +set loc1 [make_loc 1] +set loc2 [make_loc 2] +set loc3 [make_loc 3] + +mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \ + -thread "1" \ + -locations "\\\[$loc1,$loc2,$loc3\\\]" diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 767ea72ff70..0963701140e 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -2509,7 +2509,7 @@ proc mi_build_kv_pairs {attr_list {joiner ,}} { proc mi_make_breakpoint_loc {args} { parse_args {{number .*} {enabled .*} {addr .*} {func .*} {file .*} {fullname .*} {line .*} - {thread-groups \\\[.*\\\]}} + {thread-groups \\\[.*\\\]} {thread ""}} set attr_list {} foreach attr [list number enabled addr func file \ @@ -2517,17 +2517,30 @@ proc mi_make_breakpoint_loc {args} { lappend attr_list $attr [set $attr] } - return "{[mi_build_kv_pairs $attr_list]}" + set result [mi_build_kv_pairs $attr_list] + + if {[string length $thread] > 0} { + append result "," + append result [mi_build_kv_pairs [list "thread" $thread]] + } + + return "{$result}" } # Bits shared between mi_make_breakpoint and mi_make_breakpoint_multi. -proc mi_make_breakpoint_1 {attr_list cond evaluated-by times \ +proc mi_make_breakpoint_1 {attr_list thread cond evaluated-by times \ ignore script original-location} { set result "bkpt=\\\{[mi_build_kv_pairs $attr_list]" # There are always exceptions. + # If THREAD is not preset, do not output it. + if {[string length $thread] > 0} { + append result "," + append result [mi_build_kv_pairs [list "thread" $thread]] + } + # If COND is not preset, do not output it. if {[string length $cond] > 0} { append result "," @@ -2592,7 +2605,7 @@ proc mi_make_breakpoint_multi {args} { parse_args {{number .*} {type .*} {disp .*} {enabled .*} {times .*} {ignore 0} {script ""} {original-location .*} {cond ""} {evaluated-by ""} - {locations .*}} + {locations .*} {thread ""}} set attr_list {} foreach attr [list number type disp enabled] { @@ -2602,7 +2615,7 @@ proc mi_make_breakpoint_multi {args} { lappend attr_list "addr" "" set result [mi_make_breakpoint_1 \ - $attr_list $cond ${evaluated-by} $times \ + $attr_list $thread $cond ${evaluated-by} $times \ $ignore $script ${original-location}] append result "," @@ -2626,7 +2639,7 @@ proc mi_make_breakpoint_multi {args} { proc mi_make_breakpoint_pending {args} { parse_args {{number .*} {type .*} {disp .*} {enabled .*} - {pending .*} {original-location .*}} + {pending .*} {original-location .*} {thread ""}} set attr_list {} foreach attr [list number type disp enabled] { @@ -2646,7 +2659,7 @@ proc mi_make_breakpoint_pending {args} { set evaluated-by "" set result [mi_make_breakpoint_1 \ - $attr_list $cond ${evaluated-by} $times \ + $attr_list $thread $cond ${evaluated-by} $times \ $ignore $script ${original-location}] append result "\\\}" @@ -2674,7 +2687,8 @@ proc mi_make_breakpoint {args} { parse_args {{number .*} {type .*} {disp .*} {enabled .*} {addr .*} {func .*} {file .*} {fullname .*} {line .*} {thread-groups \\\[.*\\\]} {times .*} {ignore 0} - {script ""} {original-location .*} {cond ""} {evaluated-by ""}} + {script ""} {original-location .*} {cond ""} {evaluated-by ""} + {thread ""}} set attr_list {} foreach attr [list number type disp enabled addr func file \ @@ -2683,7 +2697,7 @@ proc mi_make_breakpoint {args} { } set result [mi_make_breakpoint_1 \ - $attr_list $cond ${evaluated-by} $times \ + $attr_list $thread $cond ${evaluated-by} $times \ $ignore $script ${original-location}] append result "\\\}"