[PATCH] Use -qualified flag when setting temporary breakpoint in start command

[PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Simon Marchi]

From: Simon Marchi <simon.marchi@efficios.com>

When using the "start" command, GDB puts a temporary breakpoint on the
"main" symbol (we literally invoke the tbreak command).  However, since
it does wild matching by default, it also puts a breakpoint on any C++
method or "main" function in a namespace.  For example, when debugging
GDB, it creates a total of 24 locations:

  (gdb) start
  Temporary breakpoint 1 at 0x198c1e9: main. (24 locations)

as there are a bunch of methods called main in the selftests, such as

  selftests::string_view::capacity_1::main()

If such method was called in the constructor of a global object, or a
function marked with the attribute "constructor", then we would stop at
the wrong place.  Also, this causes a few extra symtabs (those that
contain the "wrong" mains) to be expanded for nothing.

The dummiest, most straightforward solution is to add -qualified when
invoking tbreak.  With this patch, "start" creates a single-location
breakpoint, as expected.

I changed the start.exp test to use a C++ test file, which contains two
main functions.  The test now verifies that the output of "start" is the
output we get when we set a single-location breakpoint.

gdb/ChangeLog:

	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
	the "start" command.

gdb/testsuite/ChangeLog:

	* gdb.base/start.exp: Change test file to start.cpp.  Enhance
	regexp to match output of single-location breakpoint.
	* gdb.base/start.cpp: New file.
---
 gdb/infcmd.c                     |  5 ++++-
 gdb/testsuite/gdb.base/start.cpp | 31 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/start.exp |  4 ++--
 3 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/start.cpp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index c5977c48a90f..afc59d142649 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -605,7 +605,10 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
 
   /* Insert temporary breakpoint in main function if requested.  */
   if (run_how == RUN_STOP_AT_MAIN)
-    tbreak_command (main_name (), 0);
+    {
+      std::string arg = string_printf ("-qualified %s", main_name ());
+      tbreak_command (arg.c_str (), 0);
+    }
 
   exec_file = get_exec_file (0);
 
diff --git a/gdb/testsuite/gdb.base/start.cpp b/gdb/testsuite/gdb.base/start.cpp
new file mode 100644
index 000000000000..2c99bebb454f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/start.cpp
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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 <http://www.gnu.org/licenses/>.  */
+
+namespace foo
+{
+
+int main ()
+{
+  return 1;
+}
+
+} /* namespace foo */
+
+int main()
+{
+  return foo::main ();
+}
diff --git a/gdb/testsuite/gdb.base/start.exp b/gdb/testsuite/gdb.base/start.exp
index d055c9d0d12f..847e322bf379 100644
--- a/gdb/testsuite/gdb.base/start.exp
+++ b/gdb/testsuite/gdb.base/start.exp
@@ -14,7 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
-standard_testfile
+standard_testfile start.cpp
 
 if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
     return -1
@@ -30,5 +30,5 @@ if { [gdb_start_cmd] < 0 } {
 }
 
 gdb_test "" \
-         "main \\(\\) at .*start.c.*" \
+         "Temporary breakpoint $decimal at $hex: file.*main \\(\\) at .*start.cpp:.*" \
          "start"
-- 
2.21.0

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Pedro Alves]

On 4/9/19 3:55 AM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@efficios.com>
> 
> When using the "start" command, GDB puts a temporary breakpoint on the
> "main" symbol (we literally invoke the tbreak command).  However, since
> it does wild matching by default, it also puts a breakpoint on any C++
> method or "main" function in a namespace.  For example, when debugging
> GDB, it creates a total of 24 locations:
> 
>   (gdb) start
>   Temporary breakpoint 1 at 0x198c1e9: main. (24 locations)
> 
> as there are a bunch of methods called main in the selftests, such as
> 
>   selftests::string_view::capacity_1::main()
> 
> If such method was called in the constructor of a global object, or a
> function marked with the attribute "constructor", then we would stop at
> the wrong place.  Also, this causes a few extra symtabs (those that
> contain the "wrong" mains) to be expanded for nothing.
> 
> The dummiest, most straightforward solution is to add -qualified when
> invoking tbreak.  With this patch, "start" creates a single-location
> breakpoint, as expected.
> 

