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

[Guinevere Larsen <guinevere@redhat.com>]

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