On Thu, 6 May 2021 at 15:17, Warner Losh <imp@bsdimp.com> wrote:
>
>
>
> On Thu, May 6, 2021, 7:38 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> The ALLOCA(3) man-page mentions its "use is discouraged".
>>
>> Replace it by a g_new() call.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> bsd-user/syscall.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
>> index 4abff796c76..dbee0385ceb 100644
>> --- a/bsd-user/syscall.c
>> +++ b/bsd-user/syscall.c
>> @@ -355,9 +355,8 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
>> case TARGET_FREEBSD_NR_writev:
>> {
>> int count = arg3;
>> - struct iovec *vec;
>> + g_autofree struct iovec *vec = g_new(struct iovec, count);
>
>
> Where is this freed?
g_autofree, so it gets freed when it goes out of scope.
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#g-autofree
Ah. I'd missed that feature and annotation... Too many years seeing patches like
this in other projects where a feature like this wasn't there to save the day...
This means you can ignore my other message as equally misinformed.
> Also, alloca just moves a stack pointer, where malloc has complex interactions. Are you sure that's a safe change here?
alloca()ing something with size determined by the guest is
definitely not safe :-) malloc as part of "handle this syscall"
is pretty common, at least in linux-user.
Well, since this is userland emulation, the normal process boundaries will fix that. allocating from
the heap is little different..., so while unsafe, it's the domain of $SOMEONE_ELSE to enforce
the safety. linux-user has many similar usages for both malloc and alloca, and it's ok for the
same reason.
Doing a quick grep on my blitz[*] branch in the bsd-user fork shows that alloca is used almost
exclusively there. There's 24 calls to alloca in the bsd-user code. There's almost no malloc
calls (7) in that at all outside the image loader: all but one of them are confined to sysctl
emulation with the last one used to keep track of thread state in new threads...
linux-user has a similar profile (20 alloca's and 14 mallocs outside the loader),
but with more mallocs in other paths than just sysctl (which isn't present).
I had no plans on migrating to anything else...
Warner