Re: [PATCH 3/5] gdb/testsuite/: Use -qualified in runto_main / mi_runto_main

[Pedro Alves <pedro@palves.net>]

On 10/13/20 7:06 PM, Simon Marchi wrote:
> On 2020-10-11 8:47 p.m., Pedro Alves wrote:
>> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
>> index 732aed27b27..693c7d2c467 100644
>> --- a/gdb/testsuite/lib/mi-support.exp
>> +++ b/gdb/testsuite/lib/mi-support.exp
>> @@ -1056,7 +1056,7 @@ proc mi_run_to_main { } {
>>  #   -1  if test suppressed, failed, timedout
>>  #    0  if test passed
>>
>> -proc mi_runto_helper {func run_or_continue} {
>> +proc mi_runto_helper {func run_or_continue {qualified 0}} {
> 
> For new options, I'd suggest using the parse_args procedure, instead of
> using arguments with default values.  That makes it nicer when a
> procedure has multiple optional arguments and want to specifiy just one
> (that isn't the first one).

I agree that using parse_args is nice.  I've used it recently
in the MI --qualified patch, and also in a local patch I have
that adds options to Dwarf::assemble.

Here this is an internal routine, just a helper, not
meant to be used as "public" api, so I didn't bother.

But it's not really much trouble.  I've done that change.

> 
> So a call to mi_runto_helper would look like:
> 
>   mi_runto_helper main run -qualified

Below's what I merged.

Yay, we now have:

  "qualified" in gdb_breakpoint/runto,
  "-qualified" in mi_runto_helper, and
  "--qualified" in the actual MI command...

:-)

From 8abd8ee8c878f52469896c716732a974e6abeebe Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Wed, 2 Sep 2020 23:20:45 +0100
Subject: [PATCH] gdb/testsuite/: Use -qualified in runto_main / mi_runto_main

In some runtimes, there may be a "main" function in some class or
namespace.  The breakpoint created by runto_main may therefore have
unexpected locations on some other functions than the actual main.
These breakpoint locations can unexpectedly get hit during tests and
lead to failures.

I saw this while playing with AMD's ROCm toolchain -- I wrote a board
file to run the testsuite against device kernels.  There, the runtime
calls a "main" function before the device kernel code is reached:

 Thread 4 "bit_extract" hit Breakpoint 1, 0x00007ffeea140960 in lld::elf::LinkerDriver::main(llvm::ArrayRef<char const*>) () from /opt/rocm/lib/libamd_comgr.so.1
 (gdb) bt
 #0  0x00007ffeea140960 in lld::elf::LinkerDriver::main(llvm::ArrayRef<char const*>) () from /opt/rocm/lib/libamd_comgr.so.1
 #1  0x00007ffeea2257a5 in lld::elf::link(llvm::ArrayRef<char const*>, bool, llvm::raw_ostream&, llvm::raw_ostream&) () from /opt/rocm/lib/libamd_comgr.so.1
 #2  0x00007ffeea1bc374 in COMGR::linkWithLLD(llvm::ArrayRef<char const*>, llvm::raw_ostream&, llvm::raw_ostream&) () from /opt/rocm/lib/libamd_comgr.so.1
 #3  0x00007ffeea1bfb09 in COMGR::InProcessDriver::execute(llvm::ArrayRef<char const*>) () from /opt/rocm/lib/libamd_comgr.so.1
 #4  0x00007ffeea1c4da9 in COMGR::AMDGPUCompiler::linkToExecutable() () from /opt/rocm/lib/libamd_comgr.so.1
 #5  0x00007ffeea1fde20 in dispatchCompilerAction(amd_comgr_action_kind_s, COMGR::DataAction*, COMGR::DataSet*, COMGR::DataSet*, llvm::raw_ostream&) () from /opt/rocm/lib/libamd_comgr.so.1
 #6  0x00007ffeea203a87 in amd_comgr_do_action () from /opt/rocm/lib/libamd_comgr.so.1
 ...

To avoid that, pass "qualified" to runto, in runto_main, so that
gdb_breakpoint ends up creating a breakpoint with -qualified.  This
avoids creating breakpoints locations for other unrelated "main"
functions.

Note: I first tried making runto itself use "-qualified", but that
caused regressions in the gdb.ada/ tests, which use runto without
specifying the whole fully-qualified function name (i.e., without the
package).  So I end up restricting the -qualified to
runto_main/mi_runto_main.

The gdb.base/ui-redirect.exp change is necessary because that testcase
is looking at what "save breakpoint" generates.

gdb/testsuite/ChangeLog:

	* gdb.base/ui-redirect.exp: Expect "break -qualified main" in
	saved breakpoints file.
	* gdb.guile/scm-breakpoint.exp: Expect "-qualified main" when
	inspecting breakpoint list.
	* lib/gdb.exp (runto_main): Add "qualified" to options.
	* lib/mi-support.exp (mi_runto_helper): Add 'qualified' parameter,
	and handle it.
	(mi_runto_main): Pass 1 as qualified argument.

Change-Id: I51468359ab0a518f05f7c0394c97f7e33b45fe69
---
 gdb/testsuite/ChangeLog                    | 11 +++++++++++
 gdb/testsuite/gdb.base/ui-redirect.exp     |  2 +-
 gdb/testsuite/gdb.guile/scm-breakpoint.exp |  2 +-
 gdb/testsuite/lib/gdb.exp                  |  2 +-
 gdb/testsuite/lib/mi-support.exp           | 16 +++++++++++++---
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 19b4e2b138c..ea6241b77c8 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2020-10-13  Pedro Alves  <pedro@palves.net>
+
+	* gdb.base/ui-redirect.exp: Expect "break -qualified main" in
+	saved breakpoints file.
+	* gdb.guile/scm-breakpoint.exp: Expect "-qualified main" when
+	inspecting breakpoint list.
+	* lib/gdb.exp (runto_main): Add "qualified" to options.
+	* lib/mi-support.exp (mi_runto_helper): Add 'qualified' parameter,
+	and handle it.
+	(mi_runto_main): Pass 1 as qualified argument.
+
 2020-10-13  Pedro Alves  <pedro@palves.net>
 
 	* lib/mi-support.exp (mi_runto_main): New proc.
diff --git a/gdb/testsuite/gdb.base/ui-redirect.exp b/gdb/testsuite/gdb.base/ui-redirect.exp
index ef0a79178c7..9e0a694c798 100644
--- a/gdb/testsuite/gdb.base/ui-redirect.exp
+++ b/gdb/testsuite/gdb.base/ui-redirect.exp
@@ -47,7 +47,7 @@ gdb_breakpoint "foo"
 gdb_breakpoint "bar"
 
 set cmds [multi_line_input \
-	      "break main" \
+	      "break -qualified main" \
 	      "  commands" \
 	      "    print 1" \
 	      "  end" \
diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
index 7545392452e..3bcd5c81546 100644
--- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
+++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
@@ -43,7 +43,7 @@ proc test_bkpt_basic { } {
 	gdb_scm_test_silent_cmd "guile (define blist (breakpoints))" \
 	    "get breakpoint list 1"
 	gdb_test "guile (print (car blist))" \
-	    "<gdb:breakpoint #1 BP_BREAKPOINT enabled noisy hit:1 ignore:0 @main>" \
+	    "<gdb:breakpoint #1 BP_BREAKPOINT enabled noisy hit:1 ignore:0 @-qualified main>" \
 	    "check main breakpoint"
 	gdb_test "guile (print (breakpoint-location (car blist)))" \
 	    "main" "check main breakpoint location"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 608469953bf..4c9675f255f 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -689,7 +689,7 @@ proc runto { function args } {
 # If you don't want that, use gdb_start_cmd.
 
 proc runto_main { } {
-    return [runto main no-message]
+    return [runto main no-message qualified]
 }
 
 ### Continue, and expect to hit a breakpoint.
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 732aed27b27..ea59288443e 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -1055,8 +1055,12 @@ proc mi_run_to_main { } {
 # It returns:
 #   -1  if test suppressed, failed, timedout
 #    0  if test passed
+#
+# Supported options:
+#
+#  -qualified -- pass --qualified to -break-insert
 
-proc mi_runto_helper {func run_or_continue} {
+proc mi_runto_helper {func run_or_continue args} {
   global suppress_flag
   if { $suppress_flag } {
     return -1
@@ -1065,10 +1069,16 @@ proc mi_runto_helper {func run_or_continue} {
   global mi_gdb_prompt expect_out
   global hex decimal fullname_syntax
 
+  parse_args {{qualified}}
+
   set test "mi runto $func"
   set bp [mi_make_breakpoint -type breakpoint -disp del \
 	      -func $func\(\\\(.*\\\)\)?]
-  mi_gdb_test "200-break-insert -t $func" "200\\^done,$bp" \
+  set extra_opts ""
+  if {$qualified} {
+      append extra_opts "--qualified"
+  }
+  mi_gdb_test "200-break-insert $extra_opts -t $func" "200\\^done,$bp" \
       "breakpoint at $func"
 
   if {$run_or_continue == "run"} {
@@ -1089,7 +1099,7 @@ proc mi_runto {func} {
 # Just like runto_main but works with the MI interface.
 
 proc mi_runto_main {} {
-    return [mi_runto_helper "main" "run"]
+    return [mi_runto_helper "main" "run" -qualified]
 }
 
 # Next to the next statement

base-commit: d3a071228e8f7cf9da017f2d0d6c28468a652795
-- 
2.14.5