Patchwork linux-user: fix multi-threaded /proc/self/maps

login
register
mail settings
Submitter Alexander Graf
Date May 30, 2012, 12:45 p.m.
Message ID <1338381921-29912-1-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/162207/
State New
Headers show

Comments

Alexander Graf - May 30, 2012, 12:45 p.m.
When reading our faked /proc/self/maps from a secondary thread,
we get an invalid stack entry. This is because ts->stack_base is not
initialized in non-primary threads.

However, ts->info is, and the stack layout information we're looking
for is there too. So let's use that one instead!

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 linux-user/syscall.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Peter Maydell - June 1, 2012, 12:44 a.m.
On 30 May 2012 13:45, Alexander Graf <agraf@suse.de> wrote:
> When reading our faked /proc/self/maps from a secondary thread,
> we get an invalid stack entry. This is because ts->stack_base is not
> initialized in non-primary threads.
>
> However, ts->info is, and the stack layout information we're looking
> for is there too. So let's use that one instead!

So in the multithreaded case do all the thread stacks live
in this one mapping, or do the non-primary thread stacks
live in a standard mmap'd mapping?

-- PMM
Alexander Graf - June 1, 2012, 1:16 a.m.
On 01.06.2012, at 02:44, Peter Maydell wrote:

> On 30 May 2012 13:45, Alexander Graf <agraf@suse.de> wrote:
>> When reading our faked /proc/self/maps from a secondary thread,
>> we get an invalid stack entry. This is because ts->stack_base is not
>> initialized in non-primary threads.
>> 
>> However, ts->info is, and the stack layout information we're looking
>> for is there too. So let's use that one instead!
> 
> So in the multithreaded case do all the thread stacks live
> in this one mapping, or do the non-primary thread stacks
> live in a standard mmap'd mapping?

I thought /proc/self/maps always shows the initial stack map as [stack]?


Alex
Peter Maydell - June 1, 2012, 1:39 a.m.
On 1 June 2012 02:16, Alexander Graf <agraf@suse.de> wrote:
> On 01.06.2012, at 02:44, Peter Maydell wrote:
>> So in the multithreaded case do all the thread stacks live
>> in this one mapping, or do the non-primary thread stacks
>> live in a standard mmap'd mapping?
>
> I thought /proc/self/maps always shows the initial stack map as [stack]?

I dunno, I asked because I'm too lazy to check myself :-)

-- PMM
Alexander Graf - June 1, 2012, 1:45 a.m.
On 01.06.2012, at 03:39, Peter Maydell <peter.maydell@linaro.org> wrote:

> On 1 June 2012 02:16, Alexander Graf <agraf@suse.de> wrote:
>> On 01.06.2012, at 02:44, Peter Maydell wrote:
>>> So in the multithreaded case do all the thread stacks live
>>> in this one mapping, or do the non-primary thread stacks
>>> live in a standard mmap'd mapping?
>> 
>> I thought /proc/self/maps always shows the initial stack map as [stack]?
> 
> I dunno, I asked because I'm too lazy to check myself :-)

Same here. Mind to check? Your time zone fits this better atm ;). Otherwise I can try tomorrow.

Alex

> 
> -- PMM
Peter Maydell - June 1, 2012, 2:17 a.m.
On 1 June 2012 02:45, Alexander Graf <agraf@suse.de> wrote:
> On 01.06.2012, at 03:39, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 1 June 2012 02:16, Alexander Graf <agraf@suse.de> wrote:
>>> On 01.06.2012, at 02:44, Peter Maydell wrote:
>>>> So in the multithreaded case do all the thread stacks live
>>>> in this one mapping, or do the non-primary thread stacks
>>>> live in a standard mmap'd mapping?
>>>
>>> I thought /proc/self/maps always shows the initial stack map as [stack]?
>>
>> I dunno, I asked because I'm too lazy to check myself :-)
>
> Same here. Mind to check? Your time zone fits this better atm ;)

So none of the processes on my system have more than one [stack]
mapping, even the multithreaded ones. OTOH the kernel sources:
http://lxr.free-electrons.com/source/fs/proc/task_mmu.c#L262
indicate that it is possible for them to print [stack:TID] for
a thread stack in some cases...

However, I think your change is right, because the secondary
threads ought to see the main process /proc/self/maps and
that should show the main stack.

Q: I think the reason for the
 #if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
is because linux-user/main.c only sets up ts->stack_base
on those targets. If we're not using ts->stack_base
any more can we drop the #if here?

(more generally I wonder if we can drop ts->stack_base completely
in favour of ts->info->start_stack...)

-- PMM
Alexander Graf - June 1, 2012, 10:22 a.m.
On 01.06.2012, at 04:17, Peter Maydell wrote:

> On 1 June 2012 02:45, Alexander Graf <agraf@suse.de> wrote:
>> On 01.06.2012, at 03:39, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 1 June 2012 02:16, Alexander Graf <agraf@suse.de> wrote:
>>>> On 01.06.2012, at 02:44, Peter Maydell wrote:
>>>>> So in the multithreaded case do all the thread stacks live
>>>>> in this one mapping, or do the non-primary thread stacks
>>>>> live in a standard mmap'd mapping?
>>>> 
>>>> I thought /proc/self/maps always shows the initial stack map as [stack]?
>>> 
>>> I dunno, I asked because I'm too lazy to check myself :-)
>> 
>> Same here. Mind to check? Your time zone fits this better atm ;)
> 
> So none of the processes on my system have more than one [stack]
> mapping, even the multithreaded ones. OTOH the kernel sources:
> http://lxr.free-electrons.com/source/fs/proc/task_mmu.c#L262
> indicate that it is possible for them to print [stack:TID] for
> a thread stack in some cases...
> 
> However, I think your change is right, because the secondary
> threads ought to see the main process /proc/self/maps and
> that should show the main stack.
> 
> Q: I think the reason for the
> #if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
> is because linux-user/main.c only sets up ts->stack_base
> on those targets. If we're not using ts->stack_base
> any more can we drop the #if here?
> 
> (more generally I wonder if we can drop ts->stack_base completely
> in favour of ts->info->start_stack...)

Good point. Maybe?

Alex

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 39d02f8..06408bd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4920,8 +4920,8 @@  static int open_self_maps(void *cpu_env, int fd)
 #if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
     dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0          [stack]\n",
                 (unsigned long long)ts->info->stack_limit,
-                (unsigned long long)(ts->stack_base + (TARGET_PAGE_SIZE - 1))
-                                     & TARGET_PAGE_MASK,
+                (unsigned long long)(ts->info->start_stack +
+                                     (TARGET_PAGE_SIZE - 1)) & TARGET_PAGE_MASK,
                 (unsigned long long)0);
 #endif