Re: [PATCH v2 08/10] Avoid double free in startup_inferior

Simon Marchi
2020-09-08T02:21:50Z

On 2020-09-07 8:54 p.m., Kamil Rytarowski wrote:
> On 07.09.2020 21:19, Simon Marchi wrote:
>> On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
>>> Do not free the last execd pathname as it will be used in
>>> prepare_resume_reply(), after attaching a client side.
>>
>> Ok, so this function returns to its caller the last waitstatus. So indeed we
>> want to clean up all the watstatus objects except the last one, which we hand
>> over to the caller.
>>
>>> gdb/ChangeLog:
>>>
>>> * fork-inferior.c (startup_inferior): Avoid double free.
>>> ---
>>> gdb/ChangeLog | 4 ++++
>>> gdb/nat/fork-inferior.c | 5 ++++-
>>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>>> index b96e7bf08e8..1013f6a0b3c 100644
>>> --- a/gdb/ChangeLog
>>> +++ b/gdb/ChangeLog
>>> @@ -1,3 +1,7 @@
>>> +2020-09-04 Kamil Rytarowski <n54@gmx.com>
>>> +
>>> + * fork-inferior.c (startup_inferior): Avoid double free.
>>> +
>>> 2020-09-04 Kamil Rytarowski <n54@gmx.com>
>>>
>>> * netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
>>> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
>>> index 1185ef8998b..94ab0b9cbc2 100644
>>> --- a/gdb/nat/fork-inferior.c
>>> +++ b/gdb/nat/fork-inferior.c
>>> @@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps,
>>>
>>> case TARGET_WAITKIND_EXECD:
>>> /* Handle EXEC signals as if they were SIGTRAP signals. */
>>> - xfree (ws.value.execd_pathname);
>>> + /* Do not free the last execd pathname as it will be used in
>>> + prepare_resume_reply(), after attaching a client side. */
>>
>> Since this is common code, let's just write it in a gdbserver or gdb agnostic way.
>>
>> I'd suggest something like: Free the exec'ed pathname, but only if this isn't the
>> waitstatus we are returning to the caller.
>>
>
> Please be more specific how to fix.

Sorry, I just meant "write the comment in a gdbserver of gdb agnostic way". In particular,
don't mention prepare_resume_reply, which is gdbserver-specific (whereas the code is shared.

I suggest using the phrasing above for the comment.

Simon