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

[Pedro Alves <palves at redhat dot com>]

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