From patchwork Tue Dec 3 10:35:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 102297 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 BEAEB3858D26 for ; Tue, 3 Dec 2024 10:41:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BEAEB3858D26 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PvgF1lVQ 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 ESMTP id 451913858C53 for ; Tue, 3 Dec 2024 10:35:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 451913858C53 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 451913858C53 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222131; cv=none; b=LXXY6Tx0HvJiZ11YH4psu7o7MXlo2V3rvcGBYf4Yp+Qto+8b6tzNL9EmC48U2/vbsGRA/+VVGIR8RcWefOmSB7tKysGjcC/4536j76WQFWpXAG8FWyUV8ZZv0+U1YfUMZYxMkSOGJ+DppSL1eg26bEGXpdWIy6My8Pa/nUgr3xk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222131; c=relaxed/simple; bh=Obn+uFdcTClzefDjzkce1Khzl1/h64Yj0i0Fke5uSl8=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=U3ZJp4M+0TReK+GtqfJS7mmIX6Fs624ByUeuKK/i8jmTBEbuL+s8CsOPGozvraOlL9SiINPVrU/zYUwerpUksJb4G6OCrheW1aQGwI9yVhYe1MixXFaBWbMkkR+cFONow2x7r2O0A8HX+LO9RtFzcntbhUfmhmK5XQjXqFStgrw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 451913858C53 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733222131; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mgHf0ggKiSo/Tz0z9pCSon3nHQ4wiqUIvi+OLg4+sxQ=; b=PvgF1lVQF/Cu8gPte7edo1/bT5abr+tzRxSjzMK4rF5+X0FaBq+/AaqgiYIlCYKjIo/UWa ilcK4Dm91eXn2AmBfea37N1YFFVdIUJmdbJ55JtT1qAblMgjI1o31lOnd6qD0jdn9dZdKV egxDGxQrgQUubuP0oF89v/AZ2qwDGR4= 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_256_GCM_SHA384) id us-mta-652-1hEBqdK0O6eR3TJwV1xwlQ-1; Tue, 03 Dec 2024 05:35:29 -0500 X-MC-Unique: 1hEBqdK0O6eR3TJwV1xwlQ-1 X-Mimecast-MFC-AGG-ID: 1hEBqdK0O6eR3TJwV1xwlQ Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-434a104896cso44475655e9.1 for ; Tue, 03 Dec 2024 02:35:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733222128; x=1733826928; 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=mgHf0ggKiSo/Tz0z9pCSon3nHQ4wiqUIvi+OLg4+sxQ=; b=lgCuMYf7JXDG0juwb2A78MfM0yvIahRSvNV4TkQgw52z2448dlH1X67zq9c/qx62ek IvPeSxpN2TBAsGCLhPdE4lyedDpgdpSprMapH7x88kq5qZH/sW00LlUsSycmcR0qryv4 8vn0LInOCk8LrU2AcbLhCJrt//WyahopZlxBhHCmKJdW0Gb5pIbNNYuQRtTEIPdR0a89 WYtlWdTcc2UgzHkYniJ7E/pNs5aNx17OVQtLizk5V6wGtLWsPWhUXicBSy0QvSwTdNMl lZlhZr3MMINp+sSKEhD2N40jLSVeC85nmBSoJ2Q/+gfYH6mQBZPEfL+85j9P36Q6rASO TpIA== X-Gm-Message-State: AOJu0YzCb+Z63J/I2vKdrtF6fqQpPixsvHa/SUr/K4w7UUBg9o1kLFBm qOfx6maACFWa21EVvWeRVHb/02ilqVidaMMg9U8umN6/2Bdf/Ks4ZDQRTUJ68XXgO0Rv7Jq7emv F3NGPgh8eJoHRBpJZzyWyqnet/Z9evJvDKTJ2+5wM1aRSd3eOZAwHR0215CDp683TXHU0Jv367i OhDxhc+3WtBUkRv9c925Ic+4cdJBkIOvaA3WEOTD7utuQ= X-Gm-Gg: ASbGncv6qPOeAeu077IJ7shQWPsjOEbs3BzAW3/Ogr1W5MtqUgRpozSfN9+83dIor1r qObS6wx2yPQ4Dj+fXTmxmZ6qBBzk/JjK3LZfF3Lw8fO5q8ld/fxi7rPd7ERL+716v3yqmhHOjBK +RGEE1RgXeopWIr5IN3f9H58xiJCe3FiP1XlGEx02nymeHMlZtc5y8nbq39z9db0DIFGKxbYGW5 zl23zP1YeZIG6fZfelUq9JI//LFB1FM/X/q7hK0bLEFpz4rq7At9OYjzYe6+lkifYYxgDHzX9ab XA== X-Received: by 2002:a05:600c:19cc:b0:434:a90b:94fe with SMTP id 5b1f17b1804b1-434d09b4fdfmr19373535e9.10.1733222127501; Tue, 03 Dec 2024 02:35:27 -0800 (PST) X-Google-Smtp-Source: AGHT+IEBEAiPyAwfx+YwYisRpuJJJA/ADUXKip4fErYwLWvkQWJEYVDZ9vPElvTGnfgRUAAy+M4q0Q== X-Received: by 2002:a05:600c:19cc:b0:434:a90b:94fe with SMTP id 5b1f17b1804b1-434d09b4fdfmr19373175e9.10.1733222126880; Tue, 03 Dec 2024 02:35:26 -0800 (PST) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434b0f70d9csm188682175e9.38.2024.12.03.02.35.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 02:35:25 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Tom Tromey , Hannes Domani Subject: [PATCHv3 1/5] gdb/testsuite: avoid incorrect symbols in gdb.base/condbreak-multi-context.cc Date: Tue, 3 Dec 2024 10:35:18 +0000 Message-Id: <9bec1bde3d4dec398ddc0eb57d4b8f80a3c45024.1733221992.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-MFC-PROC-ID: kSBRagw6ElBf2J6DE6m9eiaeJZkvnSFJyqzRVJcXThw_1733222128 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.2 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_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, URIBL_BLOCKED, WEIRD_PORT 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org In a later commit I tweak how disabled_by_cond is handled in update_breakpoint_locations for code_breakpoint objects. I believe this fixes a bug in GDB, but when I did this I ran into a regression in the test script gdb.base/condbreak-multi-context.cc which I think is actually an issue with this test. The test relies on creating a multi-location breakpoint with a condition and having GDB disable some of the locations as the condition is only valid in some of the locations. Here's an example of the test creating one such breakpoint: Reading symbols from /tmp/build/gdb/testsuite/outputs/gdb.base/condbreak-multi-context/condbreak-multi-context... (gdb) break func if a == 10 warning: failed to validate condition at location 1, disabling: No symbol "a" in current context. warning: failed to validate condition at location 3, disabling: No symbol "a" in current context. Breakpoint 1 at 0x401142: func. (3 locations) (gdb) info breakpoints Num Type Disp Enb Address What 1 breakpoint keep y stop only if a == 10 1.1 N* 0x0000000000401142 in Base::func() at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:23 1.2 y 0x000000000040114e in A::func() at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:31 1.3 N* 0x000000000040115a in C::func() at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:39 (*): Breakpoint condition is invalid at this location. (gdb) Notice that only location 1.2 is actually enabled, 1.1 and 1.3 are disabled due to the condition 'a == 10' not being valid. However, notice that this b/p is created before GDB has started the inferior. What I noticed is that if I first start the inferior then I get a different behaviour: Reading symbols from /tmp/build/gdb/testsuite/outputs/gdb.base/condbreak-multi-context/condbreak-multi-context... (gdb) start Temporary breakpoint 1 at 0x40110e: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc, line 49. Starting program: /tmp/build/gdb/testsuite/outputs/gdb.base/condbreak-multi-context/condbreak-multi-context Temporary breakpoint 1, main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:49 49 aobj.func (); (gdb) break func if a == 10 Breakpoint 2 at 0x401142: func. (3 locations) (gdb) info breakpoints Num Type Disp Enb Address What 2 breakpoint keep y stop only if a == 10 2.1 y 0x0000000000401142 in Base::func() at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:23 2.2 y 0x000000000040114e in A::func() at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:31 2.3 y 0x000000000040115a in C::func() at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/condbreak-multi-context.cc:39 (gdb) Notice that now all three locations are valid. What's actually happening is that, on my machine libm.so contains a global symbol 'a' which for 2.1 and 2.3 is being used to satisfy the condition. I don't believe this is actually the intention of the test, this is just an unfortunate consequence of name collision. The test actually relies on the local variables 'a' and 'c', and my libm.so contains both a global version of both. So I propose that we just update the test, I've gone for the super inventive 'aaa' and 'ccc'. With this change, after starting the inferior I now see the expected behaviour where only one of the three locations is enabled. However, while I'm fixing this I figure that it would be nice if the test checked both cases, creating the breakpoints before starting the inferior, and after starting the inferior. So I've updated the test to check both cases. This has meant converting the mostly linear test script into a set of parameterised functions when I then call with a flag to indicate if the inferior should be started before of after creating the breakpoints. Approved-By: Tom Tromey Tested-By: Hannes Domani --- .../gdb.base/condbreak-multi-context.cc | 6 +- .../gdb.base/condbreak-multi-context.exp | 231 ++++++++++++------ 2 files changed, 165 insertions(+), 72 deletions(-) diff --git a/gdb/testsuite/gdb.base/condbreak-multi-context.cc b/gdb/testsuite/gdb.base/condbreak-multi-context.cc index b5628f01bd0..a674fd24a66 100644 --- a/gdb/testsuite/gdb.base/condbreak-multi-context.cc +++ b/gdb/testsuite/gdb.base/condbreak-multi-context.cc @@ -18,7 +18,7 @@ class Base { public: - static const int b = 20; + static const int bbb = 20; void func () {} }; @@ -26,7 +26,7 @@ public: class A : public Base { public: - static const int a = 10; + static const int aaa = 10; void func () {} }; @@ -34,7 +34,7 @@ public: class C : public Base { public: - static const int c = 30; + static const int ccc = 30; void func () {} }; diff --git a/gdb/testsuite/gdb.base/condbreak-multi-context.exp b/gdb/testsuite/gdb.base/condbreak-multi-context.exp index 3af37081e44..3d609a2272c 100644 --- a/gdb/testsuite/gdb.base/condbreak-multi-context.exp +++ b/gdb/testsuite/gdb.base/condbreak-multi-context.exp @@ -18,7 +18,7 @@ standard_testfile .cc -if {[prepare_for_testing "failed to prepare" ${binfile} ${srcfile}]} { +if {[build_executable "failed to prepare" ${binfile} ${srcfile}]} { return } @@ -37,17 +37,32 @@ set loc_index(A) 0 set loc_index(Base) 0 set loc_index(C) 0 -# Find breakpoint location contexts. +# Find breakpoint location contexts. This is called before any of the +# tests are run and captures the order in which GDB will create the +# breakpoint locations. +# +# This whole test script does rely on GDB always creating the b/p +# locations in the same order, but that's true right now, and doesn't +# seem likely to change in the near future. + +proc find_location_contexts { } { + global loc_name loc_index fill + global decimal hex gdb_prompt binfile + + clean_restart ${binfile} + + if {![runto_main]} { + return + } -proc find_location_contexts {} { - global loc_name loc_index bpnum1 fill - global decimal hex gdb_prompt + gdb_breakpoint func + set bpnum [get_integer_valueof "\$bpnum" "*UNKNOWN*"] - gdb_test_multiple "info breakpoint $bpnum1" "find_location_indices" { - -re "stop only if ${fill}\r\n" { + gdb_test_multiple "info breakpoint $bpnum" "find_location_indices" { + -re "\r\n$bpnum\\s+breakpoint\\s+keep\\s+y\\s+\\s*\r\n" { exp_continue } - -re "^${bpnum1}\.($decimal) ${fill} ${hex} in (A|Base|C)::func${fill}\r\n" { + -re "^${bpnum}\.($decimal) ${fill} ${hex} in (A|Base|C)::func${fill}\r\n" { set index $expect_out(1,string) set name $expect_out(2,string) set loc_name($index) $name @@ -72,7 +87,7 @@ proc find_location_contexts {} { # B::func, and 'n' at C::func, respectively. proc check_bp_locations {bpnum states cond {msg ""}} { - global loc_name fill + global loc_name fill loc_states # Map location names to location states. set loc_states(A) [lindex $states 0] @@ -103,67 +118,87 @@ proc check_bp_locations {bpnum states cond {msg ""}} { # Scenario 1: Define breakpoints conditionally, using the "break N if # cond" syntax. Run the program, check that we hit those locations # only. +# +# When START_BEFORE is true we create the breakpoints after running to +# main. When START_BEFORE is false we create the breakpoints after +# starting GDB, but before running to main. + +proc_with_prefix scenario_1 { start_before } { + global warning decimal fill bkptno_num_re binfile + + clean_restart ${binfile} + + if { $start_before } { + if {![runto_main temporary]} { + return + } + } -with_test_prefix "scenario 1" { # Define the conditional breakpoints. Two locations (Base::func # and C::func) should be disabled. We do not test location # indices strictly at this moment, because we don't know them, # yet. We have strict location index tests below. - gdb_test "break func if a == 10" \ + gdb_test "break func if aaa == 10" \ [multi_line \ "${warning} at location $decimal, disabling:" \ - " No symbol \"a\" in current context." \ + " No symbol \"aaa\" in current context." \ "${warning} at location $decimal, disabling:" \ - " No symbol \"a\" in current context." \ + " No symbol \"aaa\" in current context." \ "Breakpoint $decimal at $fill .3 locations."] \ - "define bp with condition a == 10" + "define bp with condition aaa == 10" set bpnum1 [get_integer_valueof "\$bpnum" 0 "get bpnum1"] - gdb_test "break func if c == 30" \ + gdb_test "break func if ccc == 30" \ [multi_line \ ".*${warning} at location $decimal, disabling:" \ - " No symbol \"c\" in current context." \ + " No symbol \"ccc\" in current context." \ ".*${warning} at location $decimal, disabling:" \ - " No symbol \"c\" in current context." \ + " No symbol \"ccc\" in current context." \ ".*Breakpoint $decimal at $fill .3 locations."] \ - "define bp with condition c == 30" + "define bp with condition ccc == 30" set bpnum2 [get_integer_valueof "\$bpnum" 0 "get bpnum2"] - find_location_contexts - - with_test_prefix "before run" { - check_bp_locations $bpnum1 {y N* N*} "a == 10" - check_bp_locations $bpnum2 {N* N* y} "c == 30" + with_test_prefix "after creating b/p" { + check_bp_locations $bpnum1 {y N* N*} "aaa == 10" + check_bp_locations $bpnum2 {N* N* y} "ccc == 30" } - # Do not use runto_main, it deletes all breakpoints. - gdb_run_cmd + if { !$start_before } { + if {![runto_main temporary no-delete-breakpoints]} { + return + } + } # Check our conditional breakpoints. - gdb_test "" ".*Breakpoint $bkptno_num_re, A::func .*" \ + gdb_test "continue" ".*Breakpoint $bkptno_num_re, A::func .*" \ "run until A::func" - gdb_test "print a" " = 10" + gdb_test "print aaa" " = 10" gdb_test "continue" "Continuing.*Breakpoint $bkptno_num_re, C::func .*" \ "run until C::func" - gdb_test "print c" " = 30" + gdb_test "print ccc" " = 30" # No more hits! gdb_continue_to_end with_test_prefix "after run" { - check_bp_locations $bpnum1 {y N* N*} "a == 10" - check_bp_locations $bpnum2 {N* N* y} "c == 30" + check_bp_locations $bpnum1 {y N* N*} "aaa == 10" + check_bp_locations $bpnum2 {N* N* y} "ccc == 30" } } -# Start GDB with two breakpoints and define the conditions separately. +# Assuming GDB is already started, create two breakpoints and define +# the conditions separately. +# +# BPNUM1_NAME and BPNUM2_NAME contain the name of two variables in the +# parent contents. The breakpoint numbers of the two created +# breakpoints are placed into these variables in the parent content. -proc setup_bps {} { - global srcfile binfile srcfile2 decimal - global bpnum1 bpnum2 bp_location warning loc_index +proc setup_bps { bpnum1_name bpnum2_name } { + global warning loc_index - clean_restart ${binfile} + upvar $bpnum1_name bpnum1 + upvar $bpnum2_name bpnum2 # Define the breakpoints. gdb_breakpoint "func" @@ -172,74 +207,105 @@ proc setup_bps {} { gdb_breakpoint "func" set bpnum2 [get_integer_valueof "\$bpnum" 0 "get bpnum2"] - # Defining a condition on 'a' disables 2 locations. + # Defining a condition on 'aaa' disables 2 locations. set locs [lsort -integer "$loc_index(Base) $loc_index(C)"] - gdb_test "cond $bpnum1 a == 10" \ + gdb_test "cond $bpnum1 aaa == 10" \ [multi_line \ "$warning at location ${bpnum1}.[lindex $locs 0], disabling:" \ - " No symbol \"a\" in current context." \ + " No symbol \"aaa\" in current context." \ "$warning at location ${bpnum1}.[lindex $locs 1], disabling:" \ - " No symbol \"a\" in current context."] + " No symbol \"aaa\" in current context."] # Defining a condition on 'c' disables 2 locations. set locs [lsort -integer "$loc_index(Base) $loc_index(A)"] - gdb_test "cond $bpnum2 c == 30" \ + gdb_test "cond $bpnum2 ccc == 30" \ [multi_line \ "$warning at location ${bpnum2}.[lindex $locs 0], disabling:" \ - " No symbol \"c\" in current context." \ + " No symbol \"ccc\" in current context." \ "$warning at location ${bpnum2}.[lindex $locs 1], disabling:" \ - " No symbol \"c\" in current context."] + " No symbol \"ccc\" in current context."] } # Scenario 2: Define breakpoints unconditionally, and then define # conditions using the "cond N " syntax. Expect that the # locations where is not evaluatable are disabled. Run the # program, check that we hit the enabled locations only. +# +# When START_BEFORE is true we create the breakpoints after running to +# main. When START_BEFORE is false we create the breakpoints after +# starting GDB, but before running to main. + +proc_with_prefix scenario_2 { start_before } { + global binfile bkptno_num_re + + clean_restart ${binfile} -with_test_prefix "scenario 2" { - setup_bps + if { $start_before } { + if {![runto_main temporary]} { + return + } + } + + setup_bps bpnum1 bpnum2 - with_test_prefix "before run" { - check_bp_locations $bpnum1 {y N* N*} "a == 10" - check_bp_locations $bpnum2 {N* N* y} "c == 30" + with_test_prefix "after creating b/p" { + check_bp_locations $bpnum1 {y N* N*} "aaa == 10" + check_bp_locations $bpnum2 {N* N* y} "ccc == 30" } - # Do not use runto_main, it deletes all breakpoints. - gdb_run_cmd + if { !$start_before } { + if {![runto_main temporary no-delete-breakpoints]} { + return + } + } # Check that we hit enabled locations only. - gdb_test "" ".*Breakpoint $bkptno_num_re, A::func .*" \ + gdb_test "continue" ".*Breakpoint $bkptno_num_re, A::func .*" \ "run until A::func" - gdb_test "print a" " = 10" + gdb_test "print aaa" " = 10" gdb_test "continue" "Continuing.*Breakpoint $bkptno_num_re, C::func .*" \ "run until C::func" - gdb_test "print c" " = 30" + gdb_test "print ccc" " = 30" # No more hits! gdb_continue_to_end with_test_prefix "after run" { - check_bp_locations $bpnum1 {y N* N*} "a == 10" - check_bp_locations $bpnum2 {N* N* y} "c == 30" + check_bp_locations $bpnum1 {y N* N*} "aaa == 10" + check_bp_locations $bpnum2 {N* N* y} "ccc == 30" } } # Scenario 3: Apply misc. checks on the already-defined breakpoints. +# +# When START_BEFORE is true we create the breakpoints after running to +# main. When START_BEFORE is false we create the breakpoints after +# starting GDB, but before running to main. + +proc_with_prefix scenario_3 { start_before } { + global binfile bkptno_num_re loc_index warning + + clean_restart ${binfile} -with_test_prefix "scenario 3" { - setup_bps + if { $start_before } { + if {![runto_main temporary]} { + return + } + } + + setup_bps bpnum1 bpnum2 set locs [lsort -integer "$loc_index(Base) $loc_index(A)"] - gdb_test "cond $bpnum1 c == 30" \ + gdb_test "cond $bpnum1 ccc == 30" \ [multi_line \ "${warning} at location ${bpnum1}.[lindex $locs 0], disabling:" \ - " No symbol \"c\" in current context." \ + " No symbol \"ccc\" in current context." \ "${warning} at location ${bpnum1}.[lindex $locs 1], disabling:" \ - " No symbol \"c\" in current context." \ + " No symbol \"ccc\" in current context." \ "Breakpoint ${bpnum1}'s condition is now valid at location $loc_index(C), enabling."] \ "change the condition of bp 1" - check_bp_locations $bpnum1 {N* N* y} "c == 30" "after changing the condition" + check_bp_locations $bpnum1 {N* N* y} "ccc == 30" "after changing the condition" gdb_test "cond $bpnum1" \ [multi_line \ @@ -250,7 +316,7 @@ with_test_prefix "scenario 3" { check_bp_locations $bpnum1 {y y y} "" "after resetting the condition" gdb_test_no_output "disable ${bpnum2}.$loc_index(A)" - check_bp_locations $bpnum2 {N* N* y} "c == 30" "after disabling loc for A" + check_bp_locations $bpnum2 {N* N* y} "ccc == 30" "after disabling loc for A" gdb_test "cond $bpnum2" ".*" "reset the condition of bp 2" check_bp_locations $bpnum2 {n y y} "" "loc for A should remain disabled" @@ -258,17 +324,17 @@ with_test_prefix "scenario 3" { gdb_test_no_output "disable ${bpnum2}.$loc_index(C)" check_bp_locations $bpnum2 {n y n} "" "after disabling loc for C" - gdb_test "cond $bpnum2 c == 30" \ + gdb_test "cond $bpnum2 ccc == 30" \ [multi_line \ "${warning} at location ${bpnum2}.$loc_index(Base), disabling:" \ - " No symbol \"c\" in current context."] \ + " No symbol \"ccc\" in current context."] \ "re-define a condition" - check_bp_locations $bpnum2 {N* N* n} "c == 30" "loc for C should remain disabled" + check_bp_locations $bpnum2 {N* N* n} "ccc == 30" "loc for C should remain disabled" gdb_test "enable ${bpnum2}.$loc_index(Base)" \ "Breakpoint ${bpnum2}'s condition is invalid at location $loc_index(Base), cannot enable." \ "reject enabling a location that is disabled-by-cond" - check_bp_locations $bpnum2 {N* N* n} "c == 30" "after enable attempt" + check_bp_locations $bpnum2 {N* N* n} "ccc == 30" "after enable attempt" gdb_test "cond $bpnum2 garbage" \ "No symbol \"garbage\" in current context." \ @@ -277,19 +343,34 @@ with_test_prefix "scenario 3" { gdb_test_no_output "delete $bpnum1" # Do not use runto_main, it deletes all breakpoints. - gdb_breakpoint "main" - gdb_run_cmd - gdb_test "" ".*reakpoint .*, main .*${srcfile}.*" "start" + if { !$start_before } { + if {![runto_main temporary no-delete-breakpoints]} { + return + } + } # The second BP's locations are all disabled. No more hits! gdb_continue_to_end } # Scenario 4: Test the '-force'/'-force-condition' flag. +# +# When START_BEFORE is true we create the breakpoints after running to +# main. When START_BEFORE is false we create the breakpoints after +# starting GDB, but before running to main, in fact we don't even +# bother with a run to main in this case. + +proc_with_prefix scenario_4 { start_before } { + global binfile bkptno_num_re loc_index warning -with_test_prefix "force" { clean_restart ${binfile} + if { $start_before } { + if {![runto_main temporary]} { + return + } + } + gdb_breakpoint "func" # Pick a condition that is invalid at every location. set bpnum1 [get_integer_valueof "\$bpnum" 0 "get bpnum1"] @@ -309,3 +390,15 @@ with_test_prefix "force" { set bpnum2 [get_integer_valueof "\$bpnum" 0 "get bpnum2"] check_bp_locations $bpnum2 {N* N* N*} "baz" "set using the break command" } + +# Some initial setup. Needed for most of the different scenarios +# below. +find_location_contexts + +# Now run all of the separate scenarios. +foreach_with_prefix start_before { true false } { + scenario_1 $start_before + scenario_2 $start_before + scenario_3 $start_before + scenario_4 $start_before +} From patchwork Tue Dec 3 10:35:19 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 102294 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 B9C563858D38 for ; Tue, 3 Dec 2024 10:37:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B9C563858D38 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ckVYb8Ld 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 ESMTP id 8616B3858C50 for ; Tue, 3 Dec 2024 10:35:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8616B3858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8616B3858C50 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222132; cv=none; b=l0GKb9Csq+5ki8jjj9wVCoZpVrsv1aB4UtOjnGL59HcIO2n+y+eKHzBVH76EX0WnEEORmpjs1DO4jjBl4BflKDDY1oS48wZCkbt3IXVQKjqQIw45BrOUcA0t5rzn+TH34PotWwocrK3s8zOD5W7ZjUnc1+rinDXkXXeM4kku8Vc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222132; c=relaxed/simple; bh=GTer2TNHoeP/lW2bybQsBum20NvJZgkQtRdSILjzTik=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=S5aNDT2GL5lvB+KOYuc/PvO5wdO4Mbqn65BaLdnGWwaGHWgjc5X/ZRFCsPFdVDJe2TcqlR4vQcFowSx7h6ErPigG+1Rto5m1VuwI7Rno0U67R8Lp3s6pIMt9BAOGB5K5mBrjCG7ISAxP4gUm5fUA8O7xyjxrCmH5bFdezeJSZ1A= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8616B3858C50 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733222132; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AuXuGjaUuy15/0YhJvdExvtPoIU2At7ARA/5T+m1Xmg=; b=ckVYb8LdSFJ2yxmm5A57eVuqU6psnTWlp8p4oTHxvOz+Ii+RON6JoHrxUjilWm4c3e+kj/ diA0e7K+Pmwvf+Iz3ZXVsplpCd3g2V7zVzesGorEPO1thK0Swb5TYkOlpYeZ1PEDCiFlCQ ygJKo9UZqIczYDaXiN8upgQ7D/icdvc= 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_256_GCM_SHA384) id us-mta-451-B9r6WwxPPTKDjJW8tBbkkA-1; Tue, 03 Dec 2024 05:35:31 -0500 X-MC-Unique: B9r6WwxPPTKDjJW8tBbkkA-1 X-Mimecast-MFC-AGG-ID: B9r6WwxPPTKDjJW8tBbkkA Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-385e27c5949so2123151f8f.3 for ; Tue, 03 Dec 2024 02:35:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733222129; x=1733826929; 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=AuXuGjaUuy15/0YhJvdExvtPoIU2At7ARA/5T+m1Xmg=; b=FQQWdIMtNQV86aROS5BAduOXXnqLO29YL2r1NzFTes3MxrpLG/cmZQjO+WtOCNC+RO Zq2R6q+dao4uxALGXlveFCdMa6mRZlr3pP92ENzCqgI8UJVXiIfOl4xAMKkivV+EwE1C KBLqKQJu05v/gfoIuQaYo12zyxSsbkZOM2EfhjtHjBqzUHyrVNgdJMTsdi9qwPJjmcxf 8GPIwOJnREHUmFvvrNeitKa2kt5B7kV+YAlQ1DiMcXjf1v6+D83K6uDgetJhPbKXJLUc Q2jDoraMiJLZMi1N4qTXqMFd5mNb8pi7BZBLd3+d3OgAtwBUisA7bpU+j74aMXOtyh5q tL5w== X-Gm-Message-State: AOJu0YziBLF7zG511KkXhKAQojf1bsRCSq180UOoussQgBN4G6wierfA 1EOzoY0QLph3lmkctj83e37DDOzaE1DYDRhM9T/xuOq1TR1AhXxnbr+24d5hcUjh9aWRbDdbtUO c0QD2f7l+9Awh09cpUtKy0Xo+2j5f2Q5OGHK9dvrAnBi1ft5bsC8yKkS2cljPCAKZgY5bPFKqMh ZHmTCmkuBFmv0hYSaV7fDymS7BmmZjpBjo76TWc3u6Wao= X-Gm-Gg: ASbGnctOTbT2OYyI25klmWwBvJlHayvVRYCQ+ODwuF7nTIuKxMT+fb2J+wg1K+p962h pRe1O2BpNtDG8dsIB0FHm0Ww7RJTwtN4jvfqodT8jfaSLOtTu6Iv5Dcrfm5wuO92UjAQcvrRscV D2dji1TBgx8vNy8okYM0laUxfkeBofiB4FsrKmkxLZ1YErC6zlr62twtyAM8OFJvgPobcoq7PT7 01uPdOPqQSlRXuLII0tFof4YjnJvbdzC/rJR/9duKGDxoTRNRBaiSMhDXj0A4AwOweTj44c1sgr QA== X-Received: by 2002:a5d:584f:0:b0:385:f60b:f5c2 with SMTP id ffacd0b85a97d-385fd423f82mr1262655f8f.41.1733222128924; Tue, 03 Dec 2024 02:35:28 -0800 (PST) X-Google-Smtp-Source: AGHT+IH6p8NSLDirDbei0VJHgwTWdUUL25jh1jyroM3AZz/mBlRinnM026niRTeWywuEzWQAW/G8xA== X-Received: by 2002:a5d:584f:0:b0:385:f60b:f5c2 with SMTP id ffacd0b85a97d-385fd423f82mr1262625f8f.41.1733222128105; Tue, 03 Dec 2024 02:35:28 -0800 (PST) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-385e1bba97esm10491887f8f.91.2024.12.03.02.35.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 02:35:27 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Hannes Domani Subject: [PATCHv3 2/5] gdb: fixes for code_breakpoint::disabled_by_cond logic Date: Tue, 3 Dec 2024 10:35:19 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: mvIv2hKpzrTErqyuufgkAdHJ6ZRlNvTIeyvRxFlbafo_1733222130 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.2 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, URIBL_BLOCKED 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org I spotted that the code_breakpoint::disabled_by_cond flag doesn't work how I'd expect it too. The flag appears to be "sticky" in some situations; once a code_breakpoint::disabled_by_cond flag is marked true, then, in some cases the flag wont automatically become false again, even when you'd think it should. The problem is in update_breakpoint_locations. In this function, which is called as a worker of code_breakpoint::re_set, GDB computes a new set of locations for a breakpoint, the new locations are then installed into the breakpoint. However, before installing the new locations GDB attempts to copy the bp_location::enabled and bp_location::disabled_by_cond flag from the old locations into the new locations. The reason for copying the ::enabled flag makes sense. This flag is controlled by the user. When we create the new locations if GDB can see that a new location is equivalent to one of the old locations, and if the old location was disabled by the user, then the new location should also be disabled. However, I think the logic behind copying the ::disabled_by_cond flag is wrong. The disabled_by_cond flag is controlled by GDB and should toggle automatically. If the condition string can be parsed then the flag should be false (b/p enabled), if the condition string can't be parsed then the flag should be true (b/p disabled). As we always parse the condition string in update_breakpoint_locations before we try to copy the ::enabled flag value then the ::disabled_by_cond flag should already be correct, there's no need to copy over the ::disabled_by_cond value from the old location. As a concrete example, consider a b/p placed within the main executable, but with a condition that depends on a variable within a shared library. When the b/p is initially created the b/p will be disabled as the condition string will be invalid (the shared library variable isn't available yet). When the inferior starts the shared library is loaded and the condition variable becomes available to GDB. When the shared library is loaded breakpoint_re_set is called which (eventually) calls update_breakpoint_locations. A new location is computed for the breakpoint and the condition string is parsed. As the shared library variable is now know the expression parses correctly and ::disabled_by_cond is left false for the new location. But currently GDB spots that the new location is at the same address as the old location and copies disabled_by_cond over from the old location, which marks the b/p location as disabled. This is not what I would expect. The solution is simple, don't copy over disabled_by_cond. While writing a test I found another problem though. The disabled_by_cond flag doesn't get set true when it should! This is the exact opposite of the above. The problem here is in solib_add which is (despite the name) called whenever the shared library set changes, including when a shared library is unloaded. Imagine an executable that uses dlopen/dlclose to load a shared library. Given an example of a b/p in the main executable that has a condition that uses a variable from our shared library, a library which might be unloaded with dlclose. My expectation is that, when the library is unloaded, GDB will automatically mark the breakpoint as disabled_by_cond, however, this was not happening. The problem is that in solib_add we only call breakpoint_re_set when shared libraries are added, not when shared libraries are removed. The solution I think is to just call breakpoint_re_set in both cases, now the disabled_by_cond flag is updated as I'd expect. Unfortunately, making this change causes a regression when running: make check-gdb \ TESTS="gdb.trace/change-loc.exp" \ RUNTESTFLAGS="--target_board=native-gdbserver" This test unloads a shared library and expects breakpoints within the shared library to enter the PENDING state (because the bp_location's shlib_disabled flag will be set). However, the new call to breakpoint_re_set means that this is no longer the case. The breakpoint_re_set call means that update_breakpoint_locations is called, which then checks if all locations for a breakpoint are pending or not. In this test not all locations are pending, and so GDB recalculates the locations of each breakpoint, this means that pending locations are discarded. There is a but report PR gdb/32404 which mentions the problems with shlib_disabled pending breakpoints, and how they are prone to being randomly deleted if the user can cause GDB to trigger a call to breakpoint_re_set. This patch just adds another call to breakpoint_re_set, which triggers this bug in this one test case. For now I have marked this test as KFAIL. I do plan to try and address the pending (shlib_disabled) breakpoint problem in the future, but I'm not sure when that will be right now. There are, of course, tests to cover all these cases. During review I was pointed at bug PR gdb/32079 as something that this commit might fix, or help in fixing. And this commit is part of the fix for that bug, but is not the complete solution. However, the remaining parts of the fix for that bug are not really related to the content of this commit. The problem in PR gdb/32079 is that the inferior maps multiple copies of libc in different linker namespaces using dlmopen (actually libc is loaded as a consequence of loading some other library into a different namespace, but that's just a detail). The user then uses a 'next' command to move the inferior forward. GDB sets up internal breakpoints on the longjmp symbols, of which there are multiple copies (there is a copy in every loaded libc). However, the 'next' command is, in the problem case, stepping over a dlclose call which unloads one of the loaded libc libraries. In current HEAD GDB in solib_add we fail to call breakpoint_re_set() when the library is unloaded; breakpoint_re_set() would delete and then recreate the longjmp breakpoints. As breakpoint_re_set() is not called GDB thinks that the the longjmp breakpoint in the now unloaded libc still exists, and is still inserted. When the inferior stops after the 'next' GDB tries to delete and remove the longjmp breakpoint which fails as the libc in which the breakpoint was inserted is no longer mapped in. When the user tries to 'next' again GDB tries to re-insert the still existing longjmp breakpoint which again fails as the memory in which the b/p should be inserted is no longer part of the inferior memory space. This commit helps a little. Now when the libc library is unmapped GDB does call breakpoint_re_set(). This deletes the longjmp breakpoints including the one in the unmapped library, then, when we try to recreate the longjmp breakpoints (at the end of breakpoint_re_set) we don't create a b/p in the now unmapped copy of libc. However GDB does still think that the deleted breakpoint is inserted. The breakpoint location remains in GDB's data structures until the next time the inferior stops, at which point GDB tries to remove the breakpoint .... and fails. However, as the b/p is now deleted, when the user tries to 'next' GDB no longer tries to re-insert the b/p, and so one of the problems reported in PR gdb/32079 is resolved. I'll fix the remaining issues from PR gdb/32079 in a later commit in this series. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32079 Tested-By: Hannes Domani --- gdb/breakpoint.c | 5 +- gdb/solib.c | 2 +- .../gdb.base/bp-disabled-by-cond-lib.c | 24 ++ gdb/testsuite/gdb.base/bp-disabled-by-cond.c | 64 ++++++ .../gdb.base/bp-disabled-by-cond.exp | 206 ++++++++++++++++++ gdb/testsuite/gdb.base/bp-disabled-by-cond.py | 21 ++ gdb/testsuite/gdb.trace/change-loc.exp | 2 + 7 files changed, 319 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/bp-disabled-by-cond-lib.c create mode 100644 gdb/testsuite/gdb.base/bp-disabled-by-cond.c create mode 100644 gdb/testsuite/gdb.base/bp-disabled-by-cond.exp create mode 100644 gdb/testsuite/gdb.base/bp-disabled-by-cond.py diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0a889a04e81..7b84126c241 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -13016,8 +13016,7 @@ update_breakpoint_locations (code_breakpoint *b, for (const bp_location &e : existing_locations) { - if (e.function_name == nullptr - || (e.enabled && !e.disabled_by_cond)) + if (e.function_name == nullptr || e.enabled) continue; if (have_ambiguous_names) @@ -13034,7 +13033,6 @@ update_breakpoint_locations (code_breakpoint *b, if (breakpoint_locations_match (&e, &l, true)) { l.enabled = e.enabled; - l.disabled_by_cond = e.disabled_by_cond; break; } } @@ -13047,7 +13045,6 @@ update_breakpoint_locations (code_breakpoint *b, l.function_name.get ()) == 0) { l.enabled = e.enabled; - l.disabled_by_cond = e.disabled_by_cond; break; } } diff --git a/gdb/solib.c b/gdb/solib.c index fdefdf0b142..0c63d31f645 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -973,7 +973,7 @@ solib_add (const char *pattern, int from_tty, int readsyms) } } - if (loaded_any_symbols) + if (loaded_any_symbols || !current_program_space->deleted_solibs.empty ()) breakpoint_re_set (); if (from_tty && pattern && !any_matches) diff --git a/gdb/testsuite/gdb.base/bp-disabled-by-cond-lib.c b/gdb/testsuite/gdb.base/bp-disabled-by-cond-lib.c new file mode 100644 index 00000000000..ab0cb9dcb16 --- /dev/null +++ b/gdb/testsuite/gdb.base/bp-disabled-by-cond-lib.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +volatile int lib_global = 0; + +void +lib_func (void) +{ + /* Nothing. */ +} diff --git a/gdb/testsuite/gdb.base/bp-disabled-by-cond.c b/gdb/testsuite/gdb.base/bp-disabled-by-cond.c new file mode 100644 index 00000000000..cf61c2821df --- /dev/null +++ b/gdb/testsuite/gdb.base/bp-disabled-by-cond.c @@ -0,0 +1,64 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 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 + +#ifdef __WIN32__ +#include +#define dlopen(name, mode) LoadLibrary (TEXT (name)) +#ifdef _WIN32_WCE +# define dlsym(handle, func) GetProcAddress (handle, TEXT (func)) +#else +# define dlsym(handle, func) GetProcAddress (handle, func) +#endif +#define dlclose(handle) FreeLibrary (handle) +#else +#include +#endif + +void +breakpt () +{ + /* Nothing. */ +} + +int +main (void) +{ + int res; + void *handle; + void (*func) (void); + + handle = dlopen (SHLIB_NAME, RTLD_LAZY); + assert (handle != NULL); + + func = (void (*)(void)) dlsym (handle, "lib_func"); + assert (func != NULL); + + breakpt (); /* Conditional BP location. */ + breakpt (); /* Other BP location. */ + + func (); + + res = dlclose (handle); + assert (res == 0); + + breakpt (); /* BP before exit. */ + + return 0; +} diff --git a/gdb/testsuite/gdb.base/bp-disabled-by-cond.exp b/gdb/testsuite/gdb.base/bp-disabled-by-cond.exp new file mode 100644 index 00000000000..6104787ed3a --- /dev/null +++ b/gdb/testsuite/gdb.base/bp-disabled-by-cond.exp @@ -0,0 +1,206 @@ +# Copyright 2024 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 . + +# Breakpoint locations can be marked as "disabled due to their +# condition". This test sets up a breakpoint which depends on reading +# a variable in a shared library. We then check that the b/p is +# correctly disabled before the shared library is loaded, becomes +# enabled once the shared library is loaded. And becomes disabled +# again when the shared libraries are unloaded. + +standard_testfile .c -lib.c + +set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py] + +# Build the library and copy it to the target. +set libname ${testfile}-lib +set libfile [standard_output_file $libname] +if { [build_executable "build shlib" $libfile $srcfile2 {debug shlib}] == -1} { + return +} +set libfile_on_target [gdb_download_shlib $libfile] + +# Build the executable. +set opts [list debug shlib_load additional_flags=-DSHLIB_NAME=\"${libname}\"] +if { [build_executable "build exec" $binfile $srcfile $opts] == -1} { + return +} + +set other_bp_line [gdb_get_line_number "Other BP location" $srcfile] +set cond_bp_line [gdb_get_line_number "Conditional BP location" $srcfile] +set exit_bp_line [gdb_get_line_number "BP before exit" $srcfile] + +# Setup a conditional b/p, the condition of which depends on reading a +# variable from a shared library. The b/p will initially be created +# disabled (due to the condition). +# +# Continue the inferior, when the shared library is loaded GDB should +# make the b/p enabled. +# +# Restart the inferior, which should unload the shared library, GDB +# should mark the b/p as disabled due to its condition again. +proc run_test { hit_cond } { + clean_restart $::binfile + + if {![runto_main]} { + return + } + + # Setup breakpoints. + gdb_breakpoint $::srcfile:$::other_bp_line + set other_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of other b/p"] + + gdb_breakpoint $::srcfile:$::exit_bp_line + set exit_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of exit b/p"] + + gdb_breakpoint $::srcfile:$::cond_bp_line + set cond_bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get number of conditional b/p"] + + if { $hit_cond } { + set lib_global_val 0 + } else { + set lib_global_val 1 + } + + # Set the condition. Use 'force' as we're referencing a variable in + # the shared library, which hasn't been loaded yet. The breakpoint + # will immediately be marked as disabled_by_cond. + gdb_test "condition -force $cond_bp_num lib_global == $lib_global_val" \ + [multi_line \ + "warning: failed to validate condition at location $cond_bp_num\\.1, disabling:" \ + " No symbol \"lib_global\" in current context\\."] \ + "set b/p condition, it will be disabled" + + # Source Python script if supported. + if { [allow_python_tests] } { + gdb_test_no_output "source $::pyfile" "import python scripts" + gdb_test "python print(bp_modified_list)" "\\\[\\\]" \ + "check b/p modified observer has not yet triggered" + } + + # Check the b/p is indeed marked as disabled (based on its condition). + gdb_test "info breakpoint $cond_bp_num" \ + [multi_line \ + "\r\n$cond_bp_num\\.1\\s+N\\*\\s+$::hex in main at \[^\r\n\]+" \ + "\\(\\*\\): Breakpoint condition is invalid at this location\\."] \ + "conditional breakpoint is disabled based on condition" + + if { $hit_cond } { + # Continue the inferior. The shared library is loaded and the + # breakpoint condition should become valid. We should then stop at + # the conditional breakpoint. + gdb_test "continue" \ + [multi_line \ + "Breakpoint $cond_bp_num, main \\(\\) at \[^\r\n\]+:$::cond_bp_line" \ + "$::cond_bp_line\\s+breakpt \\(\\);\\s+/\\* Conditional BP location\\. \\*/"] \ + "continue until conditional b/p is hit" + } else { + # Continue the inferior. The shared library is loaded and the + # breakpoint condition should become valid. As the condition + # is going to be false GDB will stop at the other line. + gdb_test "continue" \ + [multi_line \ + "Breakpoint $other_bp_num, main \\(\\) at \[^\r\n\]+:$::other_bp_line" \ + "$::other_bp_line\\s+breakpt \\(\\);\\s+/\\* Other BP location\\. \\*/"] \ + "continue until conditional b/p is hit" + } + + if { [allow_python_tests] } { + # We're going to look at the list of b/p that have been + # modified since we loaded the Python script. The first b/p + # modified will be the conditional b/p, this occurs when the + # b/p condition became valid. + # + # The second b/p will be whichever b/p we hit (the hit count + # increased). So figure out which b/p we are going to hit. + if { $hit_cond } { + set second_bp_num $cond_bp_num + } else { + set second_bp_num $other_bp_num + } + + # Now check the list of modified b/p. + gdb_test "python print(bp_modified_list)" \ + "\\\[$cond_bp_num, $second_bp_num\\\]" \ + "check b/p modified observer was triggered" + } + + if {[gdb_protocol_is_remote]} { + set evals_re "(?: \\(\[^) \]+ evals\\))?" + } else { + set evals_re "" + } + + # Check the b/p is no longer marked as disabled. The output is + # basically the same here whether the b/p was hit or not. It's + # just the hit counter line that we need to append or not. + set re_list \ + [list \ + "$cond_bp_num\\s+breakpoint\\s+keep\\s+y\\s+$::hex in main at \[^\r\n\]+:$::cond_bp_line" \ + "\\s+stop only if lib_global == $lib_global_val$evals_re"] + if { $hit_cond } { + lappend re_list "\\s+breakpoint already hit 1 time" + } + set re [multi_line {*}$re_list] + gdb_test "info breakpoint $cond_bp_num" $re \ + "conditional breakpoint is now enabled" + + if { $hit_cond } { + gdb_test "continue" \ + [multi_line \ + "Breakpoint $other_bp_num, main \\(\\) at \[^\r\n\]+:$::other_bp_line" \ + "$::other_bp_line\\s+breakpt \\(\\);\\s+/\\* Other BP location\\. \\*/"] \ + "continue to other b/p" + } + + if {[allow_python_tests]} { + # Clear out the list of modified b/p. This makes the results + # (see below) clearer. + gdb_test_no_output "python bp_modified_list=\[\]" \ + "clear bp_modified_list" + } + + gdb_test "continue" \ + [multi_line \ + "Breakpoint $exit_bp_num, main \\(\\) at \[^\r\n\]+:$::exit_bp_line" \ + "$::exit_bp_line\\s+breakpt \\(\\);\\s+/\\* BP before exit\\. \\*/"] \ + "continue b/p before exit" + + # Check the b/p is once again marked as disabled based on its + # condition. + gdb_test "info breakpoint $cond_bp_num" \ + [multi_line \ + "\r\n$cond_bp_num\\.1\\s+N\\*\\s+$::hex in main at \[^\r\n\]+" \ + "\\(\\*\\): Breakpoint condition is invalid at this location\\."] \ + "conditional breakpoint is again disabled based on condition" + + if { [allow_python_tests] } { + # The condition breakpoint will have been modified (moved to + # the disabled state) when GDB unloaded the shared libraries. + # And the b/p in main will have been modified in that it's hit + # count will have gone up. + gdb_test "python print(bp_modified_list)" \ + "\\\[$cond_bp_num, $exit_bp_num\\\]" \ + "check b/p modified observer was triggered during restart" + } +} + +# The tests. +foreach_with_prefix hit_cond { true false } { + run_test $hit_cond +} diff --git a/gdb/testsuite/gdb.base/bp-disabled-by-cond.py b/gdb/testsuite/gdb.base/bp-disabled-by-cond.py new file mode 100644 index 00000000000..87b374c201e --- /dev/null +++ b/gdb/testsuite/gdb.base/bp-disabled-by-cond.py @@ -0,0 +1,21 @@ +# Copyright (C) 2024 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 . + +bp_modified_list = [] + +def bp_modified(bp): + bp_modified_list.append (bp.number) + +gdb.events.breakpoint_modified.connect(bp_modified) diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp index 1316d92b116..a01cb712887 100644 --- a/gdb/testsuite/gdb.trace/change-loc.exp +++ b/gdb/testsuite/gdb.trace/change-loc.exp @@ -149,6 +149,7 @@ proc tracepoint_change_loc_1 { trace_type } { (4\.\[1-3].* in func4.*\tinstalled on target.*){2}" \ "tracepoint with two locations - installed (unload)" + setup_kfail gdb/32404 "*-*-*" gdb_test "info trace" \ "Num Type\[ \]+Disp Enb Address\[ \]+What.* \[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\.* @@ -263,6 +264,7 @@ proc tracepoint_change_loc_2 { trace_type } { (1\.\[1-3].* in func4.*\tinstalled on target.*){2}" \ "tracepoint with two locations - installed (unload)" + setup_kfail gdb/32404 "*-*-*" gdb_test "info trace" \ "Num Type\[ \]+Disp Enb Address\[ \]+What.* \[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\.* From patchwork Tue Dec 3 10:35:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 102295 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 1313A3858405 for ; Tue, 3 Dec 2024 10:39:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1313A3858405 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PzIGH6ia 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 ESMTP id E915D3858D33 for ; Tue, 3 Dec 2024 10:35:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E915D3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E915D3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222135; cv=none; b=pETe6MRQRbAeNcX0YKPWee3o9JCsZf6xeHYvdit7YPQdGBxs1In86E9X1G4hCaxf1dPcSIsydmg5f1Fat7MseEYjUaAk5UyinCHdBFz3YudQj6Kli8UdrkbXFpbtutwDTcD0PNdu/ZYaQ/w0Kx+AUsAnGjeGajj+LtmtdqhJtns= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222135; c=relaxed/simple; bh=2bAsYfmAKT8RhjSVKRVgIfwtyX0mnNXp7hImx9w5Csg=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=Pz2f5Y0SgHXpyZpcMpaZ1Yj6Kk12ttjctNwg0cHR++PQGNKotlkq9w8cQTzA1U816LP1E7OKEL6R81aE11GKmJt0/IWrSUOjvz6o6iF2qBIrxBPPc0KzeBFNjSOSJFbqezdiuQTVkCvKs4kQiTNZGfm51sTfXklLazQAF3JFrUw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E915D3858D33 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733222134; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=23oQn0vBf9IvXOmDieqnn9U3SNTpYqVzU1IKhVYwLso=; b=PzIGH6iadV9WRR0RH151HJm+gY03zB7ymisEEUIMamUesREWmaDa2G3ybQM652Ks8g/45k lv4Wg5dz/Ckhwjvc2mBYUCmNGqeHbHab1+ViRgapRv84LJNtOZ9PW1IagUdNuNlxnBRYwT XidsVbYTfZ8t6fvK33ZUAXhnM0uJh70= 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_256_GCM_SHA384) id us-mta-172-834Be6WvNv26Z7foS0oVOQ-1; Tue, 03 Dec 2024 05:35:33 -0500 X-MC-Unique: 834Be6WvNv26Z7foS0oVOQ-1 X-Mimecast-MFC-AGG-ID: 834Be6WvNv26Z7foS0oVOQ Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-385df115300so1817241f8f.2 for ; Tue, 03 Dec 2024 02:35:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733222130; x=1733826930; 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=23oQn0vBf9IvXOmDieqnn9U3SNTpYqVzU1IKhVYwLso=; b=autthhlJzVSDky2BmEc1ufa0OUcnM/TjSboIMXV41z06fs4h9JGCDsEixHTfIZWnAf wyUrX4j5uV8ajSiNAb1qTAjWePydbBSerbCB486uFY9Ewpmc5XZFm64nO5vsEdA9jU1y RYuVTvvWVsKior72SMgr7VmCWrlev3oaxsX8ZisGjOGTF5ct3JQQb7n3i/zMKu7OiEnI UMxDvtO3K1qZQaOOqIINcTQYsC6aQ5/t9SlMthdX6VgEyXrEEXmxa/udIV8ShY0O/HgC 7ImL3el7TteVBrwaqvVENlR2oFfSIVQZ8sTKyTfC56vwyzT4Buihr35vzvpmdhIo+yDh QfBg== X-Gm-Message-State: AOJu0YzhNI/5Sa+h0P/XzzeT1CiG2/a3MB1bfhLv9XRSjBLHIin/qlpU +y8gK26MqpJXQFISNVj6PYqewG31dAT+7pqqzwDpxtbPvAS5lNUdAKy8siekMkPnTgBTdGJWz81 Jviog60m1hARoAl2O+Nu9F5p77d9KaI+UZsfvi7ON7Wj+ljqfjtAju9rt09zznHqnhqM3EZGPVj WzmCRs3dsGbn/ssG9+kw0ARTfCRy/avNDT1Ob3REcK+9Y= X-Gm-Gg: ASbGncvBUk2RPD7/s3EwAK7mABiQ1Zvj79TxRc0YpEc/o8edOmrWYlDOj1lRvmXqnFe Wu+ahDmD8U2AskAWCivpXOZziIg4VTKYkBcKUxYq9Jy+5XjyT44Pg4yK3do2lrpi00Dn8UD6k79 BaVM8+FutWwL5Jyekcl4H6xYQMJkRn8frYKTZ7Sl3qLFUkudW/+WcYnoCfAgfKfMR82ebkW7U+r 5/aVb9VR+F06RyvnJSNTu7cPPbE0K9qG17TszdgkQdIO6twJiVhwr6LXicyQGfxRu9VDXo3kt5y aA== X-Received: by 2002:a05:6000:1fab:b0:385:f9db:3c4c with SMTP id ffacd0b85a97d-385fd3f237cmr1773691f8f.9.1733222129860; Tue, 03 Dec 2024 02:35:29 -0800 (PST) X-Google-Smtp-Source: AGHT+IHXzhsJnpHPdZsXBMryGg2zgUf51shKubv+qQ4qOTyzreFCcjsKa173/wpZn0K+WE7TJadYyA== X-Received: by 2002:a05:6000:1fab:b0:385:f9db:3c4c with SMTP id ffacd0b85a97d-385fd3f237cmr1773651f8f.9.1733222129280; Tue, 03 Dec 2024 02:35:29 -0800 (PST) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-385e940fef3sm8746294f8f.42.2024.12.03.02.35.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 02:35:28 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Hannes Domani Subject: [PATCHv3 3/5] gdb: restructure disable_breakpoints_in_unloaded_shlib Date: Tue, 3 Dec 2024 10:35:20 +0000 Message-Id: <67b44fedfdf5587beee7180955e422a32e86093a.1733221992.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-MFC-PROC-ID: 2rZvVaTG6lVRJh9PveaXMOphSoZLWsfTd-xFPtu4EIs_1733222132 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.2 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_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, URIBL_BLOCKED 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org This commit rewrites disable_breakpoints_in_unloaded_shlib to be more like disable_breakpoints_in_freed_objfile. Instead of looping over all b/p locations, we instead loop over all b/p and then over all locations for each b/p. The advantage of doing this is that we can fix the small bug that was documented in a comment in the code: /* This may cause duplicate notifications for the same breakpoint. */ notify_breakpoint_modified (b); By calling notify_breakpoint_modified() as we modify each location we can potentially send multiple notifications for a single b/p. Is this a bug? Maybe not. After all, at each notification one of the locations will have changed, so its probably fine. But it's not ideal, and we can easily do better, so lets do that. There's a new test which checks that we only get a single notification when the shared library is unloaded. Note that the test is written as if there are multiple related but different tests within the same test file ... but there aren't currently! The next commit will add another test proc to this test script at which point the comments will make sense. I've done this to avoid unnecessary churn in the next commit. Tested-By: Hannes Domani --- gdb/breakpoint.c | 45 +++++---- gdb/testsuite/gdb.base/shlib-unload-lib.c | 30 ++++++ gdb/testsuite/gdb.base/shlib-unload.c | 63 ++++++++++++ gdb/testsuite/gdb.base/shlib-unload.exp | 114 ++++++++++++++++++++++ gdb/testsuite/gdb.base/shlib-unload.h | 26 +++++ gdb/testsuite/gdb.base/shlib-unload.py | 31 ++++++ 6 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 gdb/testsuite/gdb.base/shlib-unload-lib.c create mode 100644 gdb/testsuite/gdb.base/shlib-unload.c create mode 100644 gdb/testsuite/gdb.base/shlib-unload.exp create mode 100644 gdb/testsuite/gdb.base/shlib-unload.h create mode 100644 gdb/testsuite/gdb.base/shlib-unload.py diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 7b84126c241..bca322172e0 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8078,29 +8078,37 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib { bool disabled_shlib_breaks = false; - for (bp_location *loc : all_bp_locations ()) + for (breakpoint &b : all_breakpoints ()) { - /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL. */ - struct breakpoint *b = loc->owner; + bool bp_modified = false; - if (pspace == loc->pspace - && !loc->shlib_disabled - && (((b->type == bp_breakpoint - || b->type == bp_jit_event - || b->type == bp_hardware_breakpoint) - && (loc->loc_type == bp_loc_hardware_breakpoint - || loc->loc_type == bp_loc_software_breakpoint)) - || is_tracepoint (b)) - && solib_contains_address_p (solib, loc->address)) + if (b.type != bp_breakpoint + && b.type != bp_jit_event + && b.type != bp_hardware_breakpoint + && !is_tracepoint (&b)) + continue; + + for (bp_location &loc : b.locations ()) { - loc->shlib_disabled = 1; + if (pspace != loc.pspace || loc.shlib_disabled) + continue; + + if (loc.loc_type != bp_loc_hardware_breakpoint + && loc.loc_type != bp_loc_software_breakpoint + && !is_tracepoint (&b)) + continue; + + if (!solib_contains_address_p (solib, loc.address)) + continue; + + loc.shlib_disabled = 1; + /* At this point, we cannot rely on remove_breakpoint succeeding so we must mark the breakpoint as not inserted to prevent future errors occurring in remove_breakpoints. */ - loc->inserted = 0; + loc.inserted = 0; - /* This may cause duplicate notifications for the same breakpoint. */ - notify_breakpoint_modified (b); + bp_modified = true; if (!disabled_shlib_breaks) { @@ -8108,9 +8116,12 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib warning (_("Temporarily disabling breakpoints " "for unloaded shared library \"%s\""), solib.so_name.c_str ()); + disabled_shlib_breaks = true; } - disabled_shlib_breaks = true; } + + if (bp_modified) + notify_breakpoint_modified (&b); } } diff --git a/gdb/testsuite/gdb.base/shlib-unload-lib.c b/gdb/testsuite/gdb.base/shlib-unload-lib.c new file mode 100644 index 00000000000..c38715f7ac9 --- /dev/null +++ b/gdb/testsuite/gdb.base/shlib-unload-lib.c @@ -0,0 +1,30 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 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 "shlib-unload.h" + +int +foo (void) +{ + return inline_func (); +} + +int +bar (void) +{ + return inline_func (); +} diff --git a/gdb/testsuite/gdb.base/shlib-unload.c b/gdb/testsuite/gdb.base/shlib-unload.c new file mode 100644 index 00000000000..4a3cb5d8b5d --- /dev/null +++ b/gdb/testsuite/gdb.base/shlib-unload.c @@ -0,0 +1,63 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 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 + +#ifdef __WIN32__ +#include +#define dlopen(name, mode) LoadLibrary (TEXT (name)) +#ifdef _WIN32_WCE +# define dlsym(handle, func) GetProcAddress (handle, TEXT (func)) +#else +# define dlsym(handle, func) GetProcAddress (handle, func) +#endif +#define dlclose(handle) FreeLibrary (handle) +#else +#include +#endif + +#include + +#include "shlib-unload.h" + +int +main (void) +{ + int res; + void *handle; + int (*func) (void); + + int val = inline_func (); + + handle = dlopen (SHLIB_NAME, RTLD_LAZY); + assert (handle != NULL); + + func = (int (*)(void)) dlsym (handle, "foo"); + assert (func != NULL); + + val += func (); + + func = (int (*)(void)) dlsym (handle, "bar"); + assert (func != NULL); + + val += func (); + + res = dlclose (handle); /* Break here. */ + assert (res == 0); + + return val; +} diff --git a/gdb/testsuite/gdb.base/shlib-unload.exp b/gdb/testsuite/gdb.base/shlib-unload.exp new file mode 100644 index 00000000000..791f9f518e8 --- /dev/null +++ b/gdb/testsuite/gdb.base/shlib-unload.exp @@ -0,0 +1,114 @@ +# Copyright 2024 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 for GDB's handling of a shared library being unloaded via a +# call to dlclose. See the individual test_* procs for a description +# of each test. + +standard_testfile .c -lib.c + +# One of the tests uses this Python file. The test_* proc checks that +# GDB supports Python tests. Some of the other procs don't use this +# Python file. +set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py] + +# Build the library and copy it to the target. +set libname ${testfile}-lib +set libfile [standard_output_file $libname] +if { [build_executable "build shlib" $libfile $srcfile2 {debug shlib}] == -1} { + return +} +set libfile_on_target [gdb_download_shlib $libfile] + +# Build the executable. +set opts [list debug shlib_load additional_flags=-DSHLIB_NAME=\"${libname}\"] +if { [build_executable "build exec" $binfile $srcfile $opts] == -1} { + return +} + +# The line number of the dlclose call. +set bp_line [gdb_get_line_number "Break here" $srcfile] + +# If the target is remote, then the library name in the bp_disabled_re +# below will have a 'target:' prefix. +if {[is_remote target]} { + set target_prefix_re "target:" +} else { + set target_prefix_re "" +} + +# The line emitted when GDB disables breakpoints after unloading a +# shared library. +set bp_disabled_re "warning: Temporarily disabling breakpoints for unloaded shared library \"$target_prefix_re[string_to_regexp $::libfile_on_target]\"" + +# The complete regexp for when GDB stops on the line after BP_LINE, +# assuming that GDB has disabled some breakpoints. +set stop_after_bp_re [multi_line \ + "^$::bp_disabled_re" \ + "[expr $::bp_line + 1]\\s+assert \\(res == 0\\);"] + +# Checking that a breakpoint with multiple locations in a shared +# library only triggers a single breakpoint modified event from +# disable_breakpoints_in_unloaded_shlib when the shared library is +# unloaded. +proc_with_prefix test_bp_modified_events {} { + if { ![allow_python_tests] } { + unsupported "python support needed" + return + } + + clean_restart $::binfile + + if {![runto_main]} { + return + } + + # If the debug information doesn't allow GDB to identify inline + # functions then this test isn't going to work. + get_debug_format + if { [skip_inline_frame_tests] } { + unsupported "skipping inline frame tests" + return + } + + gdb_breakpoint $::srcfile:$::bp_line + gdb_continue_to_breakpoint "stop before dlclose" + + gdb_breakpoint inline_func + set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get b/p number"] + + gdb_test_no_output "source $::pyfile" "import python scripts" + + gdb_test "next" $::stop_after_bp_re + + # The breakpoint should have been modified once when some of its + # locations are made pending after the shared library is unloaded. + gdb_test_multiple "python print(bp_modified_counts\[$bp_num\])" "" { + -re -wrap "^1" { + pass $gdb_test_name + } + -re -wrap "^2" { + # A second event occurs when the pending breakpoint is + # incorrectly deleted. + kfail gdb/32404 $gdb_test_name + } + -re -wrap "^$::decimal" { + fail $gdb_test_name + } + } +} + +test_bp_modified_events diff --git a/gdb/testsuite/gdb.base/shlib-unload.h b/gdb/testsuite/gdb.base/shlib-unload.h new file mode 100644 index 00000000000..3448eed5cb9 --- /dev/null +++ b/gdb/testsuite/gdb.base/shlib-unload.h @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 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 . */ + +static inline int __attribute__((__always_inline__)) +inline_func () +{ + return 0; +} + +/* Two library functions. */ +extern int foo (void); +extern int bar (void); diff --git a/gdb/testsuite/gdb.base/shlib-unload.py b/gdb/testsuite/gdb.base/shlib-unload.py new file mode 100644 index 00000000000..eb541fd53fd --- /dev/null +++ b/gdb/testsuite/gdb.base/shlib-unload.py @@ -0,0 +1,31 @@ +# Copyright (C) 2024 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 . + +# Breakpoint modification events will be recorded in this dictionary. +# The keys are the b/p numbers, and the values are the number of +# modification events seen. +bp_modified_counts = {} + +# Record breakpoint modification events into the global +# bp_modified_counts dictionary. +def bp_modified(bp): + global bp_modified_counts + if bp.number not in bp_modified_counts: + bp_modified_counts[bp.number] = 1 + else: + bp_modified_counts[bp.number] += 1 + +# Register the event handler. +gdb.events.breakpoint_modified.connect(bp_modified) From patchwork Tue Dec 3 10:35:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 102296 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 7DCE23858C78 for ; Tue, 3 Dec 2024 10:39:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7DCE23858C78 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Ny0EFba4 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 ESMTP id 311473858C62 for ; Tue, 3 Dec 2024 10:35:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 311473858C62 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 311473858C62 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222137; cv=none; b=iKDDpk5km6xO3e8tsekPSMrDe0iiGBYNBgIxeK1QOZmhRJYgSmc7VX1dldImkV1q1kGU4bqydghPe/uyGHKKNKRM20MHdfGrHrgKLByMW8Hfmdcc7DS0Lz8LrI1TPjEZu+tq3Y/Q6C73+O/mTdcrE5WyFeRcHlAJz+nCv3H4shE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222137; c=relaxed/simple; bh=F4XhwnZPFnnnBivvHCBBlu4drJq5zMgjN93xM4/A1Dc=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=gvibMTGSxJI97XlTuLboIsXGYwOnxFFI33zBXn/vgr1VNSCxu6zspPqjVALWXjBqRz2n7nWkwcm6oodN4j1aeoPty5rrLSkpV1CZbIsilbDlTF1R5RAkP8sXUBP4eSzwEgFPrdR3Hz9ful5LQp5ZFsUdUakUZio49qxynYGkEdc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 311473858C62 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733222136; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=niDzABV72YtJhP/1Z000EGR+pII6LuePo8/WoLA9BUo=; b=Ny0EFba4Oh0UuoZ5pznUTPNMVB32lueEw3rCwL6MTKZKuRy1nKkxu8h+d1eNAvEC/aP3vq FJqV8SvDDOFxXb1wbfC5gHrEAfZg8AASysMR3mtqMHjTbCXtbhIabKNELDJjXio/QXbcsB qYfeuWB37EapwloGjPMTcBqrpM0a2QQ= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-690-JTgX4zW_PVavny4VBGRkcw-1; Tue, 03 Dec 2024 05:35:35 -0500 X-MC-Unique: JTgX4zW_PVavny4VBGRkcw-1 X-Mimecast-MFC-AGG-ID: JTgX4zW_PVavny4VBGRkcw Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4349f6b337dso35051985e9.3 for ; Tue, 03 Dec 2024 02:35:35 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733222134; x=1733826934; 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=niDzABV72YtJhP/1Z000EGR+pII6LuePo8/WoLA9BUo=; b=N74jpHp/g/7AzU9R6VkuOl0/baIqpLfV4EcgKTyZqr0X82tGeLx7TcmsFp+qD++828 dvwP+WDKUGbK5z2fhM+Lb0R6n5f7sZU/v9iTuPy70Qx5d/Pdf/fdzAsyObaPZXCoe9N4 Y5pF0lKH3kYxdgbPhJXoy8UdY2P8vrENDJeu5TQawU4/bPkVwraoW7SygHweVbnw6UAu stV3eYHXgUZeGAwViEELcR89i8taTJvSJK7XO/8xBY12tjPlbD+qjGYI0FTQPaOXLzbh i4djTTj8uQ9ABj+FO78v93udAgZPsfvjpmouxUUey+yOTe9OJP67XPhFuw8NAtTll8Nk EfOQ== X-Gm-Message-State: AOJu0Yym4uNB6KpqcRVFgfS209Ip/bHno1Jkth+l8VIpGSB9qkwRWIiB PYYcOq18V7erMNBtcmzVJsmEDWXY9K0hEA/pxsOXUoJrf/mw8UYyprg97HfzbYImNLwJfcYVB91 tnvkE2oywQVHJ2v9DQK12bmIbiBLMn6KolEFkt7Bqc4tB1ZRGcGAlkcPAon7qEfX0peaoSyhHDr Z7e8Vjl8DO6c2oOIeIHcGahkyjaYfVDS/B5eIqHz1RxAA= X-Gm-Gg: ASbGncsktHdYzNemDKNBeQ823K/7beNL66i4zLWwnalRMWO0nTCnOpfxNtnvBoA1W18 luz4ARXv3YBdml5igXj6JOExM2Z46NVOKtH375reGum2LO2919yL+OUYHmrdeDhTFp87V8hwqu5 Kqs7v4LfTRMNbHK2JUcRKLuIU2HKc3nEv0gKe4CEX6Gk/rBQqC20i42vzBN/QcysG52cYU5tLwj PPNP1gC8Mw4k8S3ZW2lOuPlewrXhlceFrfOrf3gZRMSvfJnREMgvpClt+N9LW1IXXQZyBmoPVuA Wg== X-Received: by 2002:a7b:cb49:0:b0:434:a663:a59d with SMTP id 5b1f17b1804b1-434d28e51a5mr5510665e9.32.1733222133780; Tue, 03 Dec 2024 02:35:33 -0800 (PST) X-Google-Smtp-Source: AGHT+IG2NSbRA9Ph07y15aW9dHUOjDKaG9dG7gfPGJQSOxU7BbdxyxZ6mResfmyvbwV1bRNt1yQydw== X-Received: by 2002:a7b:cb49:0:b0:434:a663:a59d with SMTP id 5b1f17b1804b1-434d28e51a5mr5510455e9.32.1733222133274; Tue, 03 Dec 2024 02:35:33 -0800 (PST) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434aa763aaesm215042085e9.14.2024.12.03.02.35.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 02:35:30 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Hannes Domani Subject: [PATCHv3 4/5] gdb: handle dprintf breakpoints when unloading a shared library Date: Tue, 3 Dec 2024 10:35:21 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: ZAzIWZ3IHn2vcODPNSHbcGp8pq1KCzZbJBF7sb2Obxc_1733222135 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.2 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_H3, RCVD_IN_MSPIKE_WL, 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org While working on the previous commit I realised that GDB would not handle dprintf breakpoints correctly when a shared library was unloaded. Consider this example using the test binary from the previous commit, the function 'foo' where we create a dprintf is in a shared library: (gdb) b 59 Breakpoint 1 at 0x401215: file /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload.c, line 59. (gdb) r Starting program: /tmp/projects/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/shlib-unload/shlib-unload Breakpoint 1, main () at /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload.c:59 59 res = dlclose (handle); /* Break here. */ (gdb) dprintf foo,"In foo" Dprintf 2 at 0x7ffff7fc50fd: file /tmp/projects/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/shlib-unload-lib.c, line 23. (gdb) n Error in re-setting breakpoint 2: Function "foo" not defined. warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd warning: error removing breakpoint 2 at 0x7ffff7fc50fd Cannot remove breakpoints because program is no longer writable. Further execution is probably impossible. 60 assert (res == 0); (gdb) What happens here is that as the inferior steps over the dlclose call the shared library containing 'foo' is unloaded and disable_breakpoints_in_unloaded_shlib is called. However in disable_breakpoints_in_unloaded_shlib we have this check: if (b.type != bp_breakpoint && b.type != bp_jit_event && b.type != bp_hardware_breakpoint && !is_tracepoint (&b)) continue; As the dprintf has type bp_dprintf then this check triggers and we ignore the dprintf, meaning the dprintf is not disabled. When the inferior stops after the 'next' GDB tries to remove all breakpoints but the dprintf can no longer be removed, the memory in which it was placed has been unmapped from the inferior. The fix is to start using is_breakpoint() in disable_breakpoints_in_unloaded_shlib instead of the bp_breakpoint and bp_hardware_breakpoint checks. The is_breakpoint() function also checks for bp_dprintf. With this fix in place GDB now correctly disables the breakpoint and we no longer see the warning about removing the breakpoint. Tested-By: Hannes Domani --- gdb/breakpoint.c | 5 ++--- gdb/testsuite/gdb.base/shlib-unload.exp | 29 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index bca322172e0..d0f4cbe18a9 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8082,9 +8082,8 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib { bool bp_modified = false; - if (b.type != bp_breakpoint - && b.type != bp_jit_event - && b.type != bp_hardware_breakpoint + if (b.type != bp_jit_event + && !is_breakpoint (&b) && !is_tracepoint (&b)) continue; diff --git a/gdb/testsuite/gdb.base/shlib-unload.exp b/gdb/testsuite/gdb.base/shlib-unload.exp index 791f9f518e8..7516ee2a292 100644 --- a/gdb/testsuite/gdb.base/shlib-unload.exp +++ b/gdb/testsuite/gdb.base/shlib-unload.exp @@ -111,4 +111,33 @@ proc_with_prefix test_bp_modified_events {} { } } +# Check that GDB disables dprintf breakpoints within a shared library +# when the shared library is unloaded. +proc_with_prefix test_dprintf_at_unload {} { + clean_restart $::binfile + + if {![runto_main]} { + return + } + + gdb_breakpoint $::srcfile:$::bp_line + gdb_continue_to_breakpoint "stop before dlclose" + + # Setup a dprintf within the shared library. + gdb_test "dprintf foo,\"In foo\"" + set bp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*" \ + "get b/p number"] + + # Unload the shared library, GDB should disable our b/p. + gdb_test "next" $::stop_after_bp_re + + # Check that our b/p is now showing as disabled. + gdb_test "info breakpoints $bp_num" \ + [multi_line \ + "^Num\\s+Type\\s+Disp\\s+Enb\\s+Address\\s+What" \ + "$bp_num\\s+dprintf\\s+keep\\s+y\\s+\\s+foo" \ + "\\s+printf \"In foo\""] +} + test_bp_modified_events +test_dprintf_at_unload From patchwork Tue Dec 3 10:35:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 102298 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 1DDC73858C56 for ; Tue, 3 Dec 2024 10:41:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1DDC73858C56 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=D8xnViLW 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 ESMTP id AC0A53858C2B for ; Tue, 3 Dec 2024 10:35:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AC0A53858C2B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AC0A53858C2B Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222139; cv=none; b=McHW0YOqhhnMqCp279cUzya+GxyBLgBo6E3VRclVY792qjgrN0jLAJK1bRCj9sxdXgHTt/oxG3eVMbPbGLsvbSHe/wjeXbowB9kEqabiyGOEWc3T6P83Uga4H4iykko48PEhhVpwBJ1cZv8wZMQbhtz75WXbYaSYsgRMjF4IPxM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733222139; c=relaxed/simple; bh=yrp+w1Q3VsKEG47aIpq4CSw8jzc3rJ4idRvV9MmH4VQ=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=QphGWtduyyZFjzG+LaFCK1F0xwrnn/jcqCBaWm7CA3y9tslvy80P3HIqUpFabpvARmU+TaD+/s3cCyOjSWcpLo/FaOMwJjluWuSi+N3i2jew1i5iIq42YYdZ5e3Im0pdzX95WcosDjtK7d+bPSbSAwxellIXpxr7GxGuU3hkFCU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AC0A53858C2B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733222139; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fZAYuddCAodCwm3M2nSDVe+Fnu7gX6Ne32LCdJSPOZ8=; b=D8xnViLWbDcBVKon6yGPC0AkMm45BRELKoDeTSvd13P6chxSu8nE6g79Xjv4M7bQitjLhk bpdMPAFgkH4zir9Y0WZDsvf+WlweJWCGQjghZf+t/fHZEPJg2jvZ4eU/9K/Cb4PCcFCnnw BrW8gnbtACU58fOPHLjPRzHcG7CTtvo= 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_256_GCM_SHA384) id us-mta-404-33OWYBJAODabuplcAmEfpg-1; Tue, 03 Dec 2024 05:35:38 -0500 X-MC-Unique: 33OWYBJAODabuplcAmEfpg-1 X-Mimecast-MFC-AGG-ID: 33OWYBJAODabuplcAmEfpg Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-434a29a93cdso39953945e9.1 for ; Tue, 03 Dec 2024 02:35:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733222136; x=1733826936; 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=fZAYuddCAodCwm3M2nSDVe+Fnu7gX6Ne32LCdJSPOZ8=; b=VjeoAhzhBHVxuWpFeb7m5VANvFro8FqC/t3PN3ToFjNsSE+I3Awhkd5D3jChjzE3kg Hy+SinPCNuYU9UztvFLmgABp9ajGd9zjZ8pgFOP2/o52qK6EXufk9uNerlqoJ2T13P5v C+NO/MKx3UXLs303HC300BAjzEdqZURD12xi4wsTlN9zG7cKsTAV9012i7Pwb7CUmNIE YP5hbGNAR0D8mDtuDHG9t87dk/4ev6OpQ1urr9LgbjLoCx0uk6tOO9WGeqoyEv+Fyllk RWOizN2l97kh+syGYo2uG1c+mG/fCBbQ7NF8A4UL9pkg+I1+BlqlEp0C8mrgs577VBsB l+cw== X-Gm-Message-State: AOJu0Yz6TX9HF/MfuYIkKMvvET1SMS4OHqDOPtZiRMNmPalNEMTNCb6S T+h5X86vjFyI7fzCpTz6PUUz5T+Icngf3saJlnernbP11jXNgOrRCQ4+soezi09PT+mtPnSMjsl R7Cmi6e9a1fm7f75j1GZJPvxxeMD8pGrq/L6nuRAdIkpE10zhWn7nd4B8rhBFVHKRBUXhxblsNe kTyKS4LPSBcDwXfD8YM8IKamhGUuJ+Sa+13W1Melog5t4= X-Gm-Gg: ASbGncvW7AmGw6DSkxiKzantHHqIIDmDDrpwr8PjzpXMyjeuWLIxKN+dYfLkcvNJUSg VYmt9942fK7WaJwo7IdeZ89s9pki+81oPl71Zp59ECwxhSFzrNlszMF0E7fkbCO1XB+tyJfqoqU RwBw/4My+rBCBz12UlE0QzdnceHlAzcF/LM+tr3jomAyf1X1AdwH3Q2UNiuqBPPm7bxTPX9B0xQ 6bU/IvcscYbQlWfbJerqoIPw1J6uSB0MnuwNESRgELh6OLctubg2T9Ku+0D+lVzfswFshji1Neg tA== X-Received: by 2002:a05:600c:3aca:b0:42c:b187:bde9 with SMTP id 5b1f17b1804b1-434d0a23a76mr18323675e9.30.1733222135785; Tue, 03 Dec 2024 02:35:35 -0800 (PST) X-Google-Smtp-Source: AGHT+IHXyZGdvlKVOHW9e97WaTVZU0YLbQtrylu1dMlWj3DUwLWadD/6/CJaWgQeI6FV38tGK/xJgw== X-Received: by 2002:a05:600c:3aca:b0:42c:b187:bde9 with SMTP id 5b1f17b1804b1-434d0a23a76mr18323205e9.30.1733222135201; Tue, 03 Dec 2024 02:35:35 -0800 (PST) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-434aa7e5e59sm219663575e9.44.2024.12.03.02.35.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 02:35:34 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Hannes Domani Subject: [PATCHv3 5/5] gdb: disable internal b/p when a solib is unloaded Date: Tue, 3 Dec 2024 10:35:22 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: y2YkhHggOJKjzor5E1kUygUrqM4F-wTbcjbCxaV_FfY_1733222137 X-Mimecast-Originator: redhat.com content-type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-13.2 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_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, URIBL_BLOCKED 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org Bug PR gdb/32079 highlights an issue where GDB will try to remove a breakpoint for a shared library that has been unloaded. This will trigger an error from GDB like: (gdb) next 61 dlclose (handle[dl]); (gdb) next warning: error removing breakpoint 0 at 0x7ffff78169b9 warning: error removing breakpoint 0 at 0x7ffff7730b57 warning: error removing breakpoint 0 at 0x7ffff7730ad3 54 for (dl = 0; dl < 4; ++dl) (gdb) What happens is that as the inferior steps over the dlclose() call GDB notices that the library has been unloaded and calls disable_breakpoints_in_unloaded_shlib. However, this function only operates on user breakpoints and tracepoints. In the example above what is happening is that the test loads multiple copies of libc into different linker namespsaces. When we 'next' over the dlclose call one of the copies of libc is unloaded. As GDB placed longjmp master breakpoints within the copy of libc that was just unloaded, the warnings we see are GDB trying (and failing) to remove these breakpoints. I think the solution is for disable_breakpoints_in_unloaded_shlib to handle all breakpoints, even internal ones like the longjmp master breakpoints. If we do this then the breakpoint will be marked as shlib_disabled and also will be marked as not inserted. Later when we call breakpoint_re_set() and the longjmp breakpoints are deleted we will no longer try to remove them. This solution is inspired by a patch suggested in the bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=32079#c3 There are some differences with my approach compared to the patch suggested in the bug. First I have no need to delete the breakpoint inside disable_breakpoints_in_unloaded_shlib as an earlier patch in this series arranged for breakpoint_re_set to be called when shared libraries are removed. Calling breakpoint_re_set will take care of deleting the breakpoint for us. For details see the earlier commit titled: gdb: fixes for code_breakpoint::disabled_by_cond logic Next, rather than only handling bp_longjmp and bp_longjmp_master, I allow all breakpoints to be handled. I also only give the warning about disabling breakpoints for user breakpoints, I don't see the point of warning the user about internal b/p changes. The biggest change though is that I've added some code to detect shared libraries that appear to be mapped multiple times. Here's the 'info sharedlibrary' output for the binary for gdb.base/dlmopen.exp after all the shared libraries have been loaded: 61 dlclose (handle[dl]); (gdb) info sharedlibrary From To Syms Read Shared Object Library 0x00007ffff7fca000 0x00007ffff7ff03f5 Yes /lib64/ld-linux-x86-64.so.2 0x00007ffff7edb3d0 0x00007ffff7f4f898 Yes (*) /lib64/libm.so.6 0x00007ffff7d0f800 0x00007ffff7e6eacd Yes (*) /lib64/libc.so.6 0x00007ffff7fb8040 0x00007ffff7fb80f9 Yes /tmp/build/gdb/testsuite/outputs/gdb.base/dlmopen/dlmopen-lib-dep.so 0x00007ffff7c003d0 0x00007ffff7c74898 Yes (*) /lib64/libm.so.6 0x00007ffff7a34800 0x00007ffff7b93acd Yes (*) /lib64/libc.so.6 0x00007ffff7fca000 0x00007ffff7ff03f5 Yes /lib64/ld-linux-x86-64.so.2 0x00007ffff7ce2040 0x00007ffff7ce2116 Yes /tmp/build/gdb/testsuite/outputs/gdb.base/dlmopen/dlmopen-lib.1.so 0x00007ffff7cdd040 0x00007ffff7cdd0f9 Yes /tmp/build/gdb/testsuite/outputs/gdb.base/dlmopen/dlmopen-lib-dep.so 0x00007ffff79283d0 0x00007ffff799c898 Yes (*) /lib64/libm.so.6 0x00007ffff775c800 0x00007ffff78bbacd Yes (*) /lib64/libc.so.6 0x00007ffff7fca000 0x00007ffff7ff03f5 Yes /lib64/ld-linux-x86-64.so.2 0x00007ffff7cd8040 0x00007ffff7cd8116 Yes /tmp/build/gdb/testsuite/outputs/gdb.base/dlmopen/dlmopen-lib.2.so (*): Shared library is missing debugging information. (gdb) The important thing to spot here is /lib64/ld-linux-x86-64.so.2. This library appears 3 times in the list but is at the same address in each case. This indicates three copies of this library mapped into three different linker namespaces, but the linker has spotted a single instance of the library can be shared. When the inferior calls dlclose() GDB will still see an event for this library being unloaded. If we disable all the b/p within this library on the first event then we end up leaving software b/p in the code as the library isn't really unmapped at this point. What we need to do is spot that the library is mapped multiple times and only perform the b/p disable logic if this is the last instance of the library. With this done the issues in PR gdb/32079 are resolved. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32079 Tested-By: Hannes Domani --- gdb/breakpoint.c | 39 ++++++++++++--- gdb/testsuite/gdb.base/dlmopen.exp | 76 ++++++++++++++++++++++++------ 2 files changed, 95 insertions(+), 20 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index d0f4cbe18a9..e61da730e00 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8076,17 +8076,44 @@ disable_breakpoints_in_shlibs (program_space *pspace) static void disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib) { + /* A shared library can appear twice on the runtime-linkers shared + library list, but the shared library is actually mapped in only once. + This can happen when linker namespaces are used. When one of those + mappings is removed this function is called. If we then disable and + mark as removed all the b/p within the library then we'll end up + leaving software breakpoints in the code (as the library was never + actually unmapped). + + To prevent this, we check the shared library list. If we find some + other shared library with the same objfile then we can leave the + breakpoints alone. + + If a shared library is mapped a second time, but at a different + address, then the shared library will be given a new objfile. */ + for (const struct solib &so : pspace->solibs ()) + { + if (&solib == &so) + continue; + + /* SOLIB and SO are two different solib objects, but, if they share + the same objfile then there is really only a single mapped + instance of the library. + + If SOLIB has no objfile then it will have no target sections (as + target sections are derived from the objfile) and so the + solib_contains_address_p call below will always return false, thus + we don't need to worry about the case where neither SOLIB and SO + have no objfile. */ + if (solib.objfile != nullptr && solib.objfile == so.objfile) + return; + } + bool disabled_shlib_breaks = false; for (breakpoint &b : all_breakpoints ()) { bool bp_modified = false; - if (b.type != bp_jit_event - && !is_breakpoint (&b) - && !is_tracepoint (&b)) - continue; - for (bp_location &loc : b.locations ()) { if (pspace != loc.pspace || loc.shlib_disabled) @@ -8109,7 +8136,7 @@ disable_breakpoints_in_unloaded_shlib (program_space *pspace, const solib &solib bp_modified = true; - if (!disabled_shlib_breaks) + if (!disabled_shlib_breaks && user_breakpoint_p (&b)) { target_terminal::ours_for_output (); warning (_("Temporarily disabling breakpoints " diff --git a/gdb/testsuite/gdb.base/dlmopen.exp b/gdb/testsuite/gdb.base/dlmopen.exp index 264c5180f3f..f0a29280d8b 100644 --- a/gdb/testsuite/gdb.base/dlmopen.exp +++ b/gdb/testsuite/gdb.base/dlmopen.exp @@ -144,27 +144,75 @@ gdb_breakpoint $srcfile:$bp_main test_dlmopen # Try the same again when attaching after dlmopen(). -require can_spawn_for_attach +if { [can_spawn_for_attach] } { -clean_restart $binfile + clean_restart $binfile -# Start the test program. -set test_spawn_id [spawn_wait_for_attach $binfile] -set testpid [spawn_id_get_pid $test_spawn_id] + # Start the test program. + set test_spawn_id [spawn_wait_for_attach $binfile] + set testpid [spawn_id_get_pid $test_spawn_id] -# Attach. -if { ![gdb_attach $testpid] } { - return + # Attach. + if { ![gdb_attach $testpid] } { + return + } + + with_test_prefix "attach" { + # Remove the pause. We no longer need it. + gdb_test "print wait_for_gdb = 0" "\\\$1 = 0" + + # Set the same breakpoints again. This time, however, we do not allow the + # breakpoint to be pending since the library has already been loaded. + gdb_breakpoint $srcfile_lib:$bp_inc + gdb_breakpoint $srcfile:$bp_main + + test_dlmopen + } } -with_test_prefix "attach" { +# Check that we can 'next' over the dlclose calls without GDB giving any +# warnings or errors. +with_test_prefix "next over dlclose" { + clean_restart $binfile + + if { ![runto_main] } { + return + } + + set dlclose_lineno [gdb_get_line_number "dlclose" $srcfile] + gdb_breakpoint $srcfile:$dlclose_lineno + gdb_breakpoint $srcfile:$bp_main + # Remove the pause. We no longer need it. gdb_test "print wait_for_gdb = 0" "\\\$1 = 0" - # Set the same breakpoints again. This time, however, we do not allow the - # breakpoint to be pending since the library has already been loaded. - gdb_breakpoint $srcfile_lib:$bp_inc - gdb_breakpoint $srcfile:$bp_main + set loc_re [multi_line \ + "\[^\r\n\]+/[string_to_regexp $srcfile]:$dlclose_lineno" \ + "$dlclose_lineno\\s+dlclose \[^\r\n\]+"] + + # This loop mirrors the loop in dlmopen.c where the inferior performs + # four calls to dlclose. Here we continue to the dlclose, and then use + # 'next' to step over the call. As part of the 'next' GDB will insert + # breakpoints to catch longjmps into the multiple copies of libc which + # have been loaded. Each dlclose call will cause a copy of libc to be + # unloaded, which should disable the longjmp breakpoint that GDB + # previously inserted. + # + # At one point a bug in GDB meant that we failed to correctly disable + # the longjmp breakpoints and GDB would try to remove the breakpoint + # from a library after it had been unloaded, which would trigger a + # warning. + for { set i 0 } { $i < 4 } { incr i } { + gdb_continue_to_breakpoint "continue to dlclose $i" $loc_re + gdb_test "next" "^$decimal\\s+for \\(dl = 0;\[^\r\n\]+\\)" \ + "next over dlclose $i" + } - test_dlmopen + # Just to confirm that we are where we think we are, continue to the + # final 'return' line in main. If this isn't hit then we likely went + # wrong earlier. + gdb_continue_to_breakpoint "continue to final return" \ + [multi_line \ + "\[^\r\n\]+/[string_to_regexp $srcfile]:$bp_main" \ + "$bp_main\\s+return 0;\[^\r\n\]+"] }