[v3] gdbstub: Implement follow-fork-mode child

[PATCH v3 02/11] {linux,bsd}-user: Update ts_tid after fork()

Posted by Ilya Leoshkevich 8 months, 2 weeks ago

Currently ts_tid contains the parent tid after fork(), which is not
correct. So far it has not affected anything, but the upcoming
follow-fork-mode child support relies on the correct value, so fix it.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 bsd-user/main.c   | 1 +
 linux-user/main.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index e5efb7b8458..4140edc8311 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -127,6 +127,7 @@ void fork_end(int child)
          * state, so we don't need to end_exclusive() here.
          */
         qemu_init_cpu_list();
+        ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
         gdbserver_fork(thread_cpu);
     } else {
         mmap_fork_end(child);
diff --git a/linux-user/main.c b/linux-user/main.c
index 74b2fbb3938..e6427d72332 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -160,6 +160,7 @@ void fork_end(int child)
             }
         }
         qemu_init_cpu_list();
+        ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
         gdbserver_fork(thread_cpu);
     } else {
         cpu_list_unlock();
-- 
2.43.0

Re: [PATCH v3 02/11] {linux,bsd}-user: Update ts_tid after fork()

Posted by Alex Bennée 8 months, 2 weeks ago

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Currently ts_tid contains the parent tid after fork(), which is not
> correct. So far it has not affected anything, but the upcoming
> follow-fork-mode child support relies on the correct value, so fix it.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  bsd-user/main.c   | 1 +
>  linux-user/main.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index e5efb7b8458..4140edc8311 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -127,6 +127,7 @@ void fork_end(int child)
>           * state, so we don't need to end_exclusive() here.
>           */
>          qemu_init_cpu_list();
> +        ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
>          gdbserver_fork(thread_cpu);
>      } else {
>          mmap_fork_end(child);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 74b2fbb3938..e6427d72332 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -160,6 +160,7 @@ void fork_end(int child)
>              }
>          }
>          qemu_init_cpu_list();
> +        ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
>          gdbserver_fork(thread_cpu);
>      } else {
>          cpu_list_unlock();

Given how many functions do this cast dance it does make we wonder if we
should just have a helper for *-user:

  TaskState * get_task_state(CPUState *cs)
  {
        return (TaskState *) cs->opaque;
  }

and be done with it. Richard?

Anyway good enough for now:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 02/11] {linux,bsd}-user: Update ts_tid after fork()

Posted by Richard Henderson 8 months, 2 weeks ago

On 2/16/24 07:45, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
>> Currently ts_tid contains the parent tid after fork(), which is not
>> correct. So far it has not affected anything, but the upcoming
>> follow-fork-mode child support relies on the correct value, so fix it.
>>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>>   bsd-user/main.c   | 1 +
>>   linux-user/main.c | 1 +
>>   2 files changed, 2 insertions(+)
>>
>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>> index e5efb7b8458..4140edc8311 100644
>> --- a/bsd-user/main.c
>> +++ b/bsd-user/main.c
>> @@ -127,6 +127,7 @@ void fork_end(int child)
>>            * state, so we don't need to end_exclusive() here.
>>            */
>>           qemu_init_cpu_list();
>> +        ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
>>           gdbserver_fork(thread_cpu);
>>       } else {
>>           mmap_fork_end(child);
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index 74b2fbb3938..e6427d72332 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -160,6 +160,7 @@ void fork_end(int child)
>>               }
>>           }
>>           qemu_init_cpu_list();
>> +        ((TaskState *)thread_cpu->opaque)->ts_tid = qemu_get_thread_id();
>>           gdbserver_fork(thread_cpu);
>>       } else {
>>           cpu_list_unlock();
> 
> Given how many functions do this cast dance it does make we wonder if we
> should just have a helper for *-user:
> 
>    TaskState * get_task_state(CPUState *cs)
>    {
>          return (TaskState *) cs->opaque;
>    }
> 
> and be done with it. Richard?

Seems like a good idea.


r~