From patchwork Fri Oct 21 08:43:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59259 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 DC1DB385E020 for ; Fri, 21 Oct 2022 08:44:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DC1DB385E020 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341866; bh=5v1Bhb64nucL2yCOm9su8EAO59hy8Nl1DJEh1tqyVnI=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=KdF1E7k5aU3xZcLCxyzVeJ6C3GaRtQEjRxm1tr7kL/BYbDUB2pXf/D/S5S3fch53K ygM1P0LfDNuckQzsTTr5H+qWjuJDpnUjckYKzkfgVXr+ovOSWpSqOHlK32pKa6wDRL beXKH3GMwT59P6FIcGNdfhykeP9zFlOkDYzEr2q4= 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 57434385696E for ; Fri, 21 Oct 2022 08:43:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 57434385696E 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-665-lDsqaw1cNSGpViT13HIE2A-1; Fri, 21 Oct 2022 04:43:53 -0400 X-MC-Unique: lDsqaw1cNSGpViT13HIE2A-1 Received: by mail-wm1-f70.google.com with SMTP id n6-20020a7bc5c6000000b003c6bbe5d5cfso3031539wmk.4 for ; Fri, 21 Oct 2022 01:43:53 -0700 (PDT) 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=5v1Bhb64nucL2yCOm9su8EAO59hy8Nl1DJEh1tqyVnI=; b=2d07d850PFjrMrepw/DCIuInSA25+bxCPApo+zNYot0dScoinTnX7+N35UE5LPHMiN 3Aj2a56z1Od7CmSpk7j9M5+ShFhQBBGKRaIsEMHX2ZV9CBxWdLlEtCbRUllobsooIW8Q lrhcpruasYCrbNRytH53YJwul9hbVfdgVv/7Z+tKDX1gYKuhBhCftU9Enb9pP/tm+xQW xVLUAcMuxXFzFjofYNPKIFmPrZc6uwvjrAq9mQE7A0RbG/gsMSA2grhcRUpLHzkR0wPC 8SLMGk3/jBuQqNYQuoPdbjiBHyLkLRX2tuu4vIQEMiMZQxkySZIyzIJiwjKe7VVaHb0O gVxg== X-Gm-Message-State: ACrzQf3g6Qcu+dqo3m+NKSMnnTsRYCg6jAHfsLbmVcpup2XUtyAg0dFc Pl/Zu2aSyLD2btlDNHAFcv40WMlZv4nNMOQWznvcNJQ4hOCG+Q+IXPaLLpwCqFghDi2RJ7K1xBv tfT+x+4YM5yY3WoceLv2fYhXD9pro4ipJpLLDpisSPoKwOWYJyp58lESiFqg+OTuBd+jPssS1hA == X-Received: by 2002:a05:600c:46ce:b0:3c6:f243:8d74 with SMTP id q14-20020a05600c46ce00b003c6f2438d74mr20834520wmo.166.1666341832410; Fri, 21 Oct 2022 01:43:52 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4Y61WMAxYcGwQxAmUzp1hS1ivUG3ZQdh5mfbMkr4XmN21oaw2SnfT0V+fgX9btOkhz6YDI8Q== X-Received: by 2002:a05:600c:46ce:b0:3c6:f243:8d74 with SMTP id q14-20020a05600c46ce00b003c6f2438d74mr20834498wmo.166.1666341832091; Fri, 21 Oct 2022 01:43:52 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id n4-20020a1ca404000000b003a601a1c2f7sm2035732wme.19.2022.10.21.01.43.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:51 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 01/12] gdb: int to bool conversion for normal_stop Date: Fri, 21 Oct 2022 09:43:37 +0100 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.9 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, 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" Change the return type of normal_stop (infrun.c) from int to bool. Update callers. There should be no user visible changes after this commit. --- gdb/infcmd.c | 2 +- gdb/infrun.c | 6 +++--- gdb/infrun.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index d729732c81c..6af11ac0cb6 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -873,7 +873,7 @@ step_1 (int skip_subroutines, int single_inst, const char *count_string) proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); else { - int proceeded; + bool proceeded; /* Stepped into an inline frame. Pretend that we've stopped. */ diff --git a/gdb/infrun.c b/gdb/infrun.c index cdfe02393f5..39840ef32c8 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -8614,7 +8614,7 @@ stop_context::changed () const /* See infrun.h. */ -int +bool normal_stop (void) { struct target_waitstatus last; @@ -8774,7 +8774,7 @@ normal_stop (void) the observers would print a stop for the wrong thread/inferior. */ if (saved_context.changed ()) - return 1; + return true; /* Notify observers about the stop. This is where the interpreters print the stop event. */ @@ -8796,7 +8796,7 @@ normal_stop (void) breakpoint_auto_delete (inferior_thread ()->control.stop_bpstat); } - return 0; + return false; } int diff --git a/gdb/infrun.h b/gdb/infrun.h index c711b9b21cc..11a18583e68 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -149,7 +149,7 @@ extern process_stratum_target *user_visible_resume_target (ptid_t resume_ptid); appropriate messages, remove breakpoints, give terminal our modes, and run the stop hook. Returns true if the stop hook proceeded the target, false otherwise. */ -extern int normal_stop (void); +extern bool normal_stop (void); /* Return the cached copy of the last target/ptid/waitstatus returned by target_wait(). The data is actually cached by handle_inferior_event(), From patchwork Fri Oct 21 08:43:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59260 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 697623852759 for ; Fri, 21 Oct 2022 08:44:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 697623852759 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341871; bh=BKkXMttAX/BMDxMR5TVs8BDlEgqLcrdv46TSGdn8Yuo=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=eRFi9eJ/0EliR6YZDqxmZfU6m5G2/xZ4QdAYNxEIXfH6HOGlDLYYDp8XubeuaSl8k xmh2fJYWZ/d40qmPDe6Slog5e5nAIYLaQsx2pgibjeJg+m1UkAXVIvktt3ZXvpG2lj tWBJES86bRpmkBBkVvPyGQExokeyDrMl8djWW8VE= 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 5692038560A4 for ; Fri, 21 Oct 2022 08:43:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5692038560A4 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-541-5IOPUF2VMkW3dgRZcy6e7A-1; Fri, 21 Oct 2022 04:43:54 -0400 X-MC-Unique: 5IOPUF2VMkW3dgRZcy6e7A-1 Received: by mail-wm1-f72.google.com with SMTP id v125-20020a1cac83000000b003bd44dc5242so3020237wme.7 for ; Fri, 21 Oct 2022 01:43:54 -0700 (PDT) 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=BKkXMttAX/BMDxMR5TVs8BDlEgqLcrdv46TSGdn8Yuo=; b=JhnwQvarYQYVwetueR/5Dhb7EWxN9O+CRQQBbEePz7MPyq1xiFlf+kopMfFDH3AV5m ypsM8Sica9H2voAShXay5Jp46RfpxixOSEUSyxcHd0crYoaFsdqwXCGX9DJyJWdztCB2 uk+X5MIUbipBS3+/r77e1fv3XmTYBiVwYQh5tYBvo2eyaI4rC2+dkspNarxv5gzxgjGK kwki6UImStSnygplFuPC0zDmUjOosFBsC9X7ePRG9tmP0oKhiaBfNK/Wf2JVVcGsSpJD 7jWtSj2PzCActi/jjBMqzu6+XEySZq/hlFanTkXYk+KW2d83Do9EXjmOKJVk6Dbq7SYm BAKA== X-Gm-Message-State: ACrzQf0Lv5Ls704dbmVYGaubmXBC/BwE8BMo1YWttsYs1Lj5W97k3gpX 45qCa3hiJhNPQAKxnMKiBHAxH4oENWVkRjY4Y0eyxJ/syN0eX70FJO55h+l7vR1IRPyRZP8X78k Jdcgz0rBQ8STvJfT5OaT7/2ywKK3uSx5K/rV2mSwkGH3uudpALtVybsqRcIJxSBR5TXcB2a4pBw == X-Received: by 2002:a1c:e90b:0:b0:3b4:fb6c:7654 with SMTP id q11-20020a1ce90b000000b003b4fb6c7654mr12049026wmc.98.1666341833503; Fri, 21 Oct 2022 01:43:53 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4J0X/d3XGBHPFlTi6n3yMA9TevBeo1cu/dugW2a3OVeGoDfmvClTOiNm0WKWyYly2CfY/g+w== X-Received: by 2002:a1c:e90b:0:b0:3b4:fb6c:7654 with SMTP id q11-20020a1ce90b000000b003b4fb6c7654mr12049018wmc.98.1666341833319; Fri, 21 Oct 2022 01:43:53 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id e38-20020a5d5966000000b0022afce9ea93sm18315876wri.40.2022.10.21.01.43.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:53 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 02/12] gdb/infrun: add debug print in print_signal_received_reason Date: Fri, 21 Oct 2022 09:43:38 +0100 Message-Id: <8b853ddc70117ba5a6a357c2833070a6c3f160d2.1666341010.git.aburgess@redhat.com> 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, 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" It would have helped me to see an infrun debug line being printed from print_signal_received_reason, so I'm adding one. --- gdb/infrun.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gdb/infrun.c b/gdb/infrun.c index 39840ef32c8..c28bf28a4d9 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -8393,6 +8393,8 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal) { struct thread_info *thr = inferior_thread (); + infrun_debug_printf ("signal = %s", gdb_signal_to_string (siggnal)); + annotate_signal (); if (uiout->is_mi_like_p ()) From patchwork Fri Oct 21 08:43:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59261 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 17BCF385381B for ; Fri, 21 Oct 2022 08:44:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 17BCF385381B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341887; bh=NBqlFjvwWduZw0d7ZtqXTOu+T7xvWApowZymkxjp8GQ=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=uZe4gPfrIPx2m+Td99w2Nai6BtmjOgbbb5mB+MP0Bd0AX4qlwO4PiIagOtBpj5ABv l6YkYvLgMOQYypHoBwL5oWxmZdyCMJ4I7wu1Po0GgwuDl8hsAMuUYYWX+HI04DKrC0 SnWnSX++5E4NQP1aUvxAizdjT4dtV/2+wpu5Z8Cg= 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 1AA653856142 for ; Fri, 21 Oct 2022 08:43:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1AA653856142 Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-564-akO5eVcfNLW41qeSfRMUEw-1; Fri, 21 Oct 2022 04:43:56 -0400 X-MC-Unique: akO5eVcfNLW41qeSfRMUEw-1 Received: by mail-wr1-f70.google.com with SMTP id e11-20020adfa74b000000b0022e39e5c151so589913wrd.3 for ; Fri, 21 Oct 2022 01:43:56 -0700 (PDT) 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=NBqlFjvwWduZw0d7ZtqXTOu+T7xvWApowZymkxjp8GQ=; b=i+IGOIOKGPHN8zUamT24Wg2oeCrKQs1etFU2C1P+m5yvR+4zCsspeceFUX0o68Q83B Qnaw29s8Qw4eZwOsZUXnvAW4VPrqqTCQ1/8GI1eJreFKxHop8pwVIjWstCHNA7U/E8XT PrOuqV63XznMDK+G97eXuARdmuLVGmbT4PQ9l4n32npti3PEYmEx6U3++9KDzUizvpT0 X6JRawRsw4PhJbMpLdQsl2eaeSniIFCSfOBwrtOoJKAHQE2H3CSYZygOaV2bucjvFKNI gPcfhth/5t2wmP2RQmIOvxh200HD99KMwVF9sxUTYx/rC7P5XPtOZX5GBr1LqWZkNIKW qU6Q== X-Gm-Message-State: ACrzQf1RfVfV7zBog6oetmpAxTWcbijMzUEljzaRX2zwYTh7EiwPcg7G YJeyLqRpUIuRlhonmUhYcxmgjG1yztMzcaAed12RrD+0GdrTr7gZOZtMyZ558nCYjX2vtsOQhHP UUfvOGukknqXr+hnoiwXJr/UdZvLWEhyrGyimrkSL9FhUG/OHb+qD/N/jd1h4LUYJ8BL3yEHl6g == X-Received: by 2002:adf:e504:0:b0:236:4882:e784 with SMTP id j4-20020adfe504000000b002364882e784mr4086446wrm.670.1666341834919; Fri, 21 Oct 2022 01:43:54 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6PjLDLLh/dy4HH+IksJ4MTVGs89f+Om/gp3h7dPDDtvuxGPtrxjsiNXRfiK2YBtLigU1GFjQ== X-Received: by 2002:adf:e504:0:b0:236:4882:e784 with SMTP id j4-20020adfe504000000b002364882e784mr4086428wrm.670.1666341834544; Fri, 21 Oct 2022 01:43:54 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id p24-20020a05600c1d9800b003c6f426467fsm2076443wms.40.2022.10.21.01.43.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:54 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 03/12] gdb: include breakpoint number in testing condition error message Date: Fri, 21 Oct 2022 09:43:39 +0100 Message-Id: <61be8bbe4a276fbd1643ee447edfd3e88cc13933.1666341010.git.aburgess@redhat.com> 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.8 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 GDB fails to test the condition of a conditional breakpoint, for whatever reason, the error message looks like this: (gdb) break foo if (*(int *) 0) == 1 Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Error in testing breakpoint condition: Cannot access memory at address 0x0 Breakpoint 1, foo () at bpcond.c:11 11 int a = 32; (gdb) The line I'm interested in for this commit is this one: Error in testing breakpoint condition: In the case above we can figure out that the problematic breakpoint was #1 because in the final line of the message GDB reports the stop a breakpoint #1. However, in the next few patches I plan to change this. In some cases I don't think it makes sense for GDB to report the stop as being at breakpoint #1, consider this case: (gdb) list some_func 1 int 2 some_func () 3 { 4 int *p = 0; 5 return *p; 6 } 7 8 void 9 foo () 10 { (gdb) break foo if (some_func ()) Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Program received signal SIGSEGV, Segmentation fault. 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; Error in testing breakpoint condition: The program being debugged was signaled while in a function called from GDB. GDB remains in the frame where the signal was received. To change this behavior use "set unwindonsignal on". Evaluation of the expression containing the function (some_func) will be abandoned. When the function is done executing, GDB will silently stop. Program received signal SIGSEGV, Segmentation fault. Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; (gdb) Notice that, the final lines of output report the stop as being at breakpoint #1, even though we are actually located within some_func. I find this behaviour confusing, and propose that this should be changed. However, if I make that change then every reference to breakpoint #1 will be lost from the error message. So, in this commit, in preparation for the later commits, I propose to change the 'Error in testing breakpoint condition:' line to this: Error in testing condition for breakpoint NUMBER: where NUMBER will be filled in as appropriate. Here's the first example with the updated error: (gdb) break foo if (*(int *) 0) == 0 Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Error in testing condition for breakpoint 1: Cannot access memory at address 0x0 Breakpoint 1, foo () at bpcond.c:11 11 int a = 32; (gdb) The breakpoint number does now appear twice in the output, but I don't see that as a negative. This commit just changes the one line of the error, and updates the few tests that either included the old error in comments, or actually checked for the error in the expected output. As the only test that checked the line I modified is a Python test, I've added a new test that doesn't rely on Python that checks the error message in detail. While working on the new test, I spotted that it would fail when run with native-gdbserver and native-extended-gdbserver target boards. This turns out to be due to a gdbserver bug. To avoid cluttering this commit I've added a work around to the new test script so that the test passes for the remote boards, in the next few commits I will fix gdbserver, and update the test script to remove the work around. --- gdb/breakpoint.c | 3 +- gdb/testsuite/gdb.base/bp-cond-failure.c | 30 +++++++ gdb/testsuite/gdb.base/bp-cond-failure.exp | 83 +++++++++++++++++++ .../gdb.base/catch-signal-siginfo-cond.exp | 2 +- gdb/testsuite/gdb.base/gnu-ifunc.exp | 2 +- .../gdb.python/py-finish-breakpoint.exp | 2 +- gdb/testsuite/lib/gdb.exp | 8 ++ 7 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.base/bp-cond-failure.c create mode 100644 gdb/testsuite/gdb.base/bp-cond-failure.exp diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 91e13a03360..4f5a27a6fe8 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5412,7 +5412,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, - "Error in testing breakpoint condition:\n"); + "Error in testing condition for breakpoint %d:\n", + b->number); } } else diff --git a/gdb/testsuite/gdb.base/bp-cond-failure.c b/gdb/testsuite/gdb.base/bp-cond-failure.c new file mode 100644 index 00000000000..be9d1fdcf2d --- /dev/null +++ b/gdb/testsuite/gdb.base/bp-cond-failure.c @@ -0,0 +1,30 @@ +/* Copyright 2022 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 +foo () +{ + return 0; /* Breakpoint here. */ +} + +int +main () +{ + int res = foo (); + + return res; +} diff --git a/gdb/testsuite/gdb.base/bp-cond-failure.exp b/gdb/testsuite/gdb.base/bp-cond-failure.exp new file mode 100644 index 00000000000..6f89771d187 --- /dev/null +++ b/gdb/testsuite/gdb.base/bp-cond-failure.exp @@ -0,0 +1,83 @@ +# Copyright 2022 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 . + +# Check the format of the error message given when a breakpoint +# condition fails. +# +# In this case the breakpoint condition does not make use of inferior +# function calls, instead, the expression used for the breakpoint +# condition will throw an error when evaluated. +# +# We check that the correct breakpoint number appears in the error +# message, and that the error is reported at the correct source +# location. + +standard_testfile + +if { [prepare_for_testing "failed to prepare" ${binfile} "${srcfile}" \ + {debug}] == -1 } { + return +} + +# This test relies on reading address zero triggering a SIGSEGV. +if { [is_address_zero_readable] } { + return +} + +# Where the breakpoint will be placed. +set bp_line [gdb_get_line_number "Breakpoint here"] + +proc run_test { cond_eval } { + clean_restart ${::binfile} + + if {![runto_main]} { + fail "run to main" + return -1 + } + + if { $cond_eval != "auto" } { + gdb_test_no_output "set breakpoint condition-evaluation ${cond_eval}" + } + + # Setup the conditional breakpoint and record its number. + gdb_breakpoint "${::srcfile}:${::bp_line} if (*(int *) 0) == 0" + set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*"] + + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + "Error in testing condition for breakpoint ${bp_num}:" \ + "Cannot access memory at address 0x0" \ + "" \ + "Breakpoint ${bp_num}, foo \\(\\) at \[^\r\n\]+:${::bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+Breakpoint here\[^\r\n\]+"] +} + +# If we're using a remote target then conditions could be evaulated +# locally on the host, or on the remote target. Otherwise, conditions +# are always evaluated locally (which is what auto will select). +# +# NOTE: 'target' is not included here for remote targets as a +# gdbserver bug prevents the test from passing. This will be fixed in +# the next commit, and this test updated. +if { [gdb_is_remote_or_extended_remote_target] } { + set cond_eval_modes { "host" } +} else { + set cond_eval_modes { "auto" } +} + +foreach_with_prefix cond_eval $cond_eval_modes { + run_test $cond_eval +} diff --git a/gdb/testsuite/gdb.base/catch-signal-siginfo-cond.exp b/gdb/testsuite/gdb.base/catch-signal-siginfo-cond.exp index 85a55b29ea5..d5fdbae2409 100644 --- a/gdb/testsuite/gdb.base/catch-signal-siginfo-cond.exp +++ b/gdb/testsuite/gdb.base/catch-signal-siginfo-cond.exp @@ -18,7 +18,7 @@ # # (gdb) continue # Continuing. -# Error in testing breakpoint condition: +# Error in testing condition for breakpoint NUM: # Selected thread is running. # # Catchpoint 3 (signal SIGUSR1), 0x0000003615e35877 in __GI_raise (sig=10) at raise.c:56 diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp index 3fbf237c71c..cbdcbbf07b4 100644 --- a/gdb/testsuite/gdb.base/gnu-ifunc.exp +++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp @@ -282,7 +282,7 @@ proc misc_tests {resolver_attr resolver_debug final_debug} { # Also test a former patch regression: # Continuing. - # Error in testing breakpoint condition: + # Error in testing condition for breakpoint NUM: # Attempt to take address of value not located in memory. # # Breakpoint 2, main () at ./gdb.base/gnu-ifunc.c:33 diff --git a/gdb/testsuite/gdb.python/py-finish-breakpoint.exp b/gdb/testsuite/gdb.python/py-finish-breakpoint.exp index c525337990e..d873fd5cd0f 100644 --- a/gdb/testsuite/gdb.python/py-finish-breakpoint.exp +++ b/gdb/testsuite/gdb.python/py-finish-breakpoint.exp @@ -193,7 +193,7 @@ with_test_prefix "finish in normal frame" { gdb_test "python TestBreakpoint()" "TestBreakpoint init" "set BP in condition" gdb_test "continue" \ - "test don't stop: 1.*test don't stop: 2.*test stop.*Error in testing breakpoint condition.*The program being debugged stopped while in a function called from GDB.*" \ + "test don't stop: 1.*test don't stop: 2.*test stop.*Error in testing condition for breakpoint ${::decimal}.*The program being debugged stopped while in a function called from GDB.*" \ "stop in condition function" gdb_test "continue" "Continuing.*" "finish condition evaluation" diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index c510ab25365..528e97bf099 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -9122,5 +9122,13 @@ gdb_caching_proc have_epilogue_line_info { } } +# Return true if we are currently testing the 'remote' or +# 'extended-remote' targets. +proc gdb_is_remote_or_extended_remote_target {} { + return [target_info exists gdb_protocol] + && ([target_info gdb_protocol] == "remote" + || [target_info gdb_protocol] == "extended-remote") +} + # Always load compatibility stuff. load_lib future.exp From patchwork Fri Oct 21 08:43:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59262 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 B10213850430 for ; Fri, 21 Oct 2022 08:44:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B10213850430 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341899; bh=9kM21eg1bGLltGrw24+FTeMuW4f18IimnYl1kFvaKBY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=dQGB27RMUFxg+hbaDcFQxx2UT9XTPKXoUnl3uUE0Kflwh9TBwsyJkZ9BkrEnz0gRj nFQ7wvR5FyHXAyqFTSuGNGVv/X1z1qmzOnHLPCdBspxrZVRkF6buCSgbnJiLmTbuPf MczDvZl0LxpamzQVNBb3YL+jG0Xh6pJYKnaRok/4= 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 ED8353856179 for ; Fri, 21 Oct 2022 08:43:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ED8353856179 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-58-xLdtQ21tNK-2lqBLPqyFGw-1; Fri, 21 Oct 2022 04:43:57 -0400 X-MC-Unique: xLdtQ21tNK-2lqBLPqyFGw-1 Received: by mail-wr1-f71.google.com with SMTP id h26-20020adfaa9a000000b002364ad63bbcso587329wrc.10 for ; Fri, 21 Oct 2022 01:43:57 -0700 (PDT) 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=9kM21eg1bGLltGrw24+FTeMuW4f18IimnYl1kFvaKBY=; b=mu16fdUg5D/09zuhYC6eulRg0xxgtE0TNC9cub3/1FR6nJCqeHar44QbI1dIC/5m8+ F4kOzS0W0n0VffVsc0q2BTCoGimuPMCWuCbgxQYsjvzgqOEsQf611+1h+jIy1md91kI+ X+36gbtTlSVXCfiF0uHgRS61Nz8MFt6hKwBFYkDIJNZ8+U6htTKPdKCN7DGqGd9sN951 HbNOLk5q0FXtdcCXFNPs5Pz+u/TAw29MHkj/1rPcPk1cQ70u+XLiB/KzLpnC1FR02AvG FQaIEdS6lO0oSlLTpKOOciA+cUkz5+ZFDw1LDMFTUfoo6nsggnd2qTZi97DBALxcMeDj 1Tug== X-Gm-Message-State: ACrzQf1pi2nPAZt2y3oeR5KJGjxjS1g1Yw/cq2TDNIA95ACU0zia67g9 Xx0SzXXNLCXYOQsOB6cjmRByQDy//1DhIuO8/dqOJvxyP41Nsd0bS0RYqQzmB+CNbyWkIP/I7U2 fznWbhUJHCTkpxPogeuKNEOmIkHPnEe7zR913TVVjVbYMXmcy1jKHpbEF+PqT1S1eBObS9ZUQ5Q == X-Received: by 2002:adf:f58b:0:b0:22e:3c4:cf83 with SMTP id f11-20020adff58b000000b0022e03c4cf83mr11890803wro.379.1666341836130; Fri, 21 Oct 2022 01:43:56 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7Omd/GyC+8LvCRIQjPYHRthejIDrAT85ktO1KEuZ0pZ5J0ZjTsDXEnguBmG/H1GmCErBuSjg== X-Received: by 2002:adf:f58b:0:b0:22e:3c4:cf83 with SMTP id f11-20020adff58b000000b0022e03c4cf83mr11890791wro.379.1666341835854; Fri, 21 Oct 2022 01:43:55 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id v21-20020a05600c15d500b003b4e009deb2sm2001634wmf.41.2022.10.21.01.43.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:55 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 04/12] gdbserver: add comments to read_inferior_memory function Date: Fri, 21 Oct 2022 09:43:40 +0100 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.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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" Just adding some comments to the gdbserver read_inferior_memory function. No actual code changes. --- gdbserver/target.cc | 2 ++ gdbserver/target.h | 3 +++ gdbserver/tracepoint.cc | 2 ++ 3 files changed, 7 insertions(+) diff --git a/gdbserver/target.cc b/gdbserver/target.cc index adcfe6e7bcc..6b29c99a61b 100644 --- a/gdbserver/target.cc +++ b/gdbserver/target.cc @@ -79,6 +79,8 @@ set_desired_process () return proc != nullptr; } +/* See target.h. */ + int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len) { diff --git a/gdbserver/target.h b/gdbserver/target.h index 6c536a30778..a49d7d4bac0 100644 --- a/gdbserver/target.h +++ b/gdbserver/target.h @@ -697,6 +697,9 @@ target_thread_pending_child (thread_info *thread) return the_target->thread_pending_child (thread); } +/* Read LEN bytes from MEMADDR in the buffer MYADDR. Return 0 if the read + is successful, otherwise, return a non-zero error code. */ + int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len); /* Set GDBserver's current thread to the thread the client requested diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc index 18b2b0b3d77..efcdfe12beb 100644 --- a/gdbserver/tracepoint.cc +++ b/gdbserver/tracepoint.cc @@ -344,6 +344,8 @@ tracepoint_look_up_symbols (void) GDBserver side. */ #ifdef IN_PROCESS_AGENT +/* See target.h. */ + int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len) { From patchwork Fri Oct 21 08:43:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59263 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 1D6A138650D0 for ; Fri, 21 Oct 2022 08:45:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1D6A138650D0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341919; bh=l4TLGPtbEh+Nvv3PzqHc1UUMdfwR1uzTBdzPVWNwq8M=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=WrawCTCJFiWQWl3LnEK+zI3PPOyOaAg3Qm2B+zTUyg8l8EMvt1UwuZG36tF0UQWwp 7LFhk8gveDjTL2rBraXUtVHPLGniyIVjc/STzsKYcUZqn6dTeVznEAUlEr1u1Qs3Z8 yNvF7CUvM4q9jn9spYUobW3kGTyjemU/hOBMgyn0= 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 588D03856162 for ; Fri, 21 Oct 2022 08:44:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 588D03856162 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-498-oFWnvs4jPt6VQBBkq3aHhQ-1; Fri, 21 Oct 2022 04:43:58 -0400 X-MC-Unique: oFWnvs4jPt6VQBBkq3aHhQ-1 Received: by mail-wm1-f71.google.com with SMTP id i203-20020a1c3bd4000000b003c6d18a7ee5so1149910wma.6 for ; Fri, 21 Oct 2022 01:43:58 -0700 (PDT) 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=l4TLGPtbEh+Nvv3PzqHc1UUMdfwR1uzTBdzPVWNwq8M=; b=Jmw06MfV5ux5K1ngfmSt9cHRJjcDkCENlW6nxkA30mORH5X7Xgbxb9Gx+BvGVdEU6g YeuHkrzgC7p+DA4Rc2ASmm3a5YQ6Edd8MHO5HgngHcKeMxqRtL/4DyVyc/umjd7dmeSa rAigmOWmhNBfLAfMsG3Up+PrgcgG2lN0MPashiwullOdaj2FWqwvifd95xqWIk63A23R nNgltAKPf0X089QZzOijVv42mwmVd4BoWWpdpUvECRp0Ivcz4J+bmstU78Bsc6DnjNRX nKmtix9TQNWTmUfTJUUamZmNWqwSmyO/0wGPuRKzHl4D0YjxeVQ93B1DdzdyndBJZ8Nm UPkA== X-Gm-Message-State: ACrzQf0SI/eS172+S5TYqmireRGWMQ7l18wVFujG/JOOWrzdI1vElHKN QGVyPZvvUcOaSj27lwXm5Cml0arLzqNXyiNV2KAG18caUlDwVU/HCxPZLfl17R5V4FajDn4n4d4 wIeUGzChXTQC4Jr6c91qBEZMiTpEL+W3pRkJbXSu4W7JH9C9ePCrdkNuDpxarymJKp42w6NbETA == X-Received: by 2002:a5d:4a4d:0:b0:22d:c8cb:8687 with SMTP id v13-20020a5d4a4d000000b0022dc8cb8687mr11247789wrs.554.1666341837426; Fri, 21 Oct 2022 01:43:57 -0700 (PDT) X-Google-Smtp-Source: AMsMyM62oXpLEJ0JltHd7ftld+bQ3P0PRU4Gr4GB7I+CXzuSHmvMB27leRR8UiQPMHOmlAzjmcxPxA== X-Received: by 2002:a5d:4a4d:0:b0:22d:c8cb:8687 with SMTP id v13-20020a5d4a4d000000b0022dc8cb8687mr11247778wrs.554.1666341837097; Fri, 21 Oct 2022 01:43:57 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id q124-20020a1c4382000000b003c706e177c1sm2068383wma.14.2022.10.21.01.43.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:56 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 05/12] gdbserver: allows agent_mem_read to return an error code Date: Fri, 21 Oct 2022 09:43:41 +0100 Message-Id: <2b2e5450488c5dc9422894ef36d67a644fc6db02.1666341010.git.aburgess@redhat.com> 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.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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" Currently the gdbserver function agent_mem_read ignores any errors from calling read_inferior_memory. This means that if there is an attempt to access invalid memory then this will appear to succeed. In this I update agent_mem_read so that if read_inferior_memory fails, agent_mem_read will return an error code. However, non of the callers of agent_mem_read actually check the return value, so this commit will have no effect on anything. In the next commit I will update the users of agent_mem_read to check for the error code. I've also updated the header comments on agent_mem_read to better reflect what the function does, and its possible return values. --- gdbserver/tracepoint.cc | 11 ++++------- gdbserver/tracepoint.h | 9 +++++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc index efcdfe12beb..74774759d3a 100644 --- a/gdbserver/tracepoint.cc +++ b/gdbserver/tracepoint.cc @@ -4923,8 +4923,7 @@ condition_true_at_tracepoint (struct tracepoint_hit_ctx *ctx, return (value ? 1 : 0); } -/* Do memory copies for bytecodes. */ -/* Do the recording of memory blocks for actions and bytecodes. */ +/* See tracepoint.h. */ int agent_mem_read (struct eval_agent_expr_context *ctx, @@ -4936,10 +4935,7 @@ agent_mem_read (struct eval_agent_expr_context *ctx, /* If a 'to' buffer is specified, use it. */ if (to != NULL) - { - read_inferior_memory (from, to, len); - return 0; - } + return read_inferior_memory (from, to, len); /* Otherwise, create a new memory block in the trace buffer. */ while (remaining > 0) @@ -4960,7 +4956,8 @@ agent_mem_read (struct eval_agent_expr_context *ctx, memcpy (mspace, &blocklen, sizeof (blocklen)); mspace += sizeof (blocklen); /* Record the memory block proper. */ - read_inferior_memory (from, mspace, blocklen); + if (read_inferior_memory (from, mspace, blocklen) != 0) + return 1; trace_debug ("%d bytes recorded", blocklen); remaining -= blocklen; from += blocklen; diff --git a/gdbserver/tracepoint.h b/gdbserver/tracepoint.h index bbffb6228f4..afab22b214d 100644 --- a/gdbserver/tracepoint.h +++ b/gdbserver/tracepoint.h @@ -161,8 +161,13 @@ void gdb_agent_about_to_close (int pid); struct traceframe; struct eval_agent_expr_context; -/* Do memory copies for bytecodes. */ -/* Do the recording of memory blocks for actions and bytecodes. */ +/* When TO is not NULL, do memory copies for bytecodes, read LEN bytes + starting at address FROM, and place the result in the buffer TO. + Return 0 on success, otherwise a non-zero error code. + + When TO is NULL, do the recording of memory blocks for actions and + bytecodes into a new traceframe block. Return 0 on success, otherwise, + return 1 if there is an error. */ int agent_mem_read (struct eval_agent_expr_context *ctx, unsigned char *to, CORE_ADDR from, From patchwork Fri Oct 21 08:43:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59266 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 5FC1C384B0CA for ; Fri, 21 Oct 2022 08:45:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5FC1C384B0CA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341951; bh=kjG1QlRCscnb3YYTOFPvqaxv7698dyfFOSLXzBJQre0=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=HRnVTpF+5iDrSSg9/WBtsIrEbJDIWNuvvB2L5vJkfc6LzE+A/tKY/hJwKhEJ5O766 UqnlXTL8TGBzD1KD+nFklRcmdfe/t1jvSVib2qgPXyfP9R57081HYZJ9VUWE78Nc6u LzblbQ1VnyWSCwlO2HrF9/ADtASfsVf07Xfx8Tqo= 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 618AD385617E for ; Fri, 21 Oct 2022 08:44:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 618AD385617E Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-443-HM19IpQlOU-PEzrk4uebbw-1; Fri, 21 Oct 2022 04:43:59 -0400 X-MC-Unique: HM19IpQlOU-PEzrk4uebbw-1 Received: by mail-wr1-f69.google.com with SMTP id i26-20020adfaada000000b0022e2f38ffccso581953wrc.14 for ; Fri, 21 Oct 2022 01:43:59 -0700 (PDT) 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=kjG1QlRCscnb3YYTOFPvqaxv7698dyfFOSLXzBJQre0=; b=BM13CVcpepnkQrQkQQ2oRnhaedwBs0wImF1L2kVJ+y2UVXKD2+tt8hzfEKPR2K/bDf OYfRrpXdlQ8UzGVgmB53PK4QdUY2x7IE/h4aQJnfuflYqbJfVzvrDYEQ3gPNb5yzPZ+3 h0OYF0rEGdJHjGwJtQUKrQ/Us64bFcBhCDW/CYRXZQCou2LvJabFnp/g1gGa9mA/pTq0 3njeTkj/YhbSfdDQcy3hnZu50KdkeUOQ5SoT8gDu0wfctj2lnRjewQUwmpyVLDQbGy7y asO4P8Tq1J4cUDh8WVjKnJfFE8SGK2lA1p5z4Tun/2W9ONw1pEzFbz5qP7VQa2aUTK45 rU9g== X-Gm-Message-State: ACrzQf2iNY1mqOk8GfMnKiVx8lxXVNjBKxfT5MrPmI3sgjh0recqSOaV a3QiHx44O/CnMrq5fJzSUgxaVIZbEWpt5A5GewMlo9QwRhVaHYChfPwJ0t2cgVGRZE2+U8gO41K 0MQe6viklwxe0wfSG3L6V13rhQPyrf29gYBEnDpuIjytCAgshwy+Ehb/DM6vTFLgZ5uMpIDRHCQ == X-Received: by 2002:a05:600c:1906:b0:3c6:f154:d4b5 with SMTP id j6-20020a05600c190600b003c6f154d4b5mr12121997wmq.94.1666341838555; Fri, 21 Oct 2022 01:43:58 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5A9cn3qMMenq8UUTISrwbnTDl/BmBqicZa7TNq160y8M+QIEXTEnSMdyAc1eEtPo3hInFwIw== X-Received: by 2002:a05:600c:1906:b0:3c6:f154:d4b5 with SMTP id j6-20020a05600c190600b003c6f154d4b5mr12121983wmq.94.1666341838348; Fri, 21 Oct 2022 01:43:58 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id k5-20020a5d6d45000000b0022e57e66824sm21598578wri.99.2022.10.21.01.43.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:58 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 06/12] gdbserver: allow agent expressions to fail with invalid memory access Date: Fri, 21 Oct 2022 09:43:42 +0100 Message-Id: <225a59db6143d8793f131f9489fa66f3166e788a.1666341010.git.aburgess@redhat.com> 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.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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" This commit extends gdbserver to take account of a failed memory access from agent_mem_read, and to return a new eval_result_type expr_eval_invalid_memory_access. I have only updated the agent_mem_read calls related directly to reading memory, I have not updated any of the calls related to tracepoint data collection. This is just because I'm not familiar with that area of gdb/gdbserver, and I don't want to break anything, so leaving the existing behaviour as is seems like the safest approach. I've then update gdb.base/bp-cond-failure.exp to test evaluating the breakpoints on the target, and have also extended the test so that it checks for different sizes of memory access. --- gdb/testsuite/gdb.base/bp-cond-failure.exp | 16 +++++++--------- gdbserver/ax.cc | 12 ++++++++---- gdbserver/ax.h | 3 ++- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/gdb/testsuite/gdb.base/bp-cond-failure.exp b/gdb/testsuite/gdb.base/bp-cond-failure.exp index 6f89771d187..aa39b638592 100644 --- a/gdb/testsuite/gdb.base/bp-cond-failure.exp +++ b/gdb/testsuite/gdb.base/bp-cond-failure.exp @@ -39,7 +39,7 @@ if { [is_address_zero_readable] } { # Where the breakpoint will be placed. set bp_line [gdb_get_line_number "Breakpoint here"] -proc run_test { cond_eval } { +proc run_test { cond_eval access_type } { clean_restart ${::binfile} if {![runto_main]} { @@ -52,7 +52,7 @@ proc run_test { cond_eval } { } # Setup the conditional breakpoint and record its number. - gdb_breakpoint "${::srcfile}:${::bp_line} if (*(int *) 0) == 0" + gdb_breakpoint "${::srcfile}:${::bp_line} if (*(${access_type} *) 0) == 0" set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*"] gdb_test "continue" \ @@ -68,16 +68,14 @@ proc run_test { cond_eval } { # If we're using a remote target then conditions could be evaulated # locally on the host, or on the remote target. Otherwise, conditions # are always evaluated locally (which is what auto will select). -# -# NOTE: 'target' is not included here for remote targets as a -# gdbserver bug prevents the test from passing. This will be fixed in -# the next commit, and this test updated. if { [gdb_is_remote_or_extended_remote_target] } { - set cond_eval_modes { "host" } + set cond_eval_modes { "host" "target" } } else { set cond_eval_modes { "auto" } } -foreach_with_prefix cond_eval $cond_eval_modes { - run_test $cond_eval +foreach_with_prefix access_type { "char" "short" "int" "long long" } { + foreach_with_prefix cond_eval $cond_eval_modes { + run_test $cond_eval $access_type + } } diff --git a/gdbserver/ax.cc b/gdbserver/ax.cc index 365bd2195b2..377ac9682a6 100644 --- a/gdbserver/ax.cc +++ b/gdbserver/ax.cc @@ -1112,22 +1112,26 @@ gdb_eval_agent_expr (struct eval_agent_expr_context *ctx, break; case gdb_agent_op_ref8: - agent_mem_read (ctx, cnv.u8.bytes, (CORE_ADDR) top, 1); + if (agent_mem_read (ctx, cnv.u8.bytes, (CORE_ADDR) top, 1) != 0) + return expr_eval_invalid_memory_access; top = cnv.u8.val; break; case gdb_agent_op_ref16: - agent_mem_read (ctx, cnv.u16.bytes, (CORE_ADDR) top, 2); + if (agent_mem_read (ctx, cnv.u16.bytes, (CORE_ADDR) top, 2) != 0) + return expr_eval_invalid_memory_access; top = cnv.u16.val; break; case gdb_agent_op_ref32: - agent_mem_read (ctx, cnv.u32.bytes, (CORE_ADDR) top, 4); + if (agent_mem_read (ctx, cnv.u32.bytes, (CORE_ADDR) top, 4) != 0) + return expr_eval_invalid_memory_access; top = cnv.u32.val; break; case gdb_agent_op_ref64: - agent_mem_read (ctx, cnv.u64.bytes, (CORE_ADDR) top, 8); + if (agent_mem_read (ctx, cnv.u64.bytes, (CORE_ADDR) top, 8) != 0) + return expr_eval_invalid_memory_access; top = cnv.u64.val; break; diff --git a/gdbserver/ax.h b/gdbserver/ax.h index b4062bf24a8..aad9e11078d 100644 --- a/gdbserver/ax.h +++ b/gdbserver/ax.h @@ -41,7 +41,8 @@ enum eval_result_type expr_eval_unhandled_opcode, expr_eval_unrecognized_opcode, expr_eval_divide_by_zero, - expr_eval_invalid_goto + expr_eval_invalid_goto, + expr_eval_invalid_memory_access }; struct agent_expr From patchwork Fri Oct 21 08:43:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59264 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 F07DE38515C2 for ; Fri, 21 Oct 2022 08:45:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F07DE38515C2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341920; bh=HI7KoRzHPWo5tv2ehK3+t5/Q41/BBniUp12ZFjMIeYY=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Q0pg/gJYRJmquSJBzmJ4TB/RJtRC/WloNJPnjefx5RZRFH7ZFlfHDFnEaWoxVmkE5 RWDEgyRfxlnVesCu62EiJKYmGsxTnMrom1IO+IOdY0KhQlRQaNCcwGtvAYgJRe+tbG EH8JEOKf2CMHpUVph1HMv3ttkHT7qg7Eudq+B2mM= 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 1CE313853831 for ; Fri, 21 Oct 2022 08:44:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1CE313853831 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-414-dLqyIfT_PAKRRsNKLfP9CQ-1; Fri, 21 Oct 2022 04:44:01 -0400 X-MC-Unique: dLqyIfT_PAKRRsNKLfP9CQ-1 Received: by mail-wm1-f70.google.com with SMTP id 2-20020a1c0202000000b003c6d73209b0so1163948wmc.1 for ; Fri, 21 Oct 2022 01:44:01 -0700 (PDT) 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=HI7KoRzHPWo5tv2ehK3+t5/Q41/BBniUp12ZFjMIeYY=; b=j4B4t3AxlxuSgzLvl2mDSNviLDtRdGluXhRAWhNxBIW44+8hVWigJYlJ2wZM8Tc/lv e0+yZxVCIHQS/8A2Wm0ChUkcuV+VriuXD5KE2vLnE6eXP51/JaZ91rSGMO+E9vm8VMJq kGuniyNC1WSV4k9hdHzBvyrB23imLL4qLBtsDhPjw51klUx3XvpV6X2C/lU31pXwIaEi kUVUiyFBc01Q2LJYPRjbLNDYrS0bWU5wykCqbtA3DO7jOmVm1a0VP6eEKChAFp2Gi2Ti kZb4blcQInLWx0zpHLhm7MU7w7IOWkMwxEA8mExW784QWKKXbAfJTdb/n0NoKCJg45kE Gojg== X-Gm-Message-State: ACrzQf1hSGLcNkNUoyxuoDe95AF3h1eLeElnw0Wy1Ii0A/Q+ynmcjoXj zjf6xilYGYwiqaATtN+8bIx/mcWEmhmEQSIJmXhhy0HzrHt5qPrBP+T66CdM4HMO4KEMbqmG8hA VeByDJJtscawlnEaA/aqZfPSOQVweyzH7GrCKrTV97c2ciCW18VnRU1wAW9Wp/M0Hs3xvC5C6RA == X-Received: by 2002:a05:6000:1882:b0:230:9595:4131 with SMTP id a2-20020a056000188200b0023095954131mr11716740wri.17.1666341840060; Fri, 21 Oct 2022 01:44:00 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7Xbo2qKR9+CWaP3rQephpVtU/NHfmR6zDD7YDjDgtVyjVNvwi2CixFSDEfD/DDjq70HH7KBw== X-Received: by 2002:a05:6000:1882:b0:230:9595:4131 with SMTP id a2-20020a056000188200b0023095954131mr11716720wri.17.1666341839566; Fri, 21 Oct 2022 01:43:59 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id p24-20020a05600c1d9800b003c6f426467fsm2076639wms.40.2022.10.21.01.43.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:43:59 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 07/12] gdb: avoid repeated signal reporting during failed conditional breakpoint Date: Fri, 21 Oct 2022 09:43:43 +0100 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.8 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" Consider the following case: (gdb) list some_func 1 int 2 some_func () 3 { 4 int *p = 0; 5 return *p; 6 } 7 8 void 9 foo () 10 { (gdb) break foo if (some_func ()) Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Program received signal SIGSEGV, Segmentation fault. 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; Error in testing breakpoint condition: The program being debugged was signaled while in a function called from GDB. GDB remains in the frame where the signal was received. To change this behavior use "set unwindonsignal on". Evaluation of the expression containing the function (some_func) will be abandoned. When the function is done executing, GDB will silently stop. Program received signal SIGSEGV, Segmentation fault. Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; (gdb) Notice that this line: Program received signal SIGSEGV, Segmentation fault. Appears twice in the output. The first time is followed by the current location. The second time is a little odd, why do we print that? Printing that line is controlled, in part, by a global variable, stopped_by_random_signal. This variable is reset to zero in handle_signal_stop, and is set if/when GDB figures out that the inferior stopped due to some random signal. The problem is, in our case, GDB first stops at the breakpoint for foo, and enters handle_signal_stop and the stopped_by_random_signal global is reset to 0. Later within handle_signal_stop GDB calls bpstat_stop_status, it is within this function (via bpstat_check_breakpoint_conditions) that the breakpoint condition is checked, and, we end up calling the inferior function (some_func in our example above). In our case above the thread performing the inferior function call segfaults in some_func. GDB catches the SIGSEGV and handles the stop, this causes us to reenter handle_signal_stop. The global variable stopped_by_random_signal is updated, this time it is set to true because the thread stopped due to SIGSEGV. As a result of this we print the first instance of the line (as seen above in the example). Finally we unwind GDB's call stack, the inferior function call is complete, and we return to the original handle_signal_stop. However, the stopped_by_random_signal global is still carrying the value as computed for the inferior function call's stop, which is why we now print a second instance of the line, as seen in the example. To prevent this, I propose adding a scoped_restore before we start an inferior function call, this will save and restore the global stopped_by_random_signal value. With this done, the output from our example is now this: (gdb) list some_func 1 int 2 some_func () 3 { 4 int *p = 0; 5 return *p; 6 } 7 8 void 9 foo () 10 { (gdb) break foo if (some_func ()) Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Program received signal SIGSEGV, Segmentation fault. 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; Error in testing condition for breakpoint 1: The program being debugged stopped while in a function called from GDB. Evaluation of the expression containing the function (some_func) will be abandoned. When the function is done executing, GDB will silently stop. Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; (gdb) We now only see the 'Program received signal SIGSEGV, ...' line once, which I think makes more sense. Finally, I'm aware that the last few lines, that report the stop as being at 'Breakpoint 1', when this is not where the thread is actually located anymore, is not great. I'll address that in the next commit. --- gdb/infcall.c | 9 + gdb/testsuite/gdb.base/infcall-failure.c | 48 ++++++ gdb/testsuite/gdb.base/infcall-failure.exp | 184 +++++++++++++++++++++ 3 files changed, 241 insertions(+) create mode 100644 gdb/testsuite/gdb.base/infcall-failure.c create mode 100644 gdb/testsuite/gdb.base/infcall-failure.exp diff --git a/gdb/infcall.c b/gdb/infcall.c index e2de045d7b7..4957eee0b85 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -1299,6 +1299,15 @@ call_function_by_hand_dummy (struct value *function, /* Register a clean-up for unwind_on_terminating_exception_breakpoint. */ SCOPE_EXIT { delete_std_terminate_breakpoint (); }; + /* The stopped_by_random_signal variable is global. If we are here + as part of a breakpoint condition check then the global will have + already been setup as part of the original breakpoint stop. By + making the inferior call the global will be changed when GDB + handles the stop after the inferior call. Avoid confusion by + restoring the current value after the inferior call. */ + scoped_restore restore_stopped_by_random_signal + = make_scoped_restore (&stopped_by_random_signal, 0); + /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - If you're looking to implement asynchronous dummy-frames, then just below is the place to chop this function in two.. */ diff --git a/gdb/testsuite/gdb.base/infcall-failure.c b/gdb/testsuite/gdb.base/infcall-failure.c new file mode 100644 index 00000000000..00f4369e164 --- /dev/null +++ b/gdb/testsuite/gdb.base/infcall-failure.c @@ -0,0 +1,48 @@ +/* Copyright 2022 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 . */ + +/* A function that segfaults (assuming that reads of address zero are + prohibited), this is used from within a breakpoint condition. */ +int +func_segfault () +{ + volatile int *p = 0; + return *p; /* Segfault here. */ +} + +/* A function in which we will place a breakpoint. This function is itself + then used from within a breakpoint condition. */ +int +func_bp () +{ + int res = 0; /* Second breakpoint. */ + return res; +} + +int +foo () +{ + return 0; /* First breakpoint. */ +} + +int +main () +{ + int res = foo (); + + return res; +} diff --git a/gdb/testsuite/gdb.base/infcall-failure.exp b/gdb/testsuite/gdb.base/infcall-failure.exp new file mode 100644 index 00000000000..2dcdda34b4d --- /dev/null +++ b/gdb/testsuite/gdb.base/infcall-failure.exp @@ -0,0 +1,184 @@ +# Copyright 2022 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 . + +# Some simple tests of inferior function calls from breakpoint +# conditions, in a single-threaded inferior. +# +# Test what happens when the inferior function (from a breakpoint +# condition) either hits a nested breakpoint, or segfaults. + +standard_testfile + +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ + {debug}] == -1 } { + return +} + +set bp_1_line [gdb_get_line_number "First breakpoint"] +set bp_2_line [gdb_get_line_number "Second breakpoint"] +set segv_line [gdb_get_line_number "Segfault here"] + +# Start GDB based on TARGET_ASYNC and TARGET_NON_STOP, and then runto +# main. +proc start_gdb_and_runto_main { target_async target_non_stop } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS \ + " -ex \"maint set target-non-stop $target_non_stop\"" + append ::GDBFLAGS \ + " -ex \"maintenance set target-async ${target_async}\"" + + clean_restart ${::binfile} + } + + if {![runto_main]} { + fail "run to main" + return -1 + } + + return 0 +} + +# Start GDB according to ASYNC_P and NON_STOP_P, then setup a +# conditional breakpoint. The breakpoint condition includes an +# inferior function call that will itself hit a breakpoint. Check how +# GDB reports this to the user. +proc_with_prefix run_cond_hits_breakpoint_test { async_p non_stop_p } { + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { + return + } + + # Setup the conditional breakpoint and record its number. + gdb_breakpoint "${::srcfile}:${::bp_1_line} if (func_bp ())" + set bp_1_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of first breakpoint"] + + # Setup a breakpoint inside func_bp. + gdb_breakpoint "${::srcfile}:${::bp_2_line}" + set bp_2_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of second breakpoint"] + + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + "" \ + "Breakpoint ${bp_2_num}, func_bp \\(\\) at \[^\r\n\]+:${::bp_2_line}" \ + "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+" \ + "Error in testing condition for breakpoint ${bp_1_num}:" \ + "The program being debugged stopped while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(func_bp\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\." \ + "" \ + "Breakpoint ${bp_1_num}, \[^\r\n\]+" \ + "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+"] +} + +# Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior +# function. The inferior function being called will itself have a +# breakpoint within it. Check how GDB reports this to the user. +proc_with_prefix run_call_hits_breakpoint_test { async_p non_stop_p } { + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { + return + } + + # Setup a breakpoint inside func_bp. + gdb_breakpoint "${::srcfile}:${::bp_2_line}" + set bp_2_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of second breakpoint"] + + + gdb_test "call func_bp ()" \ + [multi_line \ + "" \ + "Breakpoint ${bp_2_num}, func_bp \\(\\) at \[^\r\n\]+:${::bp_2_line}" \ + "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+" \ + "The program being debugged stopped while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(func_bp\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\."] +} + +# Start GDB according to ASYNC_P and NON_STOP_P, then setup a +# conditional breakpoint. The breakpoint condition includes an +# inferior function call that segfaults. Check how GDB reports this +# to the user. +proc_with_prefix run_cond_hits_segfault_test { async_p non_stop_p } { + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { + return + } + + # This test relies on the inferior segfaulting when trying to + # access address zero. + if { [is_address_zero_readable] } { + return + } + + # Setup the conditional breakpoint and record its number. + gdb_breakpoint "${::srcfile}:${::bp_1_line} if (func_segfault ())" + set bp_1_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of first breakpoint"] + + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + "" \ + "Program received signal SIGSEGV, Segmentation fault\\." \ + "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \ + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \ + "Error in testing condition for breakpoint ${bp_1_num}:" \ + "The program being debugged stopped while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(func_segfault\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\." \ + "" \ + "Breakpoint ${bp_1_num}, \[^\r\n\]+" \ + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] +} + +# Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior +# function. The inferior function will segfault. Check how GDB +# reports this to the user. +proc_with_prefix run_call_hits_segfault_test { async_p non_stop_p } { + if { [start_gdb_and_runto_main $async_p $non_stop_p] == -1 } { + return + } + + # This test relies on the inferior segfaulting when trying to + # access address zero. + if { [is_address_zero_readable] } { + return + } + + gdb_test "call func_segfault ()" \ + [multi_line \ + "" \ + "Program received signal SIGSEGV, Segmentation fault\\." \ + "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \ + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \ + "The program being debugged stopped while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(func_segfault\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\."] +} + +foreach_with_prefix target_async { "on" "off" } { + foreach_with_prefix target_non_stop { "on" "off" } { + run_cond_hits_breakpoint_test $target_async $target_non_stop + run_call_hits_breakpoint_test $target_async $target_non_stop + + run_cond_hits_segfault_test $target_async $target_non_stop + run_call_hits_segfault_test $target_async $target_non_stop + } +} From patchwork Fri Oct 21 08:43:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59265 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 AF1B538515D9 for ; Fri, 21 Oct 2022 08:45:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AF1B538515D9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341938; bh=z7dP9i6n3f6pAN54dLl9QQBX5wHwWkrIYe77t8fVhps=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=DZqcoicy3dTcZIFJcBfLJWxp5UtkFETdheSwKwG6klcPutf/2BpDkTdXvH50MvRsw Na9Xonh6lTvxCQbcvcFWI6A0LaX34cVMR2ZEL4za3FQ6ZmAp8EgU3aO1ifG+Ya9TU0 gAgeX/svW75JK8GCdU/561mJMhNRa5ZKy95IK0uY= 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 7B8633854177 for ; Fri, 21 Oct 2022 08:44:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7B8633854177 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-508-uJMgO49jMC2ZWk8Dc48_OA-1; Fri, 21 Oct 2022 04:44:02 -0400 X-MC-Unique: uJMgO49jMC2ZWk8Dc48_OA-1 Received: by mail-wm1-f70.google.com with SMTP id i203-20020a1c3bd4000000b003c6d18a7ee5so1149990wma.6 for ; Fri, 21 Oct 2022 01:44:02 -0700 (PDT) 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=z7dP9i6n3f6pAN54dLl9QQBX5wHwWkrIYe77t8fVhps=; b=yL+O1AJ+KdkPwNY0KqVnURAoy8Ap9Hkh/HvsWD9CMHpp/WZDrCxUNZbtbIBndg6zHd i33Z1sCcwj4gPKO1bDbq5yyA3RecbirG3CI/Qok1+pl3oO8pneGy+mrfnEdMY5Ln3ZW3 YL44AD1abSgutrzCaORySmguM0Fra1tVcmmchYIVO+eQ4jwqxsTB2CtIjnhZ/9opCaRk UIDp1kNJ5mvKVOt0TjZnlylA6oA8TV1Xjmz3apZLVmFQooGGJfXhTy6OCmGi0It/CU8+ VhMdML5jPVLpbp/tgd8xYU8nvvRp1rC6g10S60rf6B5nF9/BW0RwAElmD1/Xdg3Py85q eONQ== X-Gm-Message-State: ACrzQf1mAR9OusxBkZo14MeXGmnhiRgOtJyiFPUSGeL5x4O7TdhTkIdQ DFlITn8kPErkv703DhCRlqsWrSn34vZo0sVEvyXjW5y82r7J48UjpAPja4dh17IwAEZOAP+Z00h joNYAWiGLWhYwNGnJP/ZoczeikI4hd6XUXRrmL2Ed3tlvfGWn/cFLVI/GZ52mNAPZuaFpoglUHw == X-Received: by 2002:adf:dc51:0:b0:235:4db8:2d3b with SMTP id m17-20020adfdc51000000b002354db82d3bmr5488669wrj.589.1666341841520; Fri, 21 Oct 2022 01:44:01 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5fzlrpZHjJ3GvovbHZfOQK13L6rTAyDvZnlr6S6/p20a5E8DC30eCTAEY7m/HAT50Kxs1BIw== X-Received: by 2002:adf:dc51:0:b0:235:4db8:2d3b with SMTP id m17-20020adfdc51000000b002354db82d3bmr5488659wrj.589.1666341841194; Fri, 21 Oct 2022 01:44:01 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id p63-20020a1c2942000000b003c6c5a5a651sm1837355wmp.28.2022.10.21.01.44.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:44:00 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 08/12] gdb: don't always print breakpoint location after failed condition check Date: Fri, 21 Oct 2022 09:43:44 +0100 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.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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" Consider the following session: (gdb) list some_func 1 int 2 some_func () 3 { 4 int *p = 0; 5 return *p; 6 } 7 8 void 9 foo () 10 { (gdb) break foo if (some_func ()) Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Program received signal SIGSEGV, Segmentation fault. 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; Error in testing condition for breakpoint 1: The program being debugged stopped while in a function called from GDB. Evaluation of the expression containing the function (some_func) will be abandoned. When the function is done executing, GDB will silently stop. Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; (gdb) What happens here is the breakpoint condition includes a call to an inferior function, and the inferior function segfaults. We can see that GDB reports the segfault, and then gives an error message that indicates that an inferior function call was interrupted. After this GDB appears to report that it is stopped at Breakpoint 1, inside some_func. I find this second stop report a little confusing. Yes, GDB has stopped as a result of hitting breakpoint 1, but, I think the message as it currently is might give the impression that the thread is actually stopped at a location of breakpoint 1, which is not the case. Also, I find the second stop message draws attention away from the "Program received signal SIGSEGV, Segmentation fault" stop message, and this second stop might be thought of as replacing in someway the earlier message. In short, I think that the in the situation above, I think things would be clearer if the second stop message were not reported at all, so the output should (I think) look like this: (gdb) list some_func 1 int 2 some_func () 3 { 4 int *p = 0; 5 return *p; 6 } 7 8 void 9 foo () 10 { (gdb) break foo if (some_func ()) Breakpoint 1 at 0x40111e: file bpcond.c, line 11. (gdb) r Starting program: /tmp/bpcond Program received signal SIGSEGV, Segmentation fault. 0x0000000000401116 in some_func () at bpcond.c:5 5 return *p; Error in testing condition for breakpoint 1: The program being debugged stopped while in a function called from GDB. Evaluation of the expression containing the function (some_func) will be abandoned. When the function is done executing, GDB will silently stop. (gdb) The user can still find the number of the breakpoint that triggered the initial stop in this line: Error in testing condition for breakpoint 1: But there's now only one stop reason reported, the SIGSEGV, which I think is much clearer. To achieve this change I set the bpstat::print field when: (a) a breakpoint condition evaluation failed, and (b) the $pc of the thread changed during condition evaluation. I've updated the existing tests that checked the error message printed when a breakpoint condition evaluation failed. --- gdb/breakpoint.c | 12 ++++++++++++ gdb/testsuite/gdb.base/infcall-failure.exp | 18 ++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 4f5a27a6fe8..606b2d191f4 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5403,6 +5403,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) else within_current_scope = 0; } + CORE_ADDR pc_before_check = get_frame_pc (get_selected_frame (nullptr)); if (within_current_scope) { try @@ -5414,6 +5415,17 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) exception_fprintf (gdb_stderr, ex, "Error in testing condition for breakpoint %d:\n", b->number); + + /* If the pc value changed as a result of evaluating the + condition then we probably stopped within an inferior + function call due to some unexpected stop, e.g. the thread + hit another breakpoint, or the thread received an + unexpected signal. In this case we don't want to also + print the information about this breakpoint. */ + CORE_ADDR pc_after_check + = get_frame_pc (get_selected_frame (nullptr)); + if (pc_before_check != pc_after_check) + bs->print = 0; } } else diff --git a/gdb/testsuite/gdb.base/infcall-failure.exp b/gdb/testsuite/gdb.base/infcall-failure.exp index 2dcdda34b4d..8fa2c407fc5 100644 --- a/gdb/testsuite/gdb.base/infcall-failure.exp +++ b/gdb/testsuite/gdb.base/infcall-failure.exp @@ -79,10 +79,7 @@ proc_with_prefix run_cond_hits_breakpoint_test { async_p non_stop_p } { "The program being debugged stopped while in a function called from GDB\\." \ "Evaluation of the expression containing the function" \ "\\(func_bp\\) will be abandoned\\." \ - "When the function is done executing, GDB will silently stop\\." \ - "" \ - "Breakpoint ${bp_1_num}, \[^\r\n\]+" \ - "${::decimal}\\s+\[^\r\n\]+Second breakpoint\[^\r\n\]+"] + "When the function is done executing, GDB will silently stop\\."] } # Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior @@ -138,13 +135,12 @@ proc_with_prefix run_cond_hits_segfault_test { async_p non_stop_p } { "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \ "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \ "Error in testing condition for breakpoint ${bp_1_num}:" \ - "The program being debugged stopped while in a function called from GDB\\." \ + "The program being debugged was signaled while in a function called from GDB\\." \ + "GDB remains in the frame where the signal was received\\." \ + "To change this behavior use \"set unwindonsignal on\"\\." \ "Evaluation of the expression containing the function" \ "\\(func_segfault\\) will be abandoned\\." \ - "When the function is done executing, GDB will silently stop\\." \ - "" \ - "Breakpoint ${bp_1_num}, \[^\r\n\]+" \ - "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] + "When the function is done executing, GDB will silently stop\\."] } # Start GDB according to ASYNC_P and NON_STOP_P, then call an inferior @@ -167,7 +163,9 @@ proc_with_prefix run_call_hits_segfault_test { async_p non_stop_p } { "Program received signal SIGSEGV, Segmentation fault\\." \ "${::hex} in func_segfault \\(\\) at \[^\r\n\]+:${::segv_line}" \ "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+" \ - "The program being debugged stopped while in a function called from GDB\\." \ + "The program being debugged was signaled while in a function called from GDB\\." \ + "GDB remains in the frame where the signal was received\\." \ + "To change this behavior use \"set unwindonsignal on\"\\." \ "Evaluation of the expression containing the function" \ "\\(func_segfault\\) will be abandoned\\." \ "When the function is done executing, GDB will silently stop\\."] From patchwork Fri Oct 21 08:43:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59267 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 956CD3839DF7 for ; Fri, 21 Oct 2022 08:45:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 956CD3839DF7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341951; bh=l7M/wtwtiivTcSlwU3MTNeRFyPZjHt7vS0UJyTsznRw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=aD6ETQqM4eLi8W6EyM79EpAS3tDD0g8EXrP5pRH66VSmQgMDdJInx1qX6g+HjzDrb T3Me5NVBXvuYWOi9cltbTfBzUOFP/Dbu63Agu2R6fElkK8yXABQ6YMrDbXHVSuCLQ0 vwgj6vaV+p4EdMNtir9W1INecNUAvSQQnzS6XUAQ= 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 7BD253856168 for ; Fri, 21 Oct 2022 08:44:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7BD253856168 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-224-ZG3ndIUWNSOQsTGVBjN5Xw-1; Fri, 21 Oct 2022 04:44:04 -0400 X-MC-Unique: ZG3ndIUWNSOQsTGVBjN5Xw-1 Received: by mail-wm1-f72.google.com with SMTP id bg21-20020a05600c3c9500b003c2acbff422so2037201wmb.0 for ; Fri, 21 Oct 2022 01:44:03 -0700 (PDT) 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=l7M/wtwtiivTcSlwU3MTNeRFyPZjHt7vS0UJyTsznRw=; b=eFgWDYKuhjh3KW0nSpk6I6paQscDOJoVxwRiYC0OxpXvAA8Zo7rwrVSSTQfrElk/kY 8juUFnBgY6UkbtZyLd1/ex3zDXrYsdir9+oL82mXug9Vaovy2op7uJB4baGF/N2FF8uF ZSxwf5sgP9KuqxxJbMZ9SEBB/u1z47N+vqpVxbvjo4NxN2VcmITXSBNk5pg9fuE6PBpP s48zCOvbnEVWDh21lQH0z7JELRqiT/ztRixh16uiK+xuJB0SMupNh13EDQU5TRKYis56 lLmcDcpzGBA0jQWnOJrPnT/voRlY1Hs0YZtLd45JqanlKUeekB9o0211tiI0S0q8hzpt +VQw== X-Gm-Message-State: ACrzQf2Bz+8vhxe34OFO244gm/z/UDnS4VwUJhfnmDtjxrFJ8bq8z8Bs ExN8ApogFdTkOwYkaaKF1EDzpn6NbmO3pypi3NbWQ+3DprSxGBFTCUbN4p4g0DoITno0idEBEZ4 f+1SVjZpX79MjJtMWoY2TPJXaCmz4aZGwQTlLEvuASd6l8iXxOe+2Af0r/hgW4PAe/jTZjeY9tQ == X-Received: by 2002:a05:600c:468a:b0:3c6:f84a:1fae with SMTP id p10-20020a05600c468a00b003c6f84a1faemr16385957wmo.148.1666341842789; Fri, 21 Oct 2022 01:44:02 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5sSYz2G15c9ArPXffg9kTB3+z0ZbbCkIKCzREVhHj69d+lMNndDD/y7GxcF/XOB/8G3pOnnA== X-Received: by 2002:a05:600c:468a:b0:3c6:f84a:1fae with SMTP id p10-20020a05600c468a00b003c6f84a1faemr16385944wmo.148.1666341842458; Fri, 21 Oct 2022 01:44:02 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id l14-20020a05600c1d0e00b003b435c41103sm2723332wms.0.2022.10.21.01.44.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:44:02 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 09/12] Revert "gdb: remove unnecessary parameter wait_ptid from do_target_wait" Date: Fri, 21 Oct 2022 09:43:45 +0100 Message-Id: <05a361f66e12aed286a5f16af402a0dd1100af93.1666341010.git.aburgess@redhat.com> 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.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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" This reverts commit ac0d67ed1dcf470bad6a3bc4800c2ddc9bedecca. There was nothing wrong with the commit which I'm reverting here, but it removed some functionality that will be needed for a later commit; that is, the ability for GDB to ask for events from a specific ptid_t via the do_target_wait function. In a follow up commit, this functionality will be used to implement inferior function calls in multi-threaded inferiors. --- gdb/infrun.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index c28bf28a4d9..b23c3e3918e 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3770,7 +3770,8 @@ do_target_wait_1 (inferior *inf, ptid_t ptid, more events. Polls for events from all inferiors/targets. */ static bool -do_target_wait (execution_control_state *ecs, target_wait_flags options) +do_target_wait (ptid_t wait_ptid, execution_control_state *ecs, + target_wait_flags options) { int num_inferiors = 0; int random_selector; @@ -3780,9 +3781,10 @@ do_target_wait (execution_control_state *ecs, target_wait_flags options) polling the rest of the inferior list starting from that one in a circular fashion until the whole list is polled once. */ - auto inferior_matches = [] (inferior *inf) + auto inferior_matches = [&wait_ptid] (inferior *inf) { - return inf->process_target () != nullptr; + return (inf->process_target () != NULL + && ptid_t (inf->pid).matches (wait_ptid)); }; /* First see how many matching inferiors we have. */ @@ -3821,7 +3823,7 @@ do_target_wait (execution_control_state *ecs, target_wait_flags options) auto do_wait = [&] (inferior *inf) { - ecs->ptid = do_target_wait_1 (inf, minus_one_ptid, &ecs->ws, options); + ecs->ptid = do_target_wait_1 (inf, wait_ptid, &ecs->ws, options); ecs->target = inf->process_target (); return (ecs->ws.kind () != TARGET_WAITKIND_IGNORE); }; @@ -4200,7 +4202,7 @@ fetch_inferior_event () the event. */ scoped_disable_commit_resumed disable_commit_resumed ("handling event"); - if (!do_target_wait (ecs, TARGET_WNOHANG)) + if (!do_target_wait (minus_one_ptid, ecs, TARGET_WNOHANG)) { infrun_debug_printf ("do_target_wait returned no event"); disable_commit_resumed.reset_and_commit (); From patchwork Fri Oct 21 08:43:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59269 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 EFD94384D17E for ; Fri, 21 Oct 2022 08:46:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EFD94384D17E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341988; bh=puSXl70rZ2OZH/iO4yINBqy0Og+JOKJvNzXzUtevKDw=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Ayhd/zJd8zaKp6SgM0I4CraK+lOjPOFobg0Xt2LbTFynYGi5hsGMPyNL5lggNzH1K pupr5g93/eXi+ewvy04V+3i19QZGxswg6NapCjCXygut7ZjxR8Hh/5aKe6hB+Ioy9b /oKp22YkYo7V5D7MK7J1ajDAmMgN/VZLFm005qvE= 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 EF47B3856DF6 for ; Fri, 21 Oct 2022 08:44:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EF47B3856DF6 Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-460-KRJSE5tePKayWfbOCXhNIw-1; Fri, 21 Oct 2022 04:44:07 -0400 X-MC-Unique: KRJSE5tePKayWfbOCXhNIw-1 Received: by mail-wr1-f72.google.com with SMTP id u20-20020adfc654000000b0022cc05e9119so576745wrg.16 for ; Fri, 21 Oct 2022 01:44:07 -0700 (PDT) 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=puSXl70rZ2OZH/iO4yINBqy0Og+JOKJvNzXzUtevKDw=; b=wyVWq8eC6AAOxAE0btWnVrCTBTLdAzq9skJcgxdTOCCscLeChnr2rSDXBe3KIfbzD7 zBU53kjLbXZ9R6vRZ/jTsRtv6ZVofYtHCaAOHE/hKm21UdjwGpPxKAH64ky6iquH1oca pxOkIjhz9CALiHjR8HrX5VYsHBdV8Po8Tjcl1Rv8dP+dlGOaxqxCIajLl4qXykjIqvaY V9x1m+VvQ6mJHrDEBq/RLIwiOX5MrUx9Wrn3vkN/einn8YsktyFIxCwJROa8RRSWTKGA FmjwRI9XtX+H04QnEMBy4eJJu9RaNbRrXfDGGXmKobb1mkwA0KOPgg521gMPxls497Cu Ry+A== X-Gm-Message-State: ACrzQf0FD6To0R5etBtDz3vuTm6zsUTpW0TnOi7FRs9kaAjvavX76qL1 xI7DO4FA8ltdoL3Lm432sI6OsCF+U37+t1CzNpI95/lSdGiLkzc+Q4JLpsEfWl9ZZFJj1xe5Dnp 9Vqtg5C7IzZ5h6B8FwmdkA80mqL/taGwe37gyYvDYwPDWScq4iZJSgvmdkIytXYI/F+/bmDq1aw == X-Received: by 2002:a5d:648c:0:b0:22e:6417:751a with SMTP id o12-20020a5d648c000000b0022e6417751amr11253810wri.261.1666341845225; Fri, 21 Oct 2022 01:44:05 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5oW7Tehn5COMGutDrsxZa7EDk0jAWUVZEgs+veibxYhCyVrGlrcOZ9HMv6NhC+EpQ/TF6BWw== X-Received: by 2002:a5d:648c:0:b0:22e:6417:751a with SMTP id o12-20020a5d648c000000b0022e6417751amr11253757wri.261.1666341843948; Fri, 21 Oct 2022 01:44:03 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id f16-20020a056000129000b0022dc6e76bbdsm18375523wrx.46.2022.10.21.01.44.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:44:03 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 10/12] gdb: fix b/p conditions with infcalls in multi-threaded inferiors Date: Fri, 21 Oct 2022 09:43:46 +0100 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.9 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" This commit fixes bug PR 28942, that is, creating a conditional breakpoint in a multi-threaded inferior, where the breakpoint condition includes an inferior function call. Currently, when a user tries to create such a breakpoint, then GDB will fail with: (gdb) break infcall-from-bp-cond-single.c:61 if (return_true ()) Breakpoint 2 at 0x4011fa: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c, line 61. (gdb) continue Continuing. [New Thread 0x7ffff7c5d700 (LWP 2460150)] [New Thread 0x7ffff745c700 (LWP 2460151)] [New Thread 0x7ffff6c5b700 (LWP 2460152)] [New Thread 0x7ffff645a700 (LWP 2460153)] [New Thread 0x7ffff5c59700 (LWP 2460154)] Error in testing breakpoint condition: Couldn't get registers: No such process. An error occurred while in a function called from GDB. Evaluation of the expression containing the function (return_true) will be abandoned. When the function is done executing, GDB will silently stop. Selected thread is running. (gdb) Or, in some cases, like this: (gdb) break infcall-from-bp-cond-simple.c:56 if (is_matching_tid (arg, 1)) Breakpoint 2 at 0x401194: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c, line 56. (gdb) continue Continuing. [New Thread 0x7ffff7c5d700 (LWP 2461106)] [New Thread 0x7ffff745c700 (LWP 2461107)] ../../src.release/gdb/nat/x86-linux-dregs.c:146: internal-error: x86_linux_update_debug_registers: Assertion `lwp_is_stopped (lwp)' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. The precise error depends on the exact thread state; so there's race conditions depending on which threads have fully started, and which have not. But the underlying problem is always the same; when GDB tries to execute the inferior function call from within the breakpoint condition, GDB will, incorrectly, try to resume threads that are already running - GDB doesn't realise that some threads might already be running. The solution proposed in this patch requires an additional member variable thread_info::in_cond_eval. This flag is set to true (in breakpoint.c) when GDB is evaluating a breakpoint condition. In user_visible_resume_ptid (infrun.c), when the in_cond_eval flag is true, then GDB will only try to resume the current thread, that is, the thread for which the breakpoint condition is being evaluated. This solves the problem of GDB trying to resume threads that are already running. The next problem is that inferior function calls are assumed to be synchronous, that is, GDB doesn't expect to start an inferior function call in thread #1, then receive a stop from thread #2 for some other, unrelated reason. To prevent GDB responding to an event from another thread, we update fetch_inferior_event and do_target_wait in infrun.c, so that, when an inferior function call (on behalf of a breakpoint condition) is in progress, we only wait for events from the current thread (the one evaluating the condition). The fix in do_target_wait is because previously, we only ever waited for the general any-thread, minus_one_ptid, for which matching against the inferior::pid would always succeed. However, now we might wait against a specific ptid value, in which case we need to ensure we only compare the pid part of the ptid. In fetch_inferior_event, after receiving the event, we only want to stop all the other threads, and call inferior_event_handler with INF_EXEC_COMPLETE, if we are not evaluating a conditional breakpoint. If we are, then all the other threads should be left doing whatever they were before. The inferior_event_handler call will be performed once the breakpoint condition has finished being evaluated, and GDB decides to stop or not. The final problem that needs solving relates to GDB's commit-resume mechanism, this allows GDB to collect resume requests into a single packet in order to reduce traffic to a remote target. The problem is that the commit-resume mechanism will not send any resume requests for an inferior if there are already events pending on the GDB side. Imagine an inferior with two threads. Both threads hit a breakpoint, maybe the same conditional breakpoint. At this point there are two pending events, one for each thread. GDB selects one of the events and spots that this is a conditional breakpoint, GDB evaluates the condition. The condition includes an inferior function call, so GDB sets up for the call and resumes the one thread, the resume request is added to the commit-resume queue. When the commit-resume queue is committed GDB sees that there is a pending event from another thread, and so doesn't send any resume requests to the actual target, GDB is assuming that when we wait we will select the event from the other thread. However, as this is an inferior function call for a condition evaluation, we will not select the event from the other thread, we only care about events from the thread that is evaluating the condition - and the resume for this thread was never sent to the target. And so, GDB hangs, waiting for an event from a thread that was never fully resumed. To fix this issue I have added the concept of "forcing" the commit-resume queue. When enabling commit resume, if the force flag is true, then any resumes will be committed to the target, even if there are other threads with pending events. A note on authorship: this patch was based on some work done by Natalia Saiapova and Tankut Baris Aktemur from Intel[1]. I have made some changes to their work in this version. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28942 [1] https://sourceware.org/pipermail/gdb-patches/2020-October/172454.html Co-authored-by: Natalia Saiapova Co-authored-by: Tankut Baris Aktemur --- gdb/breakpoint.c | 2 + gdb/gdbthread.h | 3 + gdb/infcall.c | 6 + gdb/infrun.c | 46 +++- gdb/infrun.h | 2 +- .../infcall-from-bp-cond-other-thread-event.c | 135 ++++++++++ ...nfcall-from-bp-cond-other-thread-event.exp | 175 +++++++++++++ .../gdb.threads/infcall-from-bp-cond-simple.c | 89 +++++++ .../infcall-from-bp-cond-simple.exp | 236 ++++++++++++++++++ .../gdb.threads/infcall-from-bp-cond-single.c | 139 +++++++++++ .../infcall-from-bp-cond-single.exp | 119 +++++++++ 11 files changed, 940 insertions(+), 12 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.c create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.exp create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.exp create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.exp diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 606b2d191f4..4adc8c4306a 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5408,6 +5408,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) { try { + scoped_restore reset_in_cond_eval + = make_scoped_restore (&thread->control.in_cond_eval, true); condition_result = breakpoint_cond_eval (cond); } catch (const gdb_exception &ex) diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index 1a33eb61221..c88b57ebeec 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -171,6 +171,9 @@ struct thread_control_state command. This is used to decide whether "set scheduler-locking step" behaves like "on" or "off". */ int stepping_command = 0; + + /* True if the thread is evaluating a BP condition. */ + bool in_cond_eval = false; }; /* Inferior thread specific part of `struct infcall_suspend_state'. */ diff --git a/gdb/infcall.c b/gdb/infcall.c index 4957eee0b85..99ca5792dd3 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -647,6 +647,12 @@ run_inferior_call (std::unique_ptr sm, proceed (real_pc, GDB_SIGNAL_0); + /* Enable commit resume, but pass true for the force flag. This + ensures any thread we set running in proceed will actually be + committed to the target, even if some other thread in the current + target has a pending event. */ + scoped_enable_commit_resumed enable ("infcall", true); + infrun_debug_show_threads ("non-exited threads after proceed for inferior-call", all_non_exited_threads ()); diff --git a/gdb/infrun.c b/gdb/infrun.c index b23c3e3918e..f0fe860c68c 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2176,6 +2176,14 @@ user_visible_resume_ptid (int step) mode. */ resume_ptid = inferior_ptid; } + else if (inferior_ptid != null_ptid + && inferior_thread ()->control.in_cond_eval) + { + /* The inferior thread is evaluating a BP condition. Other threads + might be stopped or running and we do not want to change their + state, thus, resume only the current thread. */ + resume_ptid = inferior_ptid; + } else if (!sched_multi && target_supports_multi_process ()) { /* Resume all threads of the current process (and none of other @@ -2888,7 +2896,7 @@ schedlock_applies (struct thread_info *tp) pending events. */ static void -maybe_set_commit_resumed_all_targets () +maybe_set_commit_resumed_all_targets (bool force_p) { scoped_restore_current_thread restore_thread; @@ -2917,7 +2925,7 @@ maybe_set_commit_resumed_all_targets () status to report, handle it before requiring the target to commit its resumed threads: handling the status might lead to resuming more threads. */ - if (proc_target->has_resumed_with_pending_wait_status ()) + if (!force_p && proc_target->has_resumed_with_pending_wait_status ()) { infrun_debug_printf ("not requesting commit-resumed for target %s, a" " thread has a pending waitstatus", @@ -2927,7 +2935,7 @@ maybe_set_commit_resumed_all_targets () switch_to_inferior_no_thread (inf); - if (target_has_pending_events ()) + if (!force_p && target_has_pending_events ()) { infrun_debug_printf ("not requesting commit-resumed for target %s, " "target has pending events", @@ -3020,7 +3028,7 @@ scoped_disable_commit_resumed::reset () { /* This is the outermost instance, re-enable COMMIT_RESUMED_STATE on the targets where it's possible. */ - maybe_set_commit_resumed_all_targets (); + maybe_set_commit_resumed_all_targets (false); } else { @@ -3053,7 +3061,7 @@ scoped_disable_commit_resumed::reset_and_commit () /* See infrun.h. */ scoped_enable_commit_resumed::scoped_enable_commit_resumed - (const char *reason) + (const char *reason, bool force_p) : m_reason (reason), m_prev_enable_commit_resumed (enable_commit_resumed) { @@ -3065,7 +3073,7 @@ scoped_enable_commit_resumed::scoped_enable_commit_resumed /* Re-enable COMMIT_RESUMED_STATE on the targets where it's possible. */ - maybe_set_commit_resumed_all_targets (); + maybe_set_commit_resumed_all_targets (force_p); maybe_call_commit_resumed_all_targets (); } @@ -3781,10 +3789,11 @@ do_target_wait (ptid_t wait_ptid, execution_control_state *ecs, polling the rest of the inferior list starting from that one in a circular fashion until the whole list is polled once. */ - auto inferior_matches = [&wait_ptid] (inferior *inf) + ptid_t wait_ptid_pid {wait_ptid.pid ()}; + auto inferior_matches = [&wait_ptid_pid] (inferior *inf) { return (inf->process_target () != NULL - && ptid_t (inf->pid).matches (wait_ptid)); + && ptid_t (inf->pid).matches (wait_ptid_pid)); }; /* First see how many matching inferiors we have. */ @@ -4202,7 +4211,17 @@ fetch_inferior_event () the event. */ scoped_disable_commit_resumed disable_commit_resumed ("handling event"); - if (!do_target_wait (minus_one_ptid, ecs, TARGET_WNOHANG)) + /* Is the current thread performing an inferior function call as part + of a breakpoint condition evaluation? */ + bool in_cond_eval = (inferior_ptid != null_ptid + && inferior_thread ()->control.in_cond_eval); + + /* If the thread is in the middle of the condition evaluation, wait for + an event from the current thread, otherwise, wait for an event from + any thread. */ + ptid_t waiton_ptid = in_cond_eval ? inferior_ptid : minus_one_ptid; + + if (!do_target_wait (waiton_ptid, ecs, TARGET_WNOHANG)) { infrun_debug_printf ("do_target_wait returned no event"); disable_commit_resumed.reset_and_commit (); @@ -4254,7 +4273,12 @@ fetch_inferior_event () bool should_notify_stop = true; int proceeded = 0; - stop_all_threads_if_all_stop_mode (); + /* If the thread that stopped just completed an inferior + function call as part of a condition evaluation, then we + don't want to stop all the other threads. */ + if (ecs->event_thread == nullptr + || !ecs->event_thread->control.in_cond_eval) + stop_all_threads_if_all_stop_mode (); clean_up_just_stopped_threads_fsms (ecs); @@ -4269,7 +4293,7 @@ fetch_inferior_event () proceeded = normal_stop (); } - if (!proceeded) + if (!proceeded && !in_cond_eval) { inferior_event_handler (INF_EXEC_COMPLETE); cmd_done = 1; diff --git a/gdb/infrun.h b/gdb/infrun.h index 11a18583e68..0d1b95d6c2d 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -395,7 +395,7 @@ extern void maybe_call_commit_resumed_all_targets (); struct scoped_enable_commit_resumed { - explicit scoped_enable_commit_resumed (const char *reason); + explicit scoped_enable_commit_resumed (const char *reason, bool force_p = false); ~scoped_enable_commit_resumed (); DISABLE_COPY_AND_ASSIGN (scoped_enable_commit_resumed); diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.c b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.c new file mode 100644 index 00000000000..a6abdeb29a6 --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.c @@ -0,0 +1,135 @@ +/* Copyright 2022 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 . */ + +#include +#include +#include +#include + +#define NUM_THREADS 2 + +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; + +/* Some global variables to poke, just for something to do. */ +volatile int global_var_0 = 0; +volatile int global_var_1 = 0; + +/* This flag is updated from GDB. */ +volatile int raise_signal = 0; + +/* Implement the breakpoint condition function. Release the other thread + and try to give the other thread a chance to run. Then return ANSWER. */ +int +condition_core_func (int answer) +{ + /* This unlock should release the other thread. */ + if (pthread_mutex_unlock (&mutex) != 0) + abort (); + + /* And this yield and sleep should (hopefully) give the other thread a + chance to run. This isn't guaranteed of course, but once the other + thread does run it should hit a breakpoint, which GDB should + (temporarily) ignore, so there's no easy way for us to know the other + thread has done what it needs to, thus, yielding and sleeping is the + best we can do. */ + sched_yield (); + sleep (2); + + return answer; +} + +void +stop_marker () +{ + int a = 100; /* Final breakpoint here. */ +} + +/* A breakpoint condition function that always returns true. */ +int +condition_true_func () +{ + return condition_core_func (1); +} + +/* A breakpoint condition function that always returns false. */ +int +condition_false_func () +{ + return condition_core_func (0); +} + +void * +worker_func (void *arg) +{ + volatile int *ptr = 0; + int tid = *((int *) arg); + + switch (tid) + { + case 0: + global_var_0 = 11; /* First thread breakpoint. */ + break; + + case 1: + if (pthread_mutex_lock (&mutex) != 0) + abort (); + if (raise_signal) + global_var_1 = *ptr; /* Signal here. */ + else + global_var_1 = 99; /* Other thread breakpoint. */ + break; + + default: + abort (); + } + + return NULL; +} + +int +main () +{ + pthread_t threads[NUM_THREADS]; + int args[NUM_THREADS]; + + /* Set an alarm, just in case the test deadlocks. */ + alarm (300); + + /* We want the mutex to start locked. */ + if (pthread_mutex_lock (&mutex) != 0) + abort (); + + for (int i = 0; i < NUM_THREADS; i++) + { + args[i] = i; + pthread_create (&threads[i], NULL, worker_func, &args[i]); + } + + for (int i = 0; i < NUM_THREADS; i++) + { + void *retval; + pthread_join (threads[i], &retval); + } + + /* Unlock once we're done, just for cleanliness. */ + if (pthread_mutex_unlock (&mutex) != 0) + abort (); + + stop_marker (); + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.exp b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.exp new file mode 100644 index 00000000000..0c8ef728009 --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-other-thread-event.exp @@ -0,0 +1,175 @@ +# Copyright 2022 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 for conditional breakpoints where the breakpoint condition includes +# an inferior function call. +# +# The tests in this script are testing what happens when an event arrives in +# another thread while GDB is waiting for the inferior function call (in the +# breakpoint condition) to finish. +# +# The expectation is that GDB will queue events for other threads and wait +# for the inferior function call to complete, if the condition is true, then +# the conditional breakpoint should be reported first. The other thread +# event should of course, not get lost, and should be reported as soon as +# the user tries to continue the inferior. +# +# If the conditional breakpoint ends up not being taken (the condition is +# false), then the other thread event should be reported immediately. +# +# This script tests what happens when the other thread event is (a) the +# other thread hitting a breakpoint, and (b) the other thread taking a +# signal (SIGSEGV in this case). + +standard_testfile + +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ + {debug pthreads}] == -1 } { + return +} + +set cond_bp_line [gdb_get_line_number "First thread breakpoint"] +set other_bp_line [gdb_get_line_number "Other thread breakpoint"] +set final_bp_line [gdb_get_line_number "Final breakpoint here"] +set signal_line [gdb_get_line_number "Signal here"] + +# Start GDB based on TARGET_ASYNC and TARGET_NON_STOP, and then runto main. +proc start_gdb_and_runto_main { target_async target_non_stop } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS \ + " -ex \"maint set target-non-stop $target_non_stop\"" + append ::GDBFLAGS \ + " -ex \"maintenance set target-async ${target_async}\"" + + clean_restart ${::binfile} + } + + if {![runto_main]} { + fail "run to main" + return -1 + } + + return 0 +} + +# Run a test of GDB's conditional breakpoints, where the conditions include +# inferior function calls. While the inferior function call is executing +# another thread will hit a breakpoint (when OTHER_THREAD_SIGNAL is false), +# or receive a signal (when OTHER_THREAD_SIGNAL is true). GDB should report +# the conditional breakpoint first (if the condition is true), and then +# report the second thread event once the inferior is continued again. +# +# When STOP_AT_COND is true then the conditional breakpoint will have a +# condition that evaluates to true (and the GDB will stop at the +# breakpoint), otherwise, the condition will evaluate to false (and GDB will +# not stop at the breakpoint). +proc run_condition_test { stop_at_cond other_thread_signal \ + target_async target_non_stop } { + if { [start_gdb_and_runto_main $target_async \ + $target_non_stop] == -1 } { + return + } + + # Setup the conditional breakpoint. + if { $stop_at_cond } { + set cond_func "condition_true_func" + } else { + set cond_func "condition_false_func" + } + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if (${cond_func} ())" + set cond_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for conditional breakpoint"] + + if { $other_thread_signal } { + # Arrange for the other thread to raise a signal while GDB is + # evaluating the breakpoint condition. + gdb_test_no_output "set raise_signal = 1" + } else { + # And a breakpoint that will be hit by another thread only once the + # breakpoint condition starts to be evaluated. + gdb_breakpoint "${::srcfile}:${::other_bp_line}" + set other_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for other breakpoint"] + } + + # A final breakpoint once the test has completed. + gdb_breakpoint "${::srcfile}:${::final_bp_line}" + set final_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for final breakpoint"] + + if { $stop_at_cond } { + # Continue. The first breakpoint we hit should be the conditional + # breakpoint. The other thread will have hit its breakpoint, but + # that will have been deferred until the conditional breakpoint is + # reported. + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${cond_bp_num}, worker_func \[^\r\n\]+:${::cond_bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+First thread breakpoint\[^\r\n\]+"] \ + "hit the conditional breakpoint" + } + + if { $other_thread_signal } { + # Now continue again, the other thread will now report that it + # received a signal. + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" received signal SIGSEGV, Segmentation fault\\." \ + "\\\[Switching to Thread \[^\r\n\]+\\\]" \ + "${::hex} in worker_func \[^\r\n\]+:${::signal_line}" \ + "${::decimal}\\s+\[^\r\n\]+Signal here\[^\r\n\]+"] \ + "received signal in other thread" + } else { + # Now continue again, the other thread will now report its + # breakpoint. + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${other_bp_num}, worker_func \[^\r\n\]+:${::other_bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+Other thread breakpoint\[^\r\n\]+"] \ + "hit the breakpoint in other thread" + + # Run to the stop marker. + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${final_bp_num}, stop_marker \[^\r\n\]+:${::final_bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+Final breakpoint here\[^\r\n\]+"] \ + "hit the final breakpoint" + } + + gdb_exit +} + +foreach_with_prefix target_async { "on" "off" } { + foreach_with_prefix target_non_stop { "on" "off" } { + foreach_with_prefix other_thread_signal { true false } { + foreach_with_prefix stop_at_cond { true false } { + run_condition_test $stop_at_cond $other_thread_signal \ + $target_async $target_non_stop + } + } + } +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c new file mode 100644 index 00000000000..f2a24a52b01 --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.c @@ -0,0 +1,89 @@ +/* Copyright 2022 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 . */ + +#include +#include + +#define NUM_THREADS 3 + +int +is_matching_tid (int *tid_ptr, int tid_value) +{ + return *tid_ptr == tid_value; +} + +int +return_true () +{ + return 1; +} + +int +return_false () +{ + return 0; +} + +int +function_that_segfaults () +{ + int *p = 0; + *p = 1; /* Segfault happens here. */ +} + +int +function_with_breakpoint () +{ + return 1; /* Nested breakpoint. */ +} + +void * +worker_func (void *arg) +{ + int a = 42; /* Breakpoint here. */ +} + +void +stop_marker () +{ + int b = 99; /* Stop marker. */ +} + +int +main () +{ + pthread_t threads[NUM_THREADS]; + int args[NUM_THREADS]; + + alarm (300); + + for (int i = 0; i < NUM_THREADS; i++) + { + args[i] = i; + pthread_create (&threads[i], NULL, worker_func, &args[i]); + } + + for (int i = 0; i < NUM_THREADS; i++) + { + void *retval; + pthread_join (threads[i], &retval); + } + + stop_marker (); + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.exp b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.exp new file mode 100644 index 00000000000..8d94d24f9b3 --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-simple.exp @@ -0,0 +1,236 @@ +# Copyright 2022 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 . + +# Some simple tests of inferior function calls from breakpoint +# conditions, in multi-threaded inferiors. +# +# This test sets up a multi-threaded inferior, and places a breakpoint +# at a location that many of the threads will reach. We repeat the +# test with different conditions, sometimes a single thread should +# stop at the breakpoint, sometimes multiple threads should stop, and +# sometime no threads should stop. + +standard_testfile + +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ + {debug pthreads}] == -1 } { + return +} + +set cond_bp_line [gdb_get_line_number "Breakpoint here"] +set stop_bp_line [gdb_get_line_number "Stop marker"] +set nested_bp_line [gdb_get_line_number "Nested breakpoint"] +set segv_line [gdb_get_line_number "Segfault happens here"] + +# Start GDB based on TARGET_ASYNC and TARGET_NON_STOP, and then runto main. +proc start_gdb_and_runto_main { target_async target_non_stop } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS \ + " -ex \"maint set target-non-stop $target_non_stop\"" + append ::GDBFLAGS \ + " -ex \"maintenance set target-async ${target_async}\"" + + clean_restart ${::binfile} + } + + if {![runto_main]} { + fail "run to main" + return -1 + } + + return 0 +} + +# Run a test of GDB's conditional breakpoints, where the conditions include +# inferior function calls. +# +# CONDITION is combined (with &&) to some additional logic, and used as the +# breakpoint condition. +# +# N_EXPECTED_HITS is the number of threads that we expect to stop due to +# CONDITON. +# +# MESSAGE is used as a test name prefix. +proc run_condition_test { message n_expected_hits condition \ + target_async target_non_stop } { + with_test_prefix $message { + + if { [start_gdb_and_runto_main $target_async \ + $target_non_stop] == -1 } { + return + } + + # Use this convenience variable to track how often the + # breakpoint condition has been evaluated, this should be once + # per thread. + gdb_test "set \$n_cond_eval = 0" + + # Setup the conditional breakpoint. + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if ((++\$n_cond_eval) ${condition})" + + # And a breakpoint that we hit when the test is over, this one is + # not conditional. Only the main thread gets here once all the + # other threads have finished. + gdb_breakpoint "${::srcfile}:${::stop_bp_line}" + + # The number of times we stop at the conditional breakpoint. + set n_hit_condition 0 + + # Now keep 'continue'-ing GDB until all the threads have finished + # and we reach the stop_marker breakpoint. + gdb_test_multiple "continue" "spot all breakpoint hits" { + -re " worker_func \[^\r\n\]+${::srcfile}:${::cond_bp_line}\r\n${::decimal}\\s+\[^\r\n\]+Breakpoint here\[^\r\n\]+\r\n${::gdb_prompt} $" { + incr n_hit_condition + send_gdb "continue\n" + exp_continue + } + + -re " stop_marker \[^\r\n\]+${::srcfile}:${::stop_bp_line}\r\n${::decimal}\\s+\[^\r\n\]+Stop marker\[^\r\n\]+\r\n${::gdb_prompt} $" { + pass $gdb_test_name + } + } + + gdb_assert { $n_hit_condition == $n_expected_hits } \ + "stopped at breakpoint the expected number of times" + + # Ensure the breakpoint condition was evaluated once per thread. + gdb_test "print \$n_cond_eval" "= 3" "condition was evaluated twice" + } +} + +# Check that after handling a conditional breakpoint (where the condition +# includes an inferior call), it is still possible to kill the running +# inferior, and then restart the inferior. +# +# At once point doing this would result in GDB giving an assertion error. +proc_with_prefix run_kill_and_restart_test { target_async target_non_stop } { + # This test relies on the 'start' command, which is not possible with + # the plain 'remote' target. + if {[target_info gdb_protocol] == "remote"} { + return + } + + if { [start_gdb_and_runto_main $target_async \ + $target_non_stop] == -1 } { + return + } + + # Setup the conditional breakpoint. + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if (is_matching_tid (arg, 1))" + gdb_continue_to_breakpoint "worker_func" + + # Now kill the program being debugged. + gdb_test "kill" "" "kill process" \ + "Kill the program being debugged.*y or n. $" "y" + + # Check we can restart the inferior. At one point this would trigger an + # assertion. + gdb_test "start" ".*" +} + +# Create a conditional breakpoint which includes a call to a function that +# segfaults. Run GDB and check what happens when the inferior segfaults +# during the inferior call. +proc_with_prefix run_bp_cond_segfaults { target_async target_non_stop } { + if { [start_gdb_and_runto_main $target_async \ + $target_non_stop] == -1 } { + return + } + + # This test relies on the inferior segfaulting when trying to + # access address zero. + if { [is_address_zero_readable] } { + return + } + + # Setup the conditional breakpoint, include a call to + # 'function_that_segfaults', which triggers the segfault. + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if (is_matching_tid (arg, 0) && function_that_segfaults ())" + set bp_1_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of conditional breakpoint"] + + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "Thread ${::decimal} \"infcall-from-bp\" received signal SIGSEGV, Segmentation fault\\." \ + "${::hex} in function_that_segfaults \\(\\) at \[^\r\n\]+:${::segv_line}" \ + "${::decimal}\\s+\[^\r\n\]+Segfault happens here\[^\r\n\]+" \ + "Error in testing condition for breakpoint ${bp_1_num}:" \ + "The program being debugged was signaled while in a function called from GDB\\." \ + "GDB remains in the frame where the signal was received\\." \ + "To change this behavior use \"set unwindonsignal on\"\\." \ + "Evaluation of the expression containing the function" \ + "\\(function_that_segfaults\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\."] +} + +# Create a conditional breakpoint which includes a call to a function that +# itself has a breakpoint set within it. Run GDB and check what happens +# when GDB hits the nested breakpoint. +proc_with_prefix run_bp_cond_hits_breakpoint { target_async target_non_stop } { + if { [start_gdb_and_runto_main $target_async \ + $target_non_stop] == -1 } { + return + } + + # Setup the conditional breakpoint, include a call to + # 'function_with_breakpoint' in which we will shortly place a + # breakpoint. + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if (is_matching_tid (arg, 0) && function_with_breakpoint ())" + set bp_1_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of conditional breakpoint"] + + gdb_breakpoint "${::srcfile}:${::nested_bp_line}" + set bp_2_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of nested breakpoint"] + + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "Thread ${::decimal} \"infcall-from-bp\" hit Breakpoint ${bp_2_num}, function_with_breakpoint \\(\\) at \[^\r\n\]+:${::nested_bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+Nested breakpoint\[^\r\n\]+" \ + "Error in testing condition for breakpoint ${bp_1_num}:" \ + "The program being debugged stopped while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(function_with_breakpoint\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\."] +} + +foreach_with_prefix target_async { "on" "off" } { + foreach_with_prefix target_non_stop { "on" "off" } { + run_condition_test "exactly one thread is hit" \ + 1 "&& is_matching_tid (arg, 1)" \ + $target_async $target_non_stop + run_condition_test "exactly two threads are hit" \ + 2 "&& (is_matching_tid (arg, 0) || is_matching_tid (arg, 2))" \ + $target_async $target_non_stop + run_condition_test "all three threads are hit" \ + 3 "|| return_true ()" \ + $target_async $target_non_stop + run_condition_test "no thread is hit" \ + 0 "&& return_false ()" \ + $target_async $target_non_stop + + run_kill_and_restart_test $target_async $target_non_stop + run_bp_cond_segfaults $target_async $target_non_stop + run_bp_cond_hits_breakpoint $target_async $target_non_stop + } +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c new file mode 100644 index 00000000000..c87f1f2bfc6 --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.c @@ -0,0 +1,139 @@ +/* Copyright 2022 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 . */ + +#include +#include +#include +#include + +#define NUM_THREADS 5 + +/* Semaphores, used to track when threads have started, and to control + when the threads finish. */ +sem_t startup_semaphore; +sem_t finish_semaphore; + +/* Mutex to control when the first worker thread hit a breakpoint + location. */ +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; + +/* Global variable to poke, just so threads have something to do. */ +volatile int global_var = 0; + +int +return_true () +{ + return 1; +} + +int +return_false () +{ + return 0; +} + +void * +worker_func (void *arg) +{ + int tid = *((int *) arg); + + switch (tid) + { + case 0: + /* Wait for MUTEX to become available, then pass through the + conditional breakpoint location. */ + if (pthread_mutex_lock (&mutex) != 0) + abort (); + global_var = 99; /* Conditional breakpoint here. */ + if (pthread_mutex_unlock (&mutex) != 0) + abort (); + break; + + default: + /* Notify the main thread that the thread has started, then wait for + the main thread to tell us to finish. */ + sem_post (&startup_semaphore); + if (sem_wait (&finish_semaphore) != 0) + abort (); + break; + } +} + +void +stop_marker () +{ + global_var = 99; /* Stop marker. */ +} + +int +main () +{ + pthread_t threads[NUM_THREADS]; + int args[NUM_THREADS]; + void *retval; + + /* An alarm, just in case the thread deadlocks. */ + alarm (300); + + /* Semaphore initialization. */ + if (sem_init (&startup_semaphore, 0, 0) != 0) + abort (); + if (sem_init (&finish_semaphore, 0, 0) != 0) + abort (); + + /* Lock MUTEX, this prevents the first worker thread from rushing ahead. */ + if (pthread_mutex_lock (&mutex) != 0) + abort (); + + /* Worker thread creation. */ + for (int i = 0; i < NUM_THREADS; i++) + { + args[i] = i; + pthread_create (&threads[i], NULL, worker_func, &args[i]); + } + + /* Wait for every thread (other than the first) to tell us it has started + up. */ + for (int i = 1; i < NUM_THREADS; i++) + { + if (sem_wait (&startup_semaphore) != 0) + abort (); + } + + /* Unlock the first thread so it can proceed. */ + if (pthread_mutex_unlock (&mutex) != 0) + abort (); + + /* Wait for the first thread only. */ + pthread_join (threads[0], &retval); + + /* Now post FINISH_SEMAPHORE to allow all the other threads to finish. */ + for (int i = 1; i < NUM_THREADS; i++) + sem_post (&finish_semaphore); + + /* Now wait for the remaining threads to complete. */ + for (int i = 1; i < NUM_THREADS; i++) + pthread_join (threads[i], &retval); + + /* Semaphore cleanup. */ + sem_destroy (&finish_semaphore); + sem_destroy (&startup_semaphore); + + stop_marker (); + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.exp b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.exp new file mode 100644 index 00000000000..aa1e94190cd --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-single.exp @@ -0,0 +1,119 @@ +# Copyright 2022 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 reprocuces bug gdb/28942, performing an inferior function +# call from a breakpoint condition in a multi-threaded inferior. +# +# The important part of this test is that, when the conditional +# breakpoint is hit, and the condition (which includes an inferior +# function call) is evaluated, the other threads are running. + +standard_testfile + +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ + {debug pthreads}] == -1 } { + return +} + +set cond_bp_line [gdb_get_line_number "Conditional breakpoint here"] +set final_bp_line [gdb_get_line_number "Stop marker"] + +# Start GDB based on TARGET_ASYNC and TARGET_NON_STOP, and then runto main. +proc start_gdb_and_runto_main { target_async target_non_stop } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS \ + " -ex \"maint set target-non-stop $target_non_stop\"" + append ::GDBFLAGS \ + " -ex \"maintenance set target-async ${target_async}\"" + + clean_restart ${::binfile} + } + + if {![runto_main]} { + fail "run to main" + return -1 + } + + return 0 +} + +# Run a test of GDB's conditional breakpoints, where the conditions include +# inferior function calls. +# +# CONDITION is combined (with &&) to some additional logic, and used as the +# breakpoint condition. +# +# N_EXPECTED_HITS is the number of threads that we expect to stop due to +# CONDITON. +# +# MESSAGE is used as a test name prefix. +proc run_condition_test { stop_at_cond \ + target_async target_non_stop } { + if { [start_gdb_and_runto_main $target_async \ + $target_non_stop] == -1 } { + return + } + + # Setup the conditional breakpoint. + if { $stop_at_cond } { + set cond_func "return_true" + } else { + set cond_func "return_false" + } + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if (${cond_func} ())" + set cond_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for conditional breakpoint"] + + # And a breakpoint that we hit when the test is over, this one is + # not conditional. + gdb_breakpoint "${::srcfile}:${::final_bp_line}" + set final_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for final breakpoint"] + + if { $stop_at_cond } { + # Continue. The first breakpoint we hit should be the conditional + # breakpoint. The other thread will have hit its breakpoint, but + # that will have been deferred until the conditional breakpoint is + # reported. + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${cond_bp_num}, worker_func \[^\r\n\]+:${::cond_bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+Conditional breakpoint here\[^\r\n\]+"] \ + "hit the conditional breakpoint" + } + + # Run to the stop marker. + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${final_bp_num}, stop_marker \[^\r\n\]+:${::final_bp_line}" \ + "${::decimal}\\s+\[^\r\n\]+Stop marker\[^\r\n\]+"] \ + "hit the final breakpoint" +} + +foreach_with_prefix target_async { "on" "off" } { + foreach_with_prefix target_non_stop { "on" "off" } { + foreach_with_prefix stop_at_cond { true false } { + run_condition_test $stop_at_cond \ + $target_async $target_non_stop + } + } +} From patchwork Fri Oct 21 08:43:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59270 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 F02E53851C2C for ; Fri, 21 Oct 2022 08:46:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F02E53851C2C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341999; bh=cyE4Puu4yx29pbLNGtIYFUM5bJ0Hy8lg8Zo/XLAQrZc=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=VvWC8rlU2mUSWzj/qTMQbhB5VP6EIBqj+RUcDL/XvIqarWv+SkfJQCQKfcCLvAg2U W6ghZxPRzEeqHlN5HsEkbx0VYqT9Zgm0VwbMHnQsAJLzJHildL/wxOPTqhLh9qMi7K 770YX33D8KNVHRkuyHq6qA5068PlqljB/ZxKNsZU= 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 ED9D6385417B for ; Fri, 21 Oct 2022 08:44:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ED9D6385417B Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-119-kJEwuR6yNrCY1PYbA2vPPw-1; Fri, 21 Oct 2022 04:44:08 -0400 X-MC-Unique: kJEwuR6yNrCY1PYbA2vPPw-1 Received: by mail-wm1-f71.google.com with SMTP id ay12-20020a05600c1e0c00b003c6e18d9aa8so3019363wmb.8 for ; Fri, 21 Oct 2022 01:44:08 -0700 (PDT) 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=cyE4Puu4yx29pbLNGtIYFUM5bJ0Hy8lg8Zo/XLAQrZc=; b=QDc4TnKT+2NulhKMka7EETfDo7V83Kp2XM5Y+sXGcSW8zCVEYjcjhCstbjsbnvLE1p f+Q/KW83L6iHCxjPO8MaPQFpKLCSBb/ZvqUhEkja9PUNmDzCnu5cPVIAYRh26qt3ivMS n1FtEs4LiPYOPS7zdIpzHmzEb9n55YoIFprmuGAAjHGBgkzjvm1CCdAZV4Ovp9orytCS NJIwhngWyrvdlfIAUTvxp6/m1gjjDa48xcPMTVBfnrNofj62XkdbEQ4+kxo4iZ81MymY SA7dhEWjH/ebRztzUzETah7ypkMUjDqbnGfnZAgFaw/xWCMubI/6Y1//Jgi6h2aWdJUR K/3g== X-Gm-Message-State: ACrzQf2yfBfmcUPiMkPSibPXF98GQdAFd7OaFeVW+kBULeSih0H0UtcL ZnY9NJxG5Pgq6nTCp9gblW3BMF9ZwOq714Hv2evgJHDc5qOCbblP2JhpwWUrt40BYm1EcNz8RN5 sPTX7qToVZgmDwTjdWCPsrsQsJEw9JViOr1IKLeUVUEsUCJoqWvE2DdZefgCQyygTnEbDmFXWDg == X-Received: by 2002:a5d:59a8:0:b0:22e:d6ff:3a7c with SMTP id p8-20020a5d59a8000000b0022ed6ff3a7cmr10883803wrr.128.1666341846311; Fri, 21 Oct 2022 01:44:06 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7MQ7NvObkntV0lCVD5OEgsqlKo1eebwL0HM+5uTSjbXyu0LZORwnAfKPKjpJ0xvHoPVNXtUQ== X-Received: by 2002:a5d:59a8:0:b0:22e:d6ff:3a7c with SMTP id p8-20020a5d59a8000000b0022ed6ff3a7cmr10883771wrr.128.1666341845491; Fri, 21 Oct 2022 01:44:05 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id g6-20020a05600c310600b003a6a3595edasm1955277wmo.27.2022.10.21.01.44.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:44:05 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 11/12] gdb: add timeouts for inferior function calls Date: Fri, 21 Oct 2022 09:43:47 +0100 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.9 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" In the previous commits I have been working on improving inferior function call support. One thing that worries me about using inferior function calls from a conditional breakpoint is: what happens if the inferior function call fails? If the failure is obvious, e.g. the thread performing the call crashes, or hits a breakpoint, then this case is already well handled, and the error is reported to the user. But what if the thread performing the inferior call just deadlocks? If the user made the call from a 'print' or 'call' command, then the user might have some expectation of when the function call should complete, and, when this time limit is exceeded, the user will (hopefully) interrupt GDB and regain control of the debug session. But, when the inferior function call is from a breakpoint condition it is much harder to understand that GDB is deadlocked within an inferior call. Maybe the breakpoint hasn't been hit yet? Or maybe the condition was always false? Or maybe GDB is deadlocked in an inferior call? The only way to know for sure is to periodically interrupt GDB, check on all the threads, and then continue. Additionally, the focus of the previous commit was inferior function calls, from a conditional breakpoint, in a multi-threaded inferior. This opens up a whole new set of potential failure conditions. For example, what if the function called relies on interaction with some other thread, and the other thread crashes? Or hits a breakpoint? Given how inferior function calls work - in a synchronous manor, a stop event in some other thread is going to be ignored when the inferior function call is being done as part of a breakpoint condition, and this means that GDB could get stuck waiting for the original condition thread, which will now never complete. In this commit I propose a solution to this problem. A timeout. For targets that support async-mode we can install an event-loop timer before starting the inferior function call. When the timer expires we will stop the thread performing the inferior function call. With this mechanism in place a user can be sure that any inferior call they make will either complete, or timeout eventually. Adding a timer like this is obviously a change in behaviour for the more common 'call' and 'print' uses of inferior function calls, so, in this patch, I propose having two different timers. One I call the 'direct-call-timeout', which is used for 'call' and 'print' commands. This timeout is by default set to unlimited, which, not surprisingly, means there is no timeout in place. A second timer, which I've called 'indirect-call-timeout', is used for inferior function calls from breakpoint conditions. This timeout has a default value of 300 seconds. This is still a pretty substantial time to be waiting for a single inferior call to complete, but I didn't want to be too aggressive with the value I selected. A user can, of course, still use Ctrl-c to interrupt an inferior function call, but this limit will ensure that GDB will stop at some point. The new commands added by this commit are: set direct-call-timeout SECONDS show direct-call-timeout set indirect-call-timeout SECONDS show indirect-call-timeout These new timeouts do depend on async-mode, so, if async-mode is disabled (maint set target-async off), or not supported (e.g. target sim), then the timeout is treated as unlimited (that is, no timeout is set). For targets that "fake" non-async mode, e.g. Linux native, where non-async mode is really just async mode, but then we park the target in a sissuspend, we could easily fix things so that the timeouts still work, however, for targets that really are not async aware, like the simulator, fixing things so that timeouts work correctly would be a much bigger task - that effort would be better spent just making the target async-aware. And so, I'm happy for now that this feature will only work on async targets. The two new show commands will display slightly different text if the current target is a non-async target, which should allow users to understand what's going on. There's a somewhat random test adjustment needed in gdb.base/help.exp, the test uses a regexp with the apropos command, and expects to find a single result. Turns out the new settings I added also matched the regexp, which broke the test. I've updated the regexp a little to exclude my new settings. --- gdb/NEWS | 16 ++ gdb/doc/gdb.texinfo | 45 +++++ gdb/infcall.c | 159 ++++++++++++++++ gdb/testsuite/gdb.base/help.exp | 2 +- gdb/testsuite/gdb.base/infcall-timeout.c | 36 ++++ gdb/testsuite/gdb.base/infcall-timeout.exp | 81 +++++++++ .../infcall-from-bp-cond-timeout.c | 169 ++++++++++++++++++ .../infcall-from-bp-cond-timeout.exp | 128 +++++++++++++ 8 files changed, 635 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.c create mode 100644 gdb/testsuite/gdb.base/infcall-timeout.exp create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c create mode 100644 gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp diff --git a/gdb/NEWS b/gdb/NEWS index 8b519a648f7..596f38f5c2f 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -115,6 +115,22 @@ set debug infcall on|off show debug infcall Print additional debug messages about inferior function calls. +set direct-call-timeout SECONDS +show direct-call-timeout +set indirect-call-timeout SECONDS +show indirect-call-timeout + These new settings can be used to limit how long GDB will wait for + an inferior function call to complete. The direct timeout is used + for inferior function calls from e.g. 'call' and 'print' commands, + while the indirect timeout is used for inferior function calls from + within a conditional breakpoint expression. + + The default for the direct timeout is unlimited, while the default + for the indirect timeout is 300 seconds. + + These timeouts will only have an effect for targets that are + operating in async mode. + * Changed commands document user-defined diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index ea66f4ee42d..1724bad673e 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20630,6 +20630,51 @@ @end table +When calling a function within a program, it is possible that the +program could enter a state from which the called function may never +return. If this happens then @value{GDBN} will appear to hang. +Should this happen then it is possible to interrupt the running +program by typing the interrupt character (often @kbd{Ctrl-c}). + +On some targets @value{GDBN} can also place a timeout on any function +calls made into the program. If the timeout expires and the function +call is still going, then @value{GDBN} will interrupt the program +automatically. + +@table @code +@item set direct-call-timeout @var{seconds} +@kindex set direct-call-timeout +@cindex timeout for called functions +Set the timeout used when calling functions in the program to +@var{seconds}, which should be an integer greater than zero, or the +special value @code{unlimited}, which indicates no timeout should be +used. The default for this setting is @code{unlimited}. + +This setting only works for targets that support asynchronous +execution (@pxref{Background Execution}), for any other target the +setting is treated as @code{unlimited}. +@end table + +It is also possible to call functions within the program from the +condition of a conditional breakpoint (@pxref{Conditions, ,Break +Conditions}). A different setting controls the timeout used for +functions made from a breakpoint condition. + +@table @code +@item set indirect-call-timeout @var{seconds} +@kindex set indirect-call-timeout +@cindex timeout for called functions +Set the timeout used when calling functions in the program from a +breakpoint condition to @var{seconds}, which should be an integer +greater than zero, or the special value @code{unlimited}, which +indicates no timeout should be used. The default for this setting is +@code{300} seconds. + +This setting only works for targets that support asynchronous +execution (@pxref{Background Execution}), for any other target the +setting is treated as @code{unlimited}. +@end table + @subsection Calling functions with no debug info @cindex no debug info functions diff --git a/gdb/infcall.c b/gdb/infcall.c index 99ca5792dd3..4eca505250b 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -95,6 +95,53 @@ show_may_call_functions_p (struct ui_file *file, int from_tty, value); } +/* A timeout (in seconds) for direct inferior calls. A direct inferior + call is one the user triggers from the prompt, e.g. with a 'call' or + 'print' command. Compare with the definition of indirect calls below. */ + +static unsigned int direct_call_timeout = UINT_MAX; + +/* Implement 'show direct-call-timeout'. */ + +static void +show_direct_call_timeout (struct ui_file *file, int from_tty, + struct cmd_list_element *c, const char *value) +{ + if (target_has_execution () && !target_can_async_p ()) + gdb_printf (file, _("Current target does not support async mode, timeout " + "for direct inferior calls is \"unlimited\".\n")); + else if (direct_call_timeout == UINT_MAX) + gdb_printf (file, _("Timeout for direct inferior function calls " + "is \"unlimited\".\n")); + else + gdb_printf (file, _("Timeout for direct inferior function calls " + "is \"%s seconds\".\n"), value); +} + +/* A timeout (in seconds) for indirect inferior calls. An indirect inferior + call is one that originates from within GDB, for example, when + evaluating an expression for a conditional breakpoint. Compare with + the definition of direct calls above. */ + +static unsigned int indirect_call_timeout = 300; + +/* Implement 'show indirect-call-timeout'. */ + +static void +show_indirect_call_timeout (struct ui_file *file, int from_tty, + struct cmd_list_element *c, const char *value) +{ + if (target_has_execution () && !target_can_async_p ()) + gdb_printf (file, _("Current target does not support async mode, timeout " + "for indirect inferior calls is \"unlimited\".\n")); + else if (indirect_call_timeout == UINT_MAX) + gdb_printf (file, _("Timeout for indirect inferior function calls " + "is \"unlimited\".\n")); + else + gdb_printf (file, _("Timeout for indirect inferior function calls " + "is \"%s seconds\".\n"), value); +} + /* How you should pass arguments to a function depends on whether it was defined in K&R style or prototype style. If you define a function using the K&R syntax that takes a `float' argument, then @@ -595,6 +642,85 @@ call_thread_fsm::should_notify_stop () return true; } +/* A class to control creation of a timer that will interrupt a thread + during an inferior call. */ +struct infcall_timer_controller +{ + /* Setup an event-loop timer that will interrupt PTID if the inferior + call takes too long. DIRECT_CALL_P is true when this inferior call is + a result of the user using a 'print' or 'call' command, and false when + this inferior call is a result of e.g. a conditional breakpoint + expression, this is used to select which timeout to use. */ + infcall_timer_controller (ptid_t ptid, bool direct_call_p) + : m_ptid (ptid) + { + unsigned int timeout + = direct_call_p ? direct_call_timeout : indirect_call_timeout; + if (timeout < UINT_MAX && target_can_async_p ()) + { + int ms = timeout * 1000; + int id = create_timer (ms, infcall_timer_controller::timed_out, this); + m_timer_id.emplace (id); + infcall_debug_printf ("Setting up infcall timeout timer for " + "ptid %s: %d milliseconds", + m_ptid.to_string ().c_str (), ms); + } + } + + /* Destructor. Ensure that the timer is removed from the event loop. */ + ~infcall_timer_controller () + { + /* If the timer has already triggered, then it will have already been + deleted from the event loop. If the timer has not triggered, then + delete it now. */ + if (m_timer_id.has_value () && !m_triggered) + delete_timer (*m_timer_id); + + /* Just for clarity, discard the timer id now. */ + m_timer_id.reset (); + } + + /* Return true if there was a timer in place, and the timer triggered, + otherwise, return false. */ + bool triggered_p () + { + gdb_assert (!m_triggered || m_timer_id.has_value ()); + return m_triggered; + } + +private: + /* The thread we should interrupt. */ + ptid_t m_ptid; + + /* Set true when the timer is triggered. */ + bool m_triggered = false; + + /* Given a value when a timer is in place. */ + gdb::optional m_timer_id; + + /* Callback for the timer, forwards to ::trigger below. */ + static void + timed_out (gdb_client_data context) + { + infcall_timer_controller *ctrl + = static_cast (context); + ctrl->trigger (); + } + + /* Called when the timer goes off. Stop thread m_ptid. */ + void + trigger () + { + m_triggered = true; + + scoped_disable_commit_resumed disable_commit_resumed ("infcall timeout"); + + infcall_debug_printf ("Stopping thread %s", + m_ptid.to_string ().c_str ()); + target_stop (m_ptid); + } +}; + /* Subroutine of call_function_by_hand to simplify it. Start up the inferior and wait for it to stop. Return the exception if there's an error, or an exception with @@ -656,10 +782,19 @@ run_inferior_call (std::unique_ptr sm, infrun_debug_show_threads ("non-exited threads after proceed for inferior-call", all_non_exited_threads ()); + /* Setup a timer (if possible, and if the settings allow) to prevent + the inferior call running forever. */ + bool direct_call_p = !call_thread->control.in_cond_eval; + infcall_timer_controller infcall_timer (inferior_ptid, direct_call_p); + /* Inferior function calls are always synchronous, even if the target supports asynchronous execution. */ wait_sync_command_done (); + /* If the timer triggered then the inferior call failed. */ + if (infcall_timer.triggered_p ()) + error (_("timeout waiting for inferior function to complete")); + infcall_debug_printf ("inferior call completed successfully"); } catch (gdb_exception &e) @@ -1649,6 +1784,30 @@ The default is to unwind the frame."), show_unwind_on_terminating_exception_p, &setlist, &showlist); + add_setshow_uinteger_cmd ("direct-call-timeout", no_class, + &direct_call_timeout, _("\ +Set the timeout, for direct calls to inferior function calls."), _("\ +Show the timeout, for direct calls to inferior function calls."), _("\ +If running on a target that supports, and is running in, async mode\n\ +then this timeout is used for any inferior function calls triggered\n\ +directly from the prompt, e.g. from a 'call' or 'print' command. The\n\ +timeout is specified in seconds."), + nullptr, + show_direct_call_timeout, + &setlist, &showlist); + + add_setshow_uinteger_cmd ("indirect-call-timeout", no_class, + &indirect_call_timeout, _("\ +Set the timeout, for indirect calls to inferior function calls."), _("\ +Show the timeout, for indirect calls to inferior function calls."), _("\ +If running on a target that supports, and is running in, async mode\n\ +then this timeout is used for any inferior function calls triggered\n\ +indirectly, e.g. when evaluating a conditional breakpoint expression.\n\ +The timeout is specified in seconds."), + nullptr, + show_indirect_call_timeout, + &setlist, &showlist); + add_setshow_boolean_cmd ("infcall", class_maintenance, &debug_infcall, _("Set inferior call debugging."), diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp index 5ee8ce0726d..54b0be32cf8 100644 --- a/gdb/testsuite/gdb.base/help.exp +++ b/gdb/testsuite/gdb.base/help.exp @@ -121,7 +121,7 @@ gdb_test "help info bogus-gdb-command" "Undefined info command: \"bogus-gdb-comm gdb_test "help gotcha" "Undefined command: \"gotcha\"\. Try \"help\"\." # Test apropos regex. -gdb_test "apropos \\\(print\[\^\[ bsiedf\\\".-\]\\\)" "handle -- Specify how to handle signals\." +gdb_test "apropos \\\(print\[\^\[ bsiedf\\\"'.-\]\\\)" "handle -- Specify how to handle signals\." # Test apropos >1 word string. gdb_test "apropos handle signal" "handle -- Specify how to handle signals\." # Test apropos apropos. diff --git a/gdb/testsuite/gdb.base/infcall-timeout.c b/gdb/testsuite/gdb.base/infcall-timeout.c new file mode 100644 index 00000000000..895e8a36d59 --- /dev/null +++ b/gdb/testsuite/gdb.base/infcall-timeout.c @@ -0,0 +1,36 @@ +/* Copyright 2022 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 . */ + +#include + +/* This function is called from GDB. */ +int +function_that_never_returns () +{ + while (1) + sleep (1); + + return 0; +} + +int +main () +{ + alarm (300); + + return 0; +} diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall-timeout.exp new file mode 100644 index 00000000000..2bca092aaf8 --- /dev/null +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp @@ -0,0 +1,81 @@ +# Copyright 2022 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 GDB's direct-call-timeout setting, that is, ensure that if an +# inferior function call, invoked from e.g. a 'print' command, takes +# too long, then GDB can interrupt it, and return control to the user. + +standard_testfile + +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ + {debug}] == -1 } { + return +} + +# Start GDB according to TARGET_ASYNC and TARGET_NON_STOP, then adjust +# the direct-call-timeout, and make an inferior function call that +# will never return. GDB should eventually timeout and stop the +# inferior. +proc_with_prefix run_test { target_async target_non_stop } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS \ + " -ex \"maint set target-non-stop $target_non_stop\"" + append ::GDBFLAGS \ + " -ex \"maintenance set target-async ${target_async}\"" + + clean_restart ${::binfile} + } + + if {![runto_main]} { + fail "run to main" + return + } + + gdb_test_no_output "set direct-call-timeout 5" + + # When non-stop mode is off we get slightly different output from GDB. + if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } { + set stopped_line_pattern "Program received signal SIGINT, Interrupt\\." + } else { + set stopped_line_pattern "Program stopped\\." + } + + gdb_test "print function_that_never_returns ()" \ + [multi_line \ + $stopped_line_pattern \ + ".*" \ + "timeout waiting for inferior function to complete" \ + "An error occurred while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(function_that_never_returns\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\."] + + gdb_test "bt" ".* function_that_never_returns .*.*" +} + +foreach_with_prefix target_async { "on" "off" } { + + if { $target_async == "off" } { + # GDB can't timeout while waiting for a thread if the target + # runs with async-mode turned off; once the target is running + # GDB is effectively blocked until the target stops for some + # reason. + continue + } + + foreach_with_prefix target_non_stop { "on" "off" } { + run_test $target_async $target_non_stop + } +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c new file mode 100644 index 00000000000..3bd91d7377d --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.c @@ -0,0 +1,169 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2022 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 . */ + +#include +#include +#include +#include +#include +#include + +#define NUM_THREADS 5 + +/* Semaphores, used to track when threads have started, and to control + when the threads finish. */ +sem_t startup_semaphore; +sem_t finish_semaphore; +sem_t thread_1_semaphore; +sem_t thread_2_semaphore; + +/* Mutex to control when the first worker thread hit a breakpoint + location. */ +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; + +/* Global variable to poke, just so threads have something to do. */ +volatile int global_var = 0; + +int +condition_func () +{ + /* Let thread 2 run. */ + if (sem_post (&thread_2_semaphore) != 0) + abort (); + + /* Wait for thread 2 to complete its actions. */ + if (sem_wait (&thread_1_semaphore) != 0) + abort (); + + return 1; +} + +void +do_segfault () +{ + volatile int *p = 0; + *p = 0; /* Segfault here. */ +} + +void * +worker_func (void *arg) +{ + int tid = *((int *) arg); + + /* Let the main thread know that this worker has started. */ + if (sem_post (&startup_semaphore) != 0) + abort (); + + switch (tid) + { + case 0: + /* Wait for MUTEX to become available, then pass through the + conditional breakpoint location. */ + if (pthread_mutex_lock (&mutex) != 0) + abort (); + global_var = 99; /* Conditional breakpoint here. */ + if (pthread_mutex_unlock (&mutex) != 0) + abort (); + break; + + case 1: + if (sem_wait (&thread_2_semaphore) != 0) + abort (); + do_segfault (); + if (sem_post (&thread_1_semaphore) != 0) + abort (); + + /* Fall through. */ + default: + /* Wait until we are allowed to finish. */ + if (sem_wait (&finish_semaphore) != 0) + abort (); + break; + } +} + +void +stop_marker () +{ + global_var = 99; /* Stop marker. */ +} + +/* The main program entry point. */ + +int +main () +{ + pthread_t threads[NUM_THREADS]; + int args[NUM_THREADS]; + void *retval; + + /* An alarm, just in case the thread deadlocks. */ + alarm (300); + + /* Semaphore initialization. */ + if (sem_init (&startup_semaphore, 0, 0) != 0) + abort (); + if (sem_init (&finish_semaphore, 0, 0) != 0) + abort (); + if (sem_init (&thread_1_semaphore, 0, 0) != 0) + abort (); + if (sem_init (&thread_2_semaphore, 0, 0) != 0) + abort (); + + /* Lock MUTEX, this prevents the first worker thread from rushing ahead. */ + if (pthread_mutex_lock (&mutex) != 0) + abort (); + + /* Worker thread creation. */ + for (int i = 0; i < NUM_THREADS; i++) + { + args[i] = i; + pthread_create (&threads[i], NULL, worker_func, &args[i]); + } + + /* Wait for every thread to start. */ + for (int i = 0; i < NUM_THREADS; i++) + { + if (sem_wait (&startup_semaphore) != 0) + abort (); + } + + /* Unlock the first thread so it can proceed. */ + if (pthread_mutex_unlock (&mutex) != 0) + abort (); + + /* Wait for the first thread only. */ + pthread_join (threads[0], &retval); + + /* Now post FINISH_SEMAPHORE to allow all the other threads to finish. */ + for (int i = 1; i < NUM_THREADS; i++) + sem_post (&finish_semaphore); + + /* Now wait for the remaining threads to complete. */ + for (int i = 1; i < NUM_THREADS; i++) + pthread_join (threads[i], &retval); + + /* Semaphore cleanup. */ + sem_destroy (&finish_semaphore); + sem_destroy (&startup_semaphore); + sem_destroy (&thread_1_semaphore); + sem_destroy (&thread_2_semaphore); + + stop_marker (); + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp new file mode 100644 index 00000000000..17beba75db0 --- /dev/null +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp @@ -0,0 +1,128 @@ +# Copyright 2020 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 . + +# Tests inferior calls executed from a breakpoint condition in +# a multi-threaded program. + +standard_testfile + +if { [build_executable "failed to prepare" ${binfile} "${srcfile}" \ + {debug pthreads}] } { + return +} + +set cond_bp_line [gdb_get_line_number "Conditional breakpoint here"] +set final_bp_line [gdb_get_line_number "Stop marker"] +set segfault_line [gdb_get_line_number "Segfault here"] + +proc run_test { other_thread_bp target_async target_non_stop } { + save_vars { ::GDBFLAGS } { + append ::GDBFLAGS " -ex \"maint set target-non-stop $target_non_stop\"" + append ::GDBFLAGS " -ex \"maintenance set target-async ${target_async}\"" + + clean_restart ${::binfile} + } + + if {![runto_main]} { + fail "run to main" + return + } + + # The default timeout for indirect inferior calls (e.g. inferior + # calls for conditional breakpoint expressions) is pretty high. + # We don't want the test to take too long, so reduce this. + # + # However, the test relies on a second thread hitting some event + # (either a breakpoint or signal) before this timeout expires. + # + # There is a chance that on a really slow system this might not + # happen, in which case the test might fail. + # + # However, we still allocate 5 seconds, which feels like it should + # be enough time in most cases. + gdb_test_no_output "set indirect-call-timeout 5" + + gdb_breakpoint \ + "${::srcfile}:${::cond_bp_line} if (condition_func ())" + set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for conditional breakpoint"] + + gdb_breakpoint "${::srcfile}:${::final_bp_line}" + set final_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for final breakpoint"] + + if { $other_thread_bp } { + gdb_breakpoint "${::srcfile}:${::segfault_line}" + set segfault_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number for segfault breakpoint"] + } + + # When non-stop mode is off we get slightly different output from GDB. + if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } { + set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" received signal SIGINT, Interrupt\\." + } else { + set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." + } + + gdb_test "continue" \ + [multi_line \ + $stopped_line_pattern \ + ".*" \ + "Error in testing condition for breakpoint ${bp_num}:" \ + "timeout waiting for inferior function to complete" \ + "An error occurred while in a function called from GDB\\." \ + "Evaluation of the expression containing the function" \ + "\\(condition_func\\) will be abandoned\\." \ + "When the function is done executing, GDB will silently stop\\."] \ + "expected timeout waiting for inferior call to complete" + + if { $other_thread_bp } { + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "" \ + "Thread ${::decimal} \"\[^\"\r\n\]+\" hit Breakpoint ${segfault_bp_num}, do_segfault \[^\r\n\]+:${::segfault_line}" \ + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] \ + "hit the segfault breakpoint" + } else { + gdb_test "continue" \ + [multi_line \ + "Continuing\\." \ + ".*" \ + "Thread ${::decimal} \"infcall-from-bp\" received signal SIGSEGV, Segmentation fault\\." \ + "\\\[Switching to Thread \[^\r\n\]+\\\]" \ + "${::hex} in do_segfault \\(\\) at \[^\r\n\]+:${::segfault_line}" \ + "${::decimal}\\s+\[^\r\n\]+Segfault here\[^\r\n\]+"] \ + "hit the segfault" + } +} + +foreach_with_prefix target_async {"on" "off" } { + + if { $target_async == "off" } { + # GDB can't timeout while waiting for a thread if the target + # runs with async-mode turned off; once the target is running + # GDB is effectively blocked until the target stops for some + # reason. + continue + } + + foreach_with_prefix target_non_stop {"off" "on"} { + foreach_with_prefix other_thread_bp { true false } { + run_test $other_thread_bp $target_async $target_non_stop + } + } +} From patchwork Fri Oct 21 08:43:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 59268 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 CDF65385383B for ; Fri, 21 Oct 2022 08:46:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CDF65385383B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1666341968; bh=y7dPvGCpD+W598dc6+zdAwz6IliS1cJiJ/REWurOutk=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=LbNnavW57/Tav+aYGbxaVsoi2RMtpv6xZFKoiV/MGHAy43A0HPu7ultSINi+T9ZgW EC/kzZqjKKZC3XqfpTOU7MbPv5bLWs2GTCDLcMzvvJpYK2P3rlHpZH2aXny/nBcYrF qhP1l9RKoaN8AM+2Eytr4iIvA6x/VA22ybsUr9ro= 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 A0F33385354F for ; Fri, 21 Oct 2022 08:44:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A0F33385354F Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-520-pLmSuEcbOBKO9oSW6lNDeQ-1; Fri, 21 Oct 2022 04:44:08 -0400 X-MC-Unique: pLmSuEcbOBKO9oSW6lNDeQ-1 Received: by mail-wm1-f72.google.com with SMTP id v125-20020a1cac83000000b003bd44dc5242so3020474wme.7 for ; Fri, 21 Oct 2022 01:44:08 -0700 (PDT) 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=y7dPvGCpD+W598dc6+zdAwz6IliS1cJiJ/REWurOutk=; b=CWeWMK6xP9PsMCkGTgjZVaxVqjnAkmazzviv0llUSYekR1J+Kgfgw8Vtb8i4FxYiIA eOe92MvrUwqXGrEPRl4kIVdIKCt9zF3k01wbjjwAz/mRs3/eI6N+RfNVqleOgrWDbqXk 5d9tsXCaCsao/h5BhwflZbDy8sFN7C8/wd7VjPf96l4/MjYceQRIzQ1PegA8UzE6o9jA 7TJIsg6S2998P6tEwa4kO9SLYDzJKL0NbiuCQufom1tpV7WigPaOPAwVE/oSMPaEaxYy OK2kuAL22VOYQI1HI+wOvhvqeaOFoyQBZ+VhZV8xwAgaq0A7NAAYHLD46Yi9j2Olb6tz 3rVw== X-Gm-Message-State: ACrzQf3QQji+DF16C1+o63307lV9tgcCkqYEwxVINsxc1WJDyrHvGnjR AKAJhRmb6oM+Z4xm6lBc5o0EyidRnA1MUfrBepP58JILai8LeR6p22grBrxzXqpZgZLbgF4fVLZ RkZ09CEfHuiSfTb3h/C/Y8c00fGGHgQyQCt5TFm/yWElONkXCpc2EVIuO0Rj710cef5drayoWyA == X-Received: by 2002:a5d:5b1b:0:b0:236:39af:f2a6 with SMTP id bx27-20020a5d5b1b000000b0023639aff2a6mr4446361wrb.549.1666341847252; Fri, 21 Oct 2022 01:44:07 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6Sglp2MfnaKIygNIPrkf8i0KrpZ8TiZRQtaxn2mxo2sfUDAAea114MBhaIi3vIGN+k5l6/uw== X-Received: by 2002:a5d:5b1b:0:b0:236:39af:f2a6 with SMTP id bx27-20020a5d5b1b000000b0023639aff2a6mr4446343wrb.549.1666341846871; Fri, 21 Oct 2022 01:44:06 -0700 (PDT) Received: from localhost ([31.111.84.238]) by smtp.gmail.com with ESMTPSA id v21-20020a05600c15d500b003b4e009deb2sm2002079wmf.41.2022.10.21.01.44.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Oct 2022 01:44:06 -0700 (PDT) To: gdb-patches@sourceware.org Subject: [PATCH 12/12] gdb/remote: avoid SIGINT after calling remote_target::stop Date: Fri, 21 Oct 2022 09:43:48 +0100 Message-Id: <2733503b33b89407c2b0ea67baa93dc604169db1.1666341010.git.aburgess@redhat.com> 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.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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" Currently, if the remote target is not running in non-stop mode, then, when GDB calls remote_target::stop, we end up sending an interrupt packet \x03 to the remote target. If the user interrupts the inferior from the GDB prompt (e.g. by typing Ctrl-c), then GDB calls remote_target::interrupt, which also ends up sending the interrupt packet. The problem here is that both of these mechanisms end up sending the interrupt packet, which means, when the target stops with a SIGINT, and this is reported back to GDB, we have no choice but to report this to the user as a SIGINT stop event. Now maybe this is the correct thing to do, after all the target has been stopped with SIGINT. However, this leads to an unfortunate change in behaviour. When running in non-stop mode, and remote_target::stop is called, the target will be stopped with a vCont packet, and this stop is then reported back to GDB as GDB_SIGNAL_0, this will cause GDB to print a message like: Program stopped. Or: Thread NN "binary name" stopped. In contrast, when non-stop mode is off, we get messages like: Program received SIGINT, Segmentation fault. Or: Thread NN "binary name" received SIGINT, Segmentation fault. In this commit I propose a mechanism where we can track that a stop has been requested for a particular thread through remote_target::stop, then, when the stop arrives, we can convert the SIGINT to a GDB_SIGNAL_0. With this done GDB will now display the "stopped" based messages rather than the "received SIGINT" messages. Two of the tests added in the previous commit exposed this issue. In the previous commit the tests looked for either of the above patterns. In this commit I've updated these tests to only look for the "stopped" based messages. --- gdb/remote.c | 17 +++++++++++++++++ gdb/testsuite/gdb.base/infcall-timeout.exp | 9 +-------- .../infcall-from-bp-cond-timeout.exp | 9 +-------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 17c2d17c8fe..141f7d3656f 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1139,6 +1139,10 @@ struct remote_thread_info : public private_thread_info std::string name; int core = -1; + /* Only used when not in non-stop mode. Set to true when a stop is + requested for the thread. */ + bool stop_requested = false; + /* Thread handle, perhaps a pthread_t or thread_t value, stored as a sequence of bytes. */ gdb::byte_vector thread_handle; @@ -7112,6 +7116,12 @@ remote_target::stop (ptid_t ptid) /* We don't currently have a way to transparently pause the remote target in all-stop mode. Interrupt it instead. */ remote_interrupt_as (); + + /* Record that this thread's stop is a result of GDB asking for the + stop, rather than the user asking for an interrupt. We can use + this information to adjust the waitstatus when it arrives. */ + remote_thread_info *remote_thr = get_remote_thread_info (this, ptid); + remote_thr->stop_requested = true; } } @@ -8095,9 +8105,16 @@ remote_target::process_stop_reply (struct stop_reply *stop_reply, /* If the target works in non-stop mode, a stop-reply indicates that only this thread stopped. */ remote_thr->set_not_resumed (); + gdb_assert (!remote_thr->stop_requested); } else { + if (status->kind () == TARGET_WAITKIND_STOPPED + && status->sig () == GDB_SIGNAL_INT + && remote_thr->stop_requested) + status->set_stopped (GDB_SIGNAL_0); + remote_thr->stop_requested = false; + /* If the target works in all-stop mode, a stop-reply indicates that all the target's threads stopped. */ for (thread_info *tp : all_non_exited_threads (this)) diff --git a/gdb/testsuite/gdb.base/infcall-timeout.exp b/gdb/testsuite/gdb.base/infcall-timeout.exp index 2bca092aaf8..64b294b2f98 100644 --- a/gdb/testsuite/gdb.base/infcall-timeout.exp +++ b/gdb/testsuite/gdb.base/infcall-timeout.exp @@ -45,16 +45,9 @@ proc_with_prefix run_test { target_async target_non_stop } { gdb_test_no_output "set direct-call-timeout 5" - # When non-stop mode is off we get slightly different output from GDB. - if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } { - set stopped_line_pattern "Program received signal SIGINT, Interrupt\\." - } else { - set stopped_line_pattern "Program stopped\\." - } - gdb_test "print function_that_never_returns ()" \ [multi_line \ - $stopped_line_pattern \ + "Program stopped\\." \ ".*" \ "timeout waiting for inferior function to complete" \ "An error occurred while in a function called from GDB\\." \ diff --git a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp index 17beba75db0..bbeb49eb41e 100644 --- a/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp +++ b/gdb/testsuite/gdb.threads/infcall-from-bp-cond-timeout.exp @@ -69,16 +69,9 @@ proc run_test { other_thread_bp target_async target_non_stop } { "get number for segfault breakpoint"] } - # When non-stop mode is off we get slightly different output from GDB. - if { [gdb_is_remote_or_extended_remote_target] && $target_non_stop == "off" } { - set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" received signal SIGINT, Interrupt\\." - } else { - set stopped_line_pattern "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." - } - gdb_test "continue" \ [multi_line \ - $stopped_line_pattern \ + "Thread ${::decimal} \"\[^\r\n\"\]+\" stopped\\." \ ".*" \ "Error in testing condition for breakpoint ${bp_num}:" \ "timeout waiting for inferior function to complete" \