[PATCH 2/3] gdb/testsuite: avoid incorrect symbols in gdb.base/condbreak-multi-context.cc

[Andrew Burgess <aburgess@redhat.com>]

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   <MULTIPLE>
  	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   <MULTIPLE>
  	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.
---
 .../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+<MULTIPLE>\\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 <cond>" syntax.  Expect that the
 # locations where <cond> 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
+}
-- 
2.25.4