From a591084285cc16a435258641691aa0a8f5bf42ee Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 27 Jan 2022 15:12:25 +0000 Subject: [PATCH] gdb: test to check one aspect of the linespec parsing code While working on the fix for PR cli/28665 (see previous couple of commits), I was playing with making a change in the linespec parsing code. Specifically, I was thinking about whether the spec_string for LINESPEC_LOCATION locations should ever be nullptr. I made a change to prevent the spec_string from ever being nullptr, tested gdb, and saw no regressions. However, as part of this work I was reviewing how the breakpoint code handles this case (spec_string being nullptr), and spotted that in parse_breakpoint_sals the nullptr case is specifically handled, so changing this should have caused a regression. But I didn't see one. So, this commit adds a comment in location.c mentioning that the nullptr case is (a) not an oversight, and (b) is required. Then I add a new test to gdb.base/break.exp that ensures a change in this area will cause a regression. This test passes on current gdb, but with my modified (and broken) gdb, the test would fail. --- gdb/location.c | 5 +++++ gdb/testsuite/gdb.base/break.exp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/gdb/location.c b/gdb/location.c index 299ef7ecadd..0459980ab8c 100644 --- a/gdb/location.c +++ b/gdb/location.c @@ -134,6 +134,11 @@ struct event_location_linespec : public event_location linespec_lex_to_end (linespec); p = remove_trailing_whitespace (orig, *linespec); + + /* If there is no valid linespec then this will leave the + spec_string as nullptr. This behaviour is relied on in the + breakpoint setting code, where spec_string being nullptr means + to use the default breakpoint location. */ if ((p - orig) > 0) linespec_location.spec_string = savestring (orig, p - orig); } diff --git a/gdb/testsuite/gdb.base/break.exp b/gdb/testsuite/gdb.base/break.exp index 39a8f32e888..2c939ada14a 100644 --- a/gdb/testsuite/gdb.base/break.exp +++ b/gdb/testsuite/gdb.base/break.exp @@ -520,6 +520,11 @@ gdb_test "break" \ "Note: breakpoints \[0-9\]*, \[0-9\]* and \[0-9\]* also set at .*Breakpoint \[0-9\]*.*" \ "break on default location, 4th time" +# Check setting a breakpoint at the default location with a condition attached. +gdb_test "break if (1)" \ + "Note: breakpoints \[0-9\]*, \[0-9\]*, \[0-9\]* and \[0-9\]* also set at .*Breakpoint \[0-9\]*.*" \ + "break on the default location, 5th time, but with a condition" + # Verify that a "silent" breakpoint can be set, and that GDB is indeed # "silent" about its triggering. # -- 2.30.2