That seems fine.

> I changed the start.exp test to use a C++ test file, which contains two
> main functions.  The test now verifies that the output of "start" is the
> output we get when we set a single-location breakpoint.

I'm mildly concerned that this drops testing with C, though.  Given
that "start" is a basic command, and that C++ symbol/name matching
differs from C, and considering that some targets don't even
support C++ (considering extended-remote too), I'd think it to
be prudent to test both C and C++.  I wouldn't say it's a big
deal, but, the .exp file is small, so it shouldn't be much of
a maintenance burden to copy & adjust it as a separate .exp
file, IMHO.

> 
> gdb/ChangeLog:
> 
> 	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
> 	the "start" command.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/start.exp: Change test file to start.cpp.  Enhance
> 	regexp to match output of single-location breakpoint.
> 	* gdb.base/start.cpp: New file.

Nit: all other C++ source files in the testsuite use .cc extension.

> +namespace foo
> +{
> +
> +int main ()
> +{
> +  return 1;
> +}
> +
> +} /* namespace foo */
> +
> +int main()

Watch out for GNU formatting.

Thanks,
Pedro Alves

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Simon Marchi]

On 2019-04-09 11:27 a.m., Pedro Alves wrote:
> On 4/9/19 3:55 AM, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> When using the "start" command, GDB puts a temporary breakpoint on the
>> "main" symbol (we literally invoke the tbreak command).  However, since
>> it does wild matching by default, it also puts a breakpoint on any C++
>> method or "main" function in a namespace.  For example, when debugging
>> GDB, it creates a total of 24 locations:
>>
>>   (gdb) start
>>   Temporary breakpoint 1 at 0x198c1e9: main. (24 locations)
>>
>> as there are a bunch of methods called main in the selftests, such as
>>
>>   selftests::string_view::capacity_1::main()
>>
>> If such method was called in the constructor of a global object, or a
>> function marked with the attribute "constructor", then we would stop at
>> the wrong place.  Also, this causes a few extra symtabs (those that
>> contain the "wrong" mains) to be expanded for nothing.
>>
>> The dummiest, most straightforward solution is to add -qualified when
>> invoking tbreak.  With this patch, "start" creates a single-location
>> breakpoint, as expected.
>>
> 
> That seems fine.
> 
>> I changed the start.exp test to use a C++ test file, which contains two
>> main functions.  The test now verifies that the output of "start" is the
>> output we get when we set a single-location breakpoint.
> 
> I'm mildly concerned that this drops testing with C, though.  Given
> that "start" is a basic command, and that C++ symbol/name matching
> differs from C, and considering that some targets don't even
> support C++ (considering extended-remote too), I'd think it to
> be prudent to test both C and C++.  I wouldn't say it's a big
> deal, but, the .exp file is small, so it shouldn't be much of
> a maintenance burden to copy & adjust it as a separate .exp
> file, IMHO.

I had initially changed start.exp to test both C and C++, then decided to just
keep C++ for simplicity.  But your point about coverage is good, so I've done
as you suggested.

>>
>> gdb/ChangeLog:
>>
>> 	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
>> 	the "start" command.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.base/start.exp: Change test file to start.cpp.  Enhance
>> 	regexp to match output of single-location breakpoint.
>> 	* gdb.base/start.cpp: New file.
> 
> Nit: all other C++ source files in the testsuite use .cc extension.

Fixed.

>> +namespace foo
>> +{
>> +
>> +int main ()
>> +{
>> +  return 1;
>> +}
>> +
>> +} /* namespace foo */
>> +
>> +int main()
> 
> Watch out for GNU formatting.

