[DTrace-devel] [PATCH 08/10] proc: don't ptrace shortlived-grabbed processes if someone else is ptracing them

[Nick Alcock]

On 22 Aug 2023, Kris Van Hees said:

> On Wed, Aug 02, 2023 at 02:26:58PM +0100, Nick Alcock via DTrace-devel wrote:
>> Shortlived grabs, by definition, are grabs that won't last long and where
>> we don't expect things like dlopen monitoring to be useful.  So it probably
>> makes sense not to require such grabs to ptrace(): if they *can*, that's
>> good because it means we can extract dynamic loader scope info and take
>> things like symbol interposition into accout when doing symbol lookup,
>> but if someone else is already tracing the process we should probably just
>> try not ptracing it.
>> 
>> This makes it practical to have multiple dtraces USDT-probing the same
>> process at the same time: one of them will ptrace it, the other one won't,
>> and both will get the mapping info they need for a reliable trace.
>
> This description seems to be (at least partly) conflicting... Maybe also due
> to the language use 'probably' etc.  Either it does or it doesn't, right?  If

... true. I was trying to get across (poorly) that the loss of
functionality from not being able to ptrace in this case is minimal, and
certainly does not merit failing the grab entirely (which is a 100%
functionality loss!).

> it is good to be able to ptrace() because it affects symbol lookup, then how
> can you say that it doesn't matter if one DTrace can pytrace() the process
> while others cannot.  It would seem that the one that can ptrace() will get
> better information than the others.

It will, but the cases in which it will are marginal enough (when
affected symbols are undergoing interposition, dlmopen() etc) that
*most* uses will probably not notice, or will produce reasonable results
except perhaps with the odd address where a symbol might otherwise be
seen: there is no reason to simply refuse to attach entirely.

Reworded significantly:
   
,----[ proc: don't ptrace shortlived-grabbed processes if someone else is ptracing them ]
| Shortlived grabs, by definition, are grabs that won't last long and where we 
| don't expect things like dlopen monitoring to be useful.  We ptrace for
| this, and to extract dynamic loader scope info and take things like symbol
| interposition into accout when doing symbol lookup; but if someone else is 
| already tracing the process we should probably just try not ptracing it,
| since the worst that will happen if we do is that some name lookups of 
| interposed symbols or symbols in alternate dlmopen namespaces might yield
| the wrong address (probably leading to a probe not firing when it 
| should).
|    
| Interposed symbols are rare enough that this failure mode is probably not
| justification for refusing to trace entirely (perhaps a warning message
| might be desirable, but honestly I think it's rare enough that not even that
| is justifiable: most ptrace failures will have no visible negative
| consequences at all, since not only must an interposed symbol exist but we 
| must also be tracing it *and* tracing the process with multiple debuggers at 
| once, which seems likely to be very rare: nearly all warnings will be 
| warning when nothing will go wrong at all).
|    
| It is even rarer in the modern toolchain world in which many ELF objects
| cannot be interposed at all, whether due to -z now or 
| -fno-semantic-interposition or whatever.
|    
| Doing this makes it practical to have multiple dtraces USDT-probing the same
| process at the same time: one of them will ptrace it, the other one won't,
| and both will get the mapping info they need for a
| reliable-enough-to-be-useful trace (and if we're lucky and can ptrace,
| symbol resolution quality goes up a bit).
|    
| test/unittest/usdt/tst.multitrace.sh tests this (though there are other
| things stopping it working well, it cannot work at all without something
| like this).
`----

> I think we need more specifics here.  Also, is the lack of this patch visible
> in any form of test?

Yes: updated to note that tst.multitrace.sh will do this (see above).

-- 
NULL && (void)