[PATCH v4 0/1] gdb: remove minimal symbol check in 'start_command'

[PATCH v4 0/1] gdb: remove minimal symbol check in 'start_command'

[Stephan Rohr]

Hello,

this is version 4 of my patch fixing the 'start' command for a specific
scenario on Windows.  

Based on the discussion from version 3 [1], I decided to remove the
minimal symbols check completely.  The motiviation for this is:

  * Adding a try / catch block in 'tbreak' or in 'run_command_1' is not straight
    forward.  If 'main' cannot be resolved, a 'NOT_FOUND_ERROR' is thrown.
  * A 'No symbol table is loaded.  Use the "file" command' error is printed.
  * The error is consumed in 'create_breakpoint' and the user may insert a 
    pending breakpoint.  The error only rethrown if pending breakpoints are not
    supported.

I appreciate your feedback.

Stephan

[1]: https://sourceware.org/pipermail/gdb-patches/2025-January/214603.html
     https://sourceware.org/pipermail/gdb-patches/2025-January/214732.html

Rohr, Stephan (1):
  gdb: remove check for minimal symbols in 'start_command'

 gdb/infcmd.c | 6 ------
 1 file changed, 6 deletions(-)

-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

[Stephan Rohr]

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);
 }
-- 
2.34.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

[Kevin Buettner]

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>

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

[Tom Tromey]

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

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

In Ada programs, the name of the 'main' program is stored in an array.
gdb reads this from the inferior (normally from the section data by
setting trust_readonly=true) by looking up the minimal symbol.

See ada-lang.c:ada_main_name.

I'd guess this patch would just cause gdb to behave differently here:
rather than getting an error, you'd get a stop in the "C main", which is
some generated Ada procedure that the user won't recognize.

Anyway it hardly seems fatal.  Perhaps we need some better way to
communicate the Ada main for this target, not sure.

Tom

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

[Guinevere Larsen]

On 1/27/25 8:28 PM, Kevin Buettner wrote:
> 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>
>
Has this patch been pushed it already? I just ran into this in a 
different situation and it would be nice to have the fix upstreamed :)

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

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

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> Has this patch been pushed it already? I just ran into this in a
Guinevere> different situation and it would be nice to have the fix upstreamed :)

If it's approved, then IMO it's fine to push it if you want.

Tom

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

[Rohr, Stephan]

Hi Tom,

Thanks for the feedback, I wanted to follow up, too.  I was not sure as there were
some concerns from Kevin regarding Ada.

I will push this then.

Stephan

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Tuesday, 11 February 2025 15:51
> To: Guinevere Larsen <guinevere@redhat.com>
> Cc: Kevin Buettner <kevinb@redhat.com>; Rohr, Stephan
> <stephan.rohr@intel.com>; gdb-patches@sourceware.org; eliz@gnu.org;
> tom@tromey.com
> Subject: Re: [PATCH v4 1/1] gdb: remove check for minimal symbols in
> 'start_command'
> 
> >>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> 
> Guinevere> Has this patch been pushed it already? I just ran into this in a
> Guinevere> different situation and it would be nice to have the fix upstreamed
> :)
> 
> If it's approved, then IMO it's fine to push it if you want.
> 
> Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Sean Fennelly, Jeffrey Schneiderman, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928