Woops.  The blame is shared between the original start.c file and me not
doing enough GDB these days.

Here is the updated patch.


From 232b274adfc6904d33bde3baa52e40836af6221b Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Mon, 8 Apr 2019 22:55:57 -0400
Subject: [PATCH] Use -qualified flag when setting temporary breakpoint in
 start command

When using the "start" command, GDB puts a temporary breakpoint on the
"main" symbol (we literally invoke the tbreak command).  However, since
it does wild matching by default, it also puts a breakpoint on any C++
method or "main" function in a namespace.  For example, when debugging
GDB, it creates a total of 24 locations:

  (gdb) start
  Temporary breakpoint 1 at 0x198c1e9: main. (24 locations)

as there are a bunch of methods called main in the selftests, such as

  selftests::string_view::capacity_1::main()

If such method was called in the constructor of a global object, or a
function marked with the attribute "constructor", then we would stop at
the wrong place.  Also, this causes a few extra symtabs (those that
contain the "wrong" mains) to be expanded for nothing.

The dummiest, most straightforward solution is to add -qualified when
invoking tbreak.  With this patch, "start" creates a single-location
breakpoint, as expected.

I copied the start.exp test to start-cpp.exp and made it use a C++ test
file, which contains two main functions.  The new test verifies that the
output of "start" is the output we get when we set a single-location
breakpoint.

gdb/ChangeLog:

	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
	the "start" command.

gdb/testsuite/ChangeLog:

	* gdb.base/start-cpp.exp: New file.
	* gdb.base/start-cpp.cc: New file.
---
 gdb/infcmd.c                         |  5 +++-
 gdb/testsuite/gdb.base/start-cpp.exp | 37 ++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/start-cpp.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3b26fd4a4671..178f89e95207 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -604,7 +604,10 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)

   /* Insert temporary breakpoint in main function if requested.  */
   if (run_how == RUN_STOP_AT_MAIN)
-    tbreak_command (main_name (), 0);
+    {
+      std::string arg = string_printf ("-qualified %s", main_name ());
+      tbreak_command (arg.c_str (), 0);
+    }

   exec_file = get_exec_file (0);

diff --git a/gdb/testsuite/gdb.base/start-cpp.exp b/gdb/testsuite/gdb.base/start-cpp.exp
new file mode 100644
index 000000000000..5f98b92ffa41
--- /dev/null
+++ b/gdb/testsuite/gdb.base/start-cpp.exp
@@ -0,0 +1,37 @@
+# Copyright 2005-2019 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 <http://www.gnu.org/licenses/>.
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# This is a testcase specifically for the `start' GDB command.  For regular
+# stop-in-main goal in the testcases consider using `runto_main' instead.
+
+# In this C++ version of the test (as opposed to start.exp), we specifically
+# test that the temporary breakpoint created by the start command has a single
+# location, even if we have a function named "main" in a non-root namespace.
+
+# For C++ programs, "start" should stop in main().
+if { [gdb_start_cmd] < 0 } {
+    untested start
+    return -1
+}
+
+gdb_test "" \
+         "Temporary breakpoint $decimal at $hex: file.*main \\(\\) at .*start-cpp.cc:.*" \
+         "start"
-- 
2.21.0

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Pedro Alves]

On 4/9/19 5:19 PM, Simon Marchi wrote:

> I had initially changed start.exp to test both C and C++, then decided to just
> keep C++ for simplicity.  But your point about coverage is good, so I've done
> as you suggested.
> 

Thanks.

> gdb/ChangeLog:
> 
> 	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
> 	the "start" command.

Typo "usind".

> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/start-cpp.exp: New file.
> 	* gdb.base/start-cpp.cc: New file.

Note start-cpp.cc wasn't included in this patch -- don't forget
to git add it.

Otherwise OK.

Thanks,
Pedro Alves

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Simon Marchi]

