On Tue, Feb 09, 2010 at 09:30:51AM -0800, Marcel Moolenaar wrote:
>
> On Feb 9, 2010, at 1:57 AM, Kostik Belousov wrote:
> >> + map = &p->p_vmspace->vm_map;
> > I think this place lacks two safety measures:
> > - vmspace should be referenced by vmspace_acquire_ref()
> > - vm_map should be read-locked before iterating the map entries.
>
> > Vmspace may be shared between stopped debugee and other process using
> > rfork(2), thus modified despite the fact that traced process is stopped.
>
> Will do. I considered it, but thought it wouldn't be necessary
> because the process has stopped. I didn't consider the rfork(2)
> case with RFMEM.
>
> Good catch!
>
>
> >> + entry = map->header.next;
> >> + if (pve->pve_cookie != NULL) {
> >> + while (entry != &map->header && entry != pve->pve_cookie)
> >> + entry = entry->next;
> > Could the entry pointed by pve_cookie be reused between ptrace(PT_VM_ENTRY)
> > invocations ? I think the debugger should be informed about this situation,
> > otherwise interface is too unreliable.
>
> I didn't think so, but I also didn't consider rfork :-)
>
> We can always put the timestamp in the structure. Either (1) the
> tracing process fills it on request or (2) the request returns
> it on completion.
>
> (1) the kernel checks the timestamp with the one in the map and
> returns an appropriate error. If the request has a timestamp
> of 0, no checking is performed.
> (2) the tracing process can check the timestamp returned by each
> request and compare that with the return value of the
> PT_VM_TIMESTAMP and restart the iteration.
>
> Either way, if the entry is reused, the timestamp will have changed
> and either the kernel or the tracing process can take appropriate
> action.
>
> Thoughts?
>
> BTW: I prefer 2.I agree that #2 looks preferably, except that I consider it more convenient when libc syscall wrapper return value is 0/-1 instead of -1/some data. This means that timestamp might be added to the existing struct ptrace_vm_entry.
pgprWBMarTceo.pgp
Description: PGP signature
