Re: [PATCH] [gdb/corefiles] Fix segfault in add_thread_silent

[Tom de Vries <tdevries@suse.de>]

On 11/21/25 12:10 PM, Andrew Burgess wrote:
> Tom de Vries <tdevries@suse.de> writes:
> 
>> On 11/5/25 7:54 PM, Tom de Vries wrote:
>>> On 10/25/25 2:20 PM, Andrew Burgess wrote:
>>>> Tom de Vries <tdevries@suse.de> writes:
>>>>
>>>>> A user reported a segfault when loading a core file [1].
>>>>
>>>> Hi Tom,
>>>>
>>>> Thanks for taking a look at this.
>>>>
>>>
>>> Sure, and thanks for this review.
>>>
>>>>>
>>>>> The core file is from arm-linux, but I reproduced the segfault on
>>>>> x86_64-linux:
>>>>> ...
>>>>> $ gdb -q --core core
>>>>>
>>>>> warning: Can't open file /usr/bin/rs_scope during file-backed mapping
>>>>> note processing
>>>>>
>>>>> warning: Can't open file /lib/libc-2.26.so during file-backed mapping
>>>>> note processing
>>>>>
>>>>> warning: File /lib/libgcc_s.so.1 doesn't match build-id from core-
>>>>> file during file-backed mapping processing
>>>>>
>>>>> warning: Can't open file /lib/libm-2.26.so during file-backed mapping
>>>>> note processing
>>>>>
>>>>> warning: Can't open file /usr/lib/libstdc++.so.6.0.28 during file-
>>>>> backed mapping note processing
>>>>>
>>>>> warning: Can't open file /lib/libpthread-2.26.so during file-backed
>>>>> mapping note processing
>>>>>
>>>>> warning: Can't open file /lib/ld-2.26.so during file-backed mapping
>>>>> note processing
>>>>>
>>>>> Fatal signal: Segmentation fault
>>>>> ----- Backtrace -----
>>>>> 0x64a4ff gdb_internal_backtrace_1
>>>>>      gdb/bt-utils.c:122
>>>>> 0x64a59d _Z22gdb_internal_backtracev
>>>>>      gdb/bt-utils.c:175
>>>>> 0x9429e7 handle_fatal_signal
>>>>>      gdb/event-top.c:1013
>>>>> 0x942b96 handle_sigsegv
>>>>>      gdb/event-top.c:1090
>>>>> 0x7fbf6a64708f ???
>>>>>      /usr/src/debug/glibc-2.40/signal/../sysdeps/unix/sysv/linux/
>>>>> x86_64/libc_sigaction.c:0
>>>>> 0x5eb453
>>>>> _ZN9__gnu_cxx17__normal_iteratorIPKSt4pairI6ptid_tP11thread_infoESt6vectorIS5_SaIS5_EEEC4ERKS7_
>>>>>      /usr/include/c++/15/bits/stl_iterator.h:1059
>>>>> 0x5eb453 _ZNKSt6vectorISt4pairI6ptid_tP11thread_infoESaIS4_EE3endEv
>>>>>      /usr/include/c++/15/bits/stl_vector.h:1029
>>>>> 0x5eae9e _ZNKSt6vectorISt4pairI6ptid_tP11thread_infoESaIS4_EE5emptyEv
>>>>>      /usr/include/c++/15/bits/stl_vector.h:1224
>>>>> 0xa77588
>>>>> _ZNK6ankerl15unordered_dense6v4_4_06detail5tableI6ptid_tP11thread_infoNS1_4hashIS4_vEESt8equal_toIS4_ESaISt4pairIS4_S6_EENS1_11bucket_type8standardELb0EE5emptyEv
>>>>>      gdb/../gdbsupport/unordered_dense.h:1351
>>>>> 0xa76533
>>>>> _ZN6ankerl15unordered_dense6v4_4_06detail5tableI6ptid_tP11thread_infoNS1_4hashIS4_vEESt8equal_toIS4_ESaISt4pairIS4_S6_EENS1_11bucket_type8standardELb0EE7do_findIS4_EEN9__gnu_cxx17__normal_iteratorIPSC_St6vectorISC_SD_EEERKT_
>>>>>      gdb/../gdbsupport/unordered_dense.h:1119
>>>>> 0xa74fef
>>>>> _ZN6ankerl15unordered_dense6v4_4_06detail5tableI6ptid_tP11thread_infoNS1_4hashIS4_vEESt8equal_toIS4_ESaISt4pairIS4_S6_EENS1_11bucket_type8standardELb0EE4findERKS4_
>>>>>      gdb/../gdbsupport/unordered_dense.h:1773
>>>>> 0xa6f787 _ZN8inferior11find_threadE6ptid_t
>>>>>      gdb/inferior.c:253
>>>>> 0xfc852a _Z17add_thread_silentP22process_stratum_target6ptid_t
>>>>>      gdb/thread.c:310
>>>>> 0x73b995 core_target_open
>>>>>      gdb/corelow.c:1111
>>>>> 0x73a095 _Z17core_file_commandPKci
>>>>>      gdb/corelow.c:708
>>>>> 0xb6cb38 catch_command_errors
>>>>>      gdb/main.c:510
>>>>> 0xb6e354 captured_main_1
>>>>>      gdb/main.c:1279
>>>>> 0xb6e9a2 captured_main
>>>>>      gdb/main.c:1372
>>>>> 0xb6eaa3 _Z8gdb_mainP18captured_main_args
>>>>>      gdb/main.c:1401
>>>>> 0x419704 main
>>>>>      gdb/gdb.c:38
>>>>> ...
>>>>>
>>>>> The problem happens as follows.  In core_target_open, we do:
>>>>> ...
>>>>>         if (thread == NULL)
>>>>>           thread = add_thread_silent (target, ptid_t (CORELOW_PID));
>>>>> ...
>>>>> and then in add_thread_silent:
>>>>> ...
>>>>> struct thread_info *
>>>>> add_thread_silent (process_stratum_target *targ, ptid_t ptid)
>>>>> {
>>>>>     gdb_assert (targ != nullptr);
>>>>>
>>>>>     inferior *inf = find_inferior_ptid (targ, ptid);
>>>>> ...
>>>>> find_inferior_ptid returns nullptr, which eventually causes the
>>>>> segfault.
>>>>>
>>>>> So, why can't we find an inferior with CORELOW_PID?
>>>>>
>>>>> A bit earlier in core_target_open, we do:
>>>>> ...
>>>>>     /* Find (or fake) the pid for the process in this core file, and
>>>>>        initialise the current inferior with that pid.  */
>>>>>     bool fake_pid_p = false;
>>>>>     int pid = bfd_core_file_pid (target->core_bfd ());
>>>>>     if (pid == 0)
>>>>>       {
>>>>>         fake_pid_p = true;
>>>>>         pid = CORELOW_PID;
>>>>>       }
>>>>>
>>>>>     inferior *inf = current_inferior ();
>>>>>     gdb_assert (inf->pid == 0);
>>>>>     inferior_appeared (inf, pid);
>>>>>     inf->fake_pid_p = fake_pid_p;
>>>>> ...
>>>>>
>>>>> The problem is that looking for an inferior using CORELOW_PID is
>>>>> correct in
>>>>> case fake_pid_p == true, but otherwise not.
>>>>>
>>>>> Fix this by using inf->pid instead:
>>>>> ...
>>>>> -    thread = add_thread_silent (target, ptid_t (CORELOW_PID));
>>>>> +    thread = add_thread_silent (target, ptid_t (inf->pid));
>>>>> ...
>>>>
>>>> You've skipped over all the really interesting details here.
>>>>
>>>> The
>>>>
>>>>     if (thread == NULL)
>>>>       thread = add_thread_silent (target, ptid_t (CORELOW_PID));
>>>>
>>>> that you quoted earlier is wrapped in:
>>>>
>>>>     if (inferior_ptid == null_ptid)
>>>>     {
>>>>        ...
>>>>     }
>>>>
>>>> So for the case you've identified, inf->pid has some value non-zero
>>>> value, but inferior_ptid doesn't reflect this?
>>>>
>>>
>>> Yes.
>>>
>>>> I guess bfd_core_file_pid returned non-zero, which we then passed to
>>>> inferior_appeared.
>>>
>>> Yes.
>>>
>>>> So in my mind, the interesting detail here is why
>>>> does inferior_ptid not get updated in this case, but it does get updated
>>>> in "normal" cases?
>>>>
>>>
>>> I generated a core file using a source file containing
>>> __builtin_abort(), and set a watchpoint on inferior_ptid with gdb
>>> loading the core file.  The variable got set during a call to
>>> add_to_thread_list here in core_target_open:
>>> ...
>>>     /* Build up thread list from BFD sections, and possibly set the
>>>        current thread to the .reg/NN section matching the .reg
>>>        section.  */
>>>     asection *reg_sect
>>>       = bfd_get_section_by_name (target->core_bfd (), ".reg");
>>>     for (asection *sect : gdb_bfd_sections (target->core_bfd ()))
>>>       add_to_thread_list (sect, reg_sect, inf);
>>> ...
>>>
>>> In the case of the core file described in this patch, this doesn't
>>> triggers because there are no .reg sections, so reg_sect == nullptr.
>>>
>>
>> Ping.  Any further comments?
>>
> 
> Tom,
> 
> Sorry for loosing track of this thread, and thanks for the explanation,
> that makes sense.
> 
> I do think it is worth chasing what's going wrong in BFD to find out why
> we cannot find the .reg sections.  But that said, the core file is an
> external input, so we cannot rely on it actually _having_ the required
> .reg notes/sections, so GDB should be able to handle this case.
> 
> So, with that in mind, I agree that your patch should go in:
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>
> 

Thanks, I've pushed this.  Before pushing I've updated the commit 
message to include my theory about why the .reg pseudo-section is missing.

> But I think we (you?) should continue to investigate BFD to see what's
> going wrong there (though it looks like you already have it mostly
> figured out).
> 

I think so, yes.  I'll submit a patch for elf32_arm_nabi_grok_prstatus 
to the @binutils.

Thanks,
- Tom

> Thanks,
> Andrew
>