On 2019-04-09 12:30 p.m., Pedro Alves wrote:
> On 4/9/19 5:19 PM, Simon Marchi wrote:
> 
>> I had initially changed start.exp to test both C and C++, then decided to just
>> keep C++ for simplicity.  But your point about coverage is good, so I've done
>> as you suggested.
>>
> 
> Thanks.
> 
>> gdb/ChangeLog:
>>
>> 	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
>> 	the "start" command.
> 
> Typo "usind".

Damn, I noticed this comment just after pushing.  I fixed it in a follow up commit.

>>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.base/start-cpp.exp: New file.
>> 	* gdb.base/start-cpp.cc: New file.
> 
> Note start-cpp.cc wasn't included in this patch -- don't forget
> to git add it.

Ah, I think it's because I've renamed it from start-cpp.cpp at the last minute,
when I noticed your comment about the extension.

Here's what the definitive version I pushed (with start-cpp.cc included).

Thanks for the review.

Simon


From e242fd1249ae85a97f08f95d5c61f4cbe3b906e0 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Tue, 9 Apr 2019 12:32:26 -0400
Subject: [PATCH] Use -qualified flag when setting temporary breakpoint in
 start command

When using the "start" command, GDB puts a temporary breakpoint on the
"main" symbol (we literally invoke the tbreak command).  However, since
it does wild matching by default, it also puts a breakpoint on any C++
method or "main" function in a namespace.  For example, when debugging
GDB, it creates a total of 24 locations:

  (gdb) start
  Temporary breakpoint 1 at 0x198c1e9: main. (24 locations)

as there are a bunch of methods called main in the selftests, such as

  selftests::string_view::capacity_1::main()

If such method was called in the constructor of a global object, or a
function marked with the attribute "constructor", then we would stop at
the wrong place.  Also, this causes a few extra symtabs (those that
contain the "wrong" mains) to be expanded for nothing.

The dummiest, most straightforward solution is to add -qualified when
invoking tbreak.  With this patch, "start" creates a single-location
breakpoint, as expected.

I copied the start.exp test to start-cpp.exp and made it use a C++ test
file, which contains two main functions.  The new test verifies that the
output of "start" is the output we get when we set a single-location
breakpoint.

gdb/ChangeLog:

	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
	the "start" command.

gdb/testsuite/ChangeLog:

	* gdb.base/start-cpp.exp: New file.
	* gdb.base/start-cpp.cc: New file.
---
 gdb/ChangeLog                        |  5 ++++
 gdb/infcmd.c                         |  5 +++-
 gdb/testsuite/ChangeLog              |  5 ++++
 gdb/testsuite/gdb.base/start-cpp.cc  | 33 +++++++++++++++++++++++++
 gdb/testsuite/gdb.base/start-cpp.exp | 37 ++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/start-cpp.cc
 create mode 100644 gdb/testsuite/gdb.base/start-cpp.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e1d20fd4f20e..7af9b9047018 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-09  Simon Marchi  <simon.marchi@efficios.com>
+
+	* infcmd.c (run_command_1): Pass -qualified to tbreak when usind
+	the "start" command.
+
 2019-04-08  Kevin Buettner  <kevinb@redhat.com>

 	* python/py-inferior.c (infpy_thread_from_thread_handle):
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3b26fd4a4671..178f89e95207 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -604,7 +604,10 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)

   /* Insert temporary breakpoint in main function if requested.  */
   if (run_how == RUN_STOP_AT_MAIN)
-    tbreak_command (main_name (), 0);
+    {
+      std::string arg = string_printf ("-qualified %s", main_name ());
+      tbreak_command (arg.c_str (), 0);
+    }

   exec_file = get_exec_file (0);

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 5f1ce3cb32d8..5c7cadf10a19 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-09  Simon Marchi  <simon.marchi@efficios.com>
+
+	* gdb.base/start-cpp.exp: New file.
+	* gdb.base/start-cpp.cc: New file.
+
 2019-04-08  Kevin Buettner  <kevinb@redhat.com>

 	* gdb.python/py-thrhandle.exp: Adjust tests to call
diff --git a/gdb/testsuite/gdb.base/start-cpp.cc b/gdb/testsuite/gdb.base/start-cpp.cc
new file mode 100644
index 000000000000..1d0fd7d323c9
--- /dev/null
+++ b/gdb/testsuite/gdb.base/start-cpp.cc
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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 <http://www.gnu.org/licenses/>.  */
+
+namespace foo
+{
+
+int
+main ()
+{
+  return 1;
+}
+
+} /* namespace foo */
+
+int
+main ()
+{
+  return foo::main ();
+}
diff --git a/gdb/testsuite/gdb.base/start-cpp.exp b/gdb/testsuite/gdb.base/start-cpp.exp
new file mode 100644
index 000000000000..5f98b92ffa41
--- /dev/null
+++ b/gdb/testsuite/gdb.base/start-cpp.exp
@@ -0,0 +1,37 @@
+# Copyright 2005-2019 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 <http://www.gnu.org/licenses/>.
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+# This is a testcase specifically for the `start' GDB command.  For regular
+# stop-in-main goal in the testcases consider using `runto_main' instead.
+
+# In this C++ version of the test (as opposed to start.exp), we specifically
+# test that the temporary breakpoint created by the start command has a single
+# location, even if we have a function named "main" in a non-root namespace.
+
+# For C++ programs, "start" should stop in main().
+if { [gdb_start_cmd] < 0 } {
+    untested start
+    return -1
+}
+
+gdb_test "" \
+         "Temporary breakpoint $decimal at $hex: file.*main \\(\\) at .*start-cpp.cc:.*" \
+         "start"
-- 
2.21.0

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[André Pönitz]

On Mon, Apr 08, 2019 at 10:55:57PM -0400, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@efficios.com>
> 
> When using the "start" command, GDB puts a temporary breakpoint on the
> "main" symbol (we literally invoke the tbreak command).  However, since
> it does wild matching by default, it also puts a breakpoint on any C++
> method or "main" function in a namespace.  For example, when debugging
> GDB, it creates a total of 24 locations:

I wonder whether there's still a chance to have a(n additional) way
to specify the effect of -qualified using a syntax that is the same
across GDB versions.

I have pretty much the same effect like Simon for 'start' for a feature
'Break on abort()' in 'my' IDE, that post-8.1 triggers on any function
called 'abort'.

Even with the Python interface there seems to be no way to have
a single way to set the breakpoint short of having Pre/Post81Breakpoint
classes using different base constructors and try:/except...: 
to find the matching version.

To be honest, I am tempted to call the whole pattern matching on
function names a mis-feature. C++ name resolution is not really
compatible with regexps, so at the very least when naming the
global explicitly ('b ::abort') there should be no match on
'struct Foo { void abort() {} };'

Andre'

Re: [PP?] Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Simon Marchi]

On 2019-04-09 13:12, André Pönitz wrote:
> I wonder whether there's still a chance to have a(n additional) way
> to specify the effect of -qualified using a syntax that is the same
> across GDB versions.
> 
> I have pretty much the same effect like Simon for 'start' for a feature
> 'Break on abort()' in 'my' IDE, that post-8.1 triggers on any function
> called 'abort'.
> 
> Even with the Python interface there seems to be no way to have
> a single way to set the breakpoint short of having Pre/Post81Breakpoint
> classes using different base constructors and try:/except...:
> to find the matching version.
> 
> To be honest, I am tempted to call the whole pattern matching on
> function names a mis-feature. C++ name resolution is not really
> compatible with regexps, so at the very least when naming the
> global explicitly ('b ::abort') there should be no match on
> 'struct Foo { void abort() {} };'

There is no pattern matching/regexp happening here.  The wild matching 
introduced in 8.1 just means that the identifier that you look up may be 
at any level, under some namespace or class.

Unless I am missing something, I believe that 'b ::abort' should do what 
you want, and the current behavior is simply buggy.

Simon

Re: [PP?] Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[André Pönitz]

On Tue, Apr 09, 2019 at 06:02:09PM -0400, Simon Marchi wrote:
> On 2019-04-09 13:12, André Pönitz wrote:
> > I wonder whether there's still a chance to have a(n additional) way
> > to specify the effect of -qualified using a syntax that is the same
> > across GDB versions.
> > 
> > I have pretty much the same effect like Simon for 'start' for a feature
> > 'Break on abort()' in 'my' IDE, that post-8.1 triggers on any function
> > called 'abort'.
> > 
> > Even with the Python interface there seems to be no way to have
> > a single way to set the breakpoint short of having Pre/Post81Breakpoint
> > classes using different base constructors and try:/except...:
> > to find the matching version.
> > 
> > To be honest, I am tempted to call the whole pattern matching on
> > function names a mis-feature. C++ name resolution is not really
> > compatible with regexps, so at the very least when naming the
> > global explicitly ('b ::abort') there should be no match on
> > 'struct Foo { void abort() {} };'
> 
> There is no pattern matching/regexp happening here.  The wild matching
> introduced in 8.1 just means that the identifier that you look up may be at
> any level, under some namespace or class.

Ok. Sorry for not having checked the implementation before writing
the mail. 

I have some sympathy for the 'ignore namespaces' case as I am living in a 
world of user(!)-configurable namespaces myself, but I still do not see
much benefit in treating 'some_class::bar()' and 'some_ns::bar()' "the same"
under "default" circumstances. Ignoring the static some_class case these
are pretty much always really different functions. 

> Unless I am missing something, I believe that 'b ::abort' should do what you
> want, and the current behavior is simply buggy.

Buggy or not, it's certainly intentionally changed user-visible behaviour
without good way on the user side to distinguish between the versions.
This makes maintaining GDB 

I cannot reasonbly say "without any way" as one apparently can distinguish the
versions by trying to instantiate gdb.Breakpoint sub-classes with different
constructors and tell from the failures on which side of the fence one lives,
but that's, erm...

Anyway, mileage varies, and I'll rest my case, and remain happy enough that this
doesn't happen very often.

Andre'

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> When using the "start" command, GDB puts a temporary breakpoint on the
Simon> "main" symbol (we literally invoke the tbreak command).

[...]

Simon> The dummiest, most straightforward solution is to add -qualified when
Simon> invoking tbreak.  With this patch, "start" creates a single-location
Simon> breakpoint, as expected.

This seems like a good idea to me, with the possible caveat that it
should be tested for Rust and Ada main programs as well.  (Though
probably there are already test cases covering this...?)

thanks,
Tom

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Simon Marchi]

On 2019-04-25 10:10 a.m., Tom Tromey wrote:
> This seems like a good idea to me, with the possible caveat that it
> should be tested for Rust and Ada main programs as well.  (Though
> probably there are already test cases covering this...?)

I presume there is no Ada or Rust test that specifically check the situation
where a non-qualified breakpoint on "main" (or equivalent for the language) would
result in a multi-location breakpoint.

Can you help by providing Ada and Rust snippets that would result in multi-location
breakpoints on main (or whatever the entry point is called)?  I could then convert
them to test cases.

Thanks,

Simon

Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> I presume there is no Ada or Rust test that specifically check the situation
Simon> where a non-qualified breakpoint on "main" (or equivalent for the language) would
Simon> result in a multi-location breakpoint.

Probably not.  I think my main concern is just that it continues to work
when the name of "main" is something that already has qualifiers.

Simon> Can you help by providing Ada and Rust snippets that would result in multi-location
Simon> breakpoints on main (or whatever the entry point is called)?  I could then convert
Simon> them to test cases.

I think actually my worry would be addressed by any "start" test in
those languages, because the main to stop at is always qualified.

Tom