Re: [PATCH v1 10/11] linux-user: fix /proc/self/stat handling

["Alex Bennée" <alex.bennee@linaro.org>]

Brice Goglin <Brice.Goglin@inria.fr> writes:

> Le 10/04/2020 à 14:33, Alex Bennée a écrit :
>> That was by inspection on my system which seems to truncate a lot
>> earlier. It would be nice to find where in the Linux kernel it is
>> output but I failed to grep the relevant function last night.
>
>
> It's in proc/array.c, do_task_stat() calls proc_task_name(). In the end,
> it seems to use task->tcomm or task->comm which is limited by
>
> #define TASK_COMM_LEN			16

Thanks. I'll amend the commit message. Are you happy with the fix on
your end?

>
> Brice
>
>
>
>>
>> On Fri, 10 Apr 2020, 12:11 Philippe Mathieu-Daudé, <philmd@redhat.com
>> <mailto:philmd@redhat.com>> wrote:
>>
>>     Cc'ing Ludovic in case he can test with Guix-HPC.
>>
>>     On 4/9/20 11:15 PM, Alex Bennée wrote:
>>     > In the original bug report long files names in Guix caused
>>     > /proc/self/stat be truncated without the trailing ") " as
>>     specified in
>>     > proc manpage which says:
>>     >      (2) comm  %s
>>     >             The  filename of the executable, in parentheses.  This
>>     >             is visible whether or not the  executable  is  swapped
>>     >             out.
>>     >
>>     > Additionally it should only be reporting the executable name rather
>>     > than the full path. Fix both these failings while cleaning up
>>     the code
>>     > to use GString to build up the reported values. As the whole
>>     function
>>     > is cleaned up also adjust the white space to the current coding
>>     style.
>>     >
>>     > Message-ID: <fb4c55fa-d539-67ee-c6c9-de8fb63c8488@inria.fr
>>     <mailto:fb4c55fa-d539-67ee-c6c9-de8fb63c8488@inria.fr>>
>>     > Reported-by: Brice Goglin <Brice.Goglin@inria.fr
>>     <mailto:Brice.Goglin@inria.fr>>
>>     > Cc: Philippe_Mathieu-Daudé <philmd@redhat.com
>>     <mailto:philmd@redhat.com>>
>>     > Signed-off-by: Alex Bennée <alex.bennee@linaro.org
>>     <mailto:alex.bennee@linaro.org>>
>>     > ---
>>     >   linux-user/syscall.c | 43
>>     +++++++++++++++++++------------------------
>>     >   1 file changed, 19 insertions(+), 24 deletions(-)
>>     >
>>     > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>     > index 6495ddc4cda..674f70e70a5 100644
>>     > --- a/linux-user/syscall.c
>>     > +++ b/linux-user/syscall.c
>>     > @@ -7295,34 +7295,29 @@ static int open_self_stat(void *cpu_env,
>>     int fd)
>>     >   {
>>     >       CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
>>     >       TaskState *ts = cpu->opaque;
>>     > -    abi_ulong start_stack = ts->info->start_stack;
>>     > +    g_autoptr(GString) buf = g_string_new(NULL);
>>     >       int i;
>>     >   
>>     >       for (i = 0; i < 44; i++) {
>>     > -      char buf[128];
>>     > -      int len;
>>     > -      uint64_t val = 0;
>>     > -
>>     > -      if (i == 0) {
>>     > -        /* pid */
>>     > -        val = getpid();
>>     > -        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>     > -      } else if (i == 1) {
>>     > -        /* app name */
>>     > -        snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>     > -      } else if (i == 27) {
>>     > -        /* stack bottom */
>>     > -        val = start_stack;
>>     > -        snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>     > -      } else {
>>     > -        /* for the rest, there is MasterCard */
>>     > -        snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' ');
>>     > -      }
>>     > +        if (i == 0) {
>>     > +            /* pid */
>>     > +            g_string_printf(buf, FMT_pid " ", getpid());
>>     > +        } else if (i == 1) {
>>     > +            /* app name */
>>     > +            gchar *bin = g_strrstr(ts->bprm->argv[0], "/");
>>     > +            bin = bin ? bin + 1 : ts->bprm->argv[0];
>>     > +            g_string_printf(buf, "(%.15s) ", bin);
>>
>>     15 or 125? 15 seems short. From your previous test I understood it
>>     was
>>     124, for
>>     sizeof("cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___40").
>>
>>     > +        } else if (i == 27) {
>>     > +            /* stack bottom */
>>     > +            g_string_printf(buf, TARGET_ABI_FMT_ld " ",
>>     ts->info->start_stack);
>>     > +        } else {
>>     > +            /* for the rest, there is MasterCard */
>>     > +            g_string_printf(buf, "0%c", i == 43 ? '\n' : ' ');
>>     > +        }
>>     >   
>>     > -      len = strlen(buf);
>>     > -      if (write(fd, buf, len) != len) {
>>     > -          return -1;
>>     > -      }
>>     > +        if (write(fd, buf->str, buf->len) != buf->len) {
>>     > +            return -1;
>>     > +        }
>>     >       }
>>     >   
>>     >       return 0;
>>     >
>>


-- 
Alex Bennée