[DTrace-devel] [PATCH 05/10] dtrace provider: don't use functions from uprobes.h any more

[Nick Alcock]

On 22 Aug 2023, Kris Van Hees spake thusly:
> On Wed, Aug 02, 2023 at 02:26:55PM +0100, Nick Alcock via DTrace-devel wrote:
>> +		if (mapp->pr_file->first_segment != mapp)
>> +			mapp = mapp->pr_file->first_segment;
>> +
>> +		asprintf(&spec, "%s:0x%lx", mapp->pr_file->prf_mapname,
>> +			 sym.st_value - mapp->pr_vaddr);
>
> One problem is that asprintf() allocates memory for spec and that is done
> using the libc allocation routines and not dt_alloc(), and so it is not
> really correct that it gets freed in attach() using dt_free().  Right now,
> in practical sense, that ends up being the same thing but it would be wrong
> if dt_alloc() / dt_free() ever gets implemented to use alternative routines.
>
> To avoid possible future issues, it should be freed using free().

Ack (and done), but frankly my opinion here is that dt_alloc, dt_free,
dt_zalloc et al are needless complexity, and we should just drop the lot
with a coccinelle script or something, just like I did for libctf. The
way to replace the allocator is to interpose malloc/calloc/realloc like
everyone else does, not to define your own allocator routines with a
different and less convenient interface (which were never used
consistently even in the Solaris days, and certainly never paired
consistently). Every other way is just a recipe for, not fewer bugs, but
many more. Even having them tied to malloc() as they are now is
annoying, because a bunch of bug detectors simply point at dt_alloc()
etc as the source of memory leaks, overrun allocations, etc, rather than
their *actual* source one call frame up. (Thankfully valgrind shows the
whole call stack.)

Using dt_alloc et al consistently would be awful: you'd have to eschew
everything else that allocated memory, like asprintf() above, in favour
of calling dt_alloc ourselves (and just forget things that can't be
passed preallocated buffers: we certainly wouldn't be able to use e.g.
libelf any more). And using it, but not consistently, is pointless. It's
no use for leak detection, bounds detection, or alternative allocators.
What's it for? Making DTrace harder to maintain? :)

-- 
NULL && (void)