This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at polymtl dot ca>, gdb-patches at sourceware dot org
- Cc: Simon Marchi <simon dot marchi at efficios dot com>
- Date: Tue, 9 Apr 2019 16:27:22 +0100
- Subject: Re: [PATCH] Use -qualified flag when setting temporary breakpoint in start command
- References: <20190409025557.28846-1-simon.marchi@polymtl.ca>
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