Re: [PATCH v4 1/1] gdb: remove check for minimal symbols in 'start_command'

[Kevin Buettner <kevinb@redhat.com>]

On Mon, 27 Jan 2025 07:42:11 -0800
Stephan Rohr <stephan.rohr@intel.com> wrote:

> From: "Rohr, Stephan" <stephan.rohr@intel.com>
> 
> GDB aborts the 'start' command if the minimal symbols cannot be
> resolved.  On Windows, GDB reads the minimal symbols from the COFF
> header of the PE file.  The symbol table is deprecated and the
> number of symbols in the COFF header may be zero:
> 
>   https://learn.microsoft.com/en-us/windows/win32/debug/pe-format
> 
> This is reproducible with clang version 18.1.8 on Windows:
> 
>   clang++ -g -O0 -gdwarf -fuse-ld=lld test.cpp -o test_clang
> 
> The COFF file header shows:
> 
>   FILE HEADER VALUES
>         8664 machine (x64)
>            E number of sections
>     66E889EC time date stamp Mon Sep 16 21:41:32 2024
>        FB400 file pointer to symbol table
>            0 number of symbols
>           F0 size of optional header
>           22 characteristics
> 
> GDB is not able to read the minimal symbols; the `start' command fails
> with an error:
> 
>   (gdb) start
>   No symbol table loaded.  Use the "file" command.
> 
> Manually inserting a breakpoint in main works fine:
> 
>   (gdb) tbreak main
>   Temporary breakpoint 1 at 0x14000100c: file test.cpp, line 6.
>   (gdb) run
>   Starting program: C:\test-clang
> 
>   Temporary breakpoint 1, main () at test.cpp:6
>   6         std::cout << "Hello World.\n";
> 
> Remove the check entirely; a 'NOT_FOUND_ERROR' is thrown if 'main'
> cannot be resolved.  The error is consumed in 'create_breakpoint ()'
> and an error message is displayed to the user.
> ---
>  gdb/infcmd.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index b6b21a46b3d..00703e44b7b 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -517,12 +517,6 @@ run_command (const char *args, int from_tty)
>  static void
>  start_command (const char *args, int from_tty)
>  {
> -  /* Some languages such as Ada need to search inside the program
> -     minimal symbols for the location where to put the temporary
> -     breakpoint before starting.  */
> -  if (!have_minimal_symbols (current_program_space))
> -    error (_("No symbol table loaded.  Use the \"file\" command."));
> -
>    /* Run the program until reaching the main procedure...  */
>    run_command_1 (args, from_tty, RUN_STOP_AT_MAIN);
>  }

It's possible to reproduce this behavior on Linux too:

$ gcc -static -g -o hello hello.c
$ strip --keep-section='.debug*' hello

Using a gdb built without this patch:

    (gdb) file hello
    Reading symbols from hello...
    (gdb) start
    No symbol table loaded.  Use the "file" command.

Which demonstrates the behavior that you're fixing with this patch.

And yet...

    (gdb) b main
    Breakpoint 1 at 0x401849: file hello.c, line 6.
    (gdb) run
    Starting program: /home/kev/ctests/hello 

    This GDB supports auto-downloading debuginfo from the following URLs:
      <https://debuginfod.fedoraproject.org/>
    Enable debuginfod for this session? (y or [n]) y
    Debuginfod has been enabled.
    To make this setting permanent, add 'set debuginfod enabled on' to .gdbinit.

    Breakpoint 1, main () at hello.c:6
    6	    printf("Hello world\n");

...it's still possible to set a breakpoint on main and run to it.

With your patch:

(gdb) file hello
Reading symbols from hello...
(gdb) start
Temporary breakpoint 1 at 0x401849: file hello.c, line 6.
Starting program: /home/kev/ctests/hello 

This GDB supports auto-downloading debuginfo from the following URLs:
  <https://debuginfod.fedoraproject.org/>
Enable debuginfod for this session? (y or [n]) y
Debuginfod has been enabled.
To make this setting permanent, add 'set debuginfod enabled on' to .gdbinit.

Temporary breakpoint 1, main () at hello.c:6
6	    printf("Hello world\n");

The one thing that concerns me is the comment regarding the Ada language
and the need to search for minimal symbols.  I'm going to give this patch
an "Approved-by", but please wait a few days for others to weigh in
regarding this concern.

Approved-by: Kevin Buettner <kevinb@redhat.com>