From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 11A9E385DC0C for ; Mon, 13 Apr 2020 19:16:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 11A9E385DC0C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 7081D1E60C; Mon, 13 Apr 2020 15:16:13 -0400 (EDT) Subject: Re: [PATCH] Implement IP_STAT on NetBSD To: Kamil Rytarowski , gdb-patches@sourceware.org References: <20200413181911.17133-1-n54@gmx.com> From: Simon Marchi Message-ID: <64ef8889-0782-a541-6efa-3c37ed06b372@simark.ca> Date: Mon, 13 Apr 2020 15:16:12 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200413181911.17133-1-n54@gmx.com> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-24.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Apr 2020 19:16:16 -0000 On 2020-04-13 2:19 p.m., Kamil Rytarowski wrote: > NetBSD ships with a compatibility layer in a /proc virtual file system > and delivers optionally /proc//stat. In case of missing > /proc//stat fallback to emulating it with sysctl(3) APIs. > > gdb/ChangeLog: > > * nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h". > (nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur) > (nbsd_pid_to_statstr): New. > (nbsd_nat_target::info_proc): Add do_stat. > --- > gdb/ChangeLog | 7 ++ > gdb/nbsd-nat.c | 278 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 285 insertions(+) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 7d91abf6334..5ff76e85cb3 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,10 @@ > +2020-04-13 Kamil Rytarowski > + > + * nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h". > + (nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur) > + (nbsd_pid_to_statstr): New. > + (nbsd_nat_target::info_proc): Add do_stat. > + > 2020-04-12 Kamil Rytarowski > > * nbsd-nat.c (nbsd_nat_target::info_proc): Add IP_MINIMAL and > diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c > index 5eaf9dec8af..b2a05d63122 100644 > --- a/gdb/nbsd-nat.c > +++ b/gdb/nbsd-nat.c > @@ -28,6 +28,8 @@ > #include > #include > #include > +#include > +#include > > /* Return the name of a file that can be opened to get the symbols for > the child process identified by PID. */ > @@ -58,6 +60,134 @@ nbsd_pid_to_cwd (int pid) > return buf; > } > > +/* Return the kinfo_proc2 structure for the process identified by PID. */ > + > +static bool > +nbsd_pid_to_kinfo_proc2(pid_t pid, struct kinfo_proc2 *ki) Space. > +{ > + gdb_assert (ki != nullptr); > + > + size_t size = sizeof (*ki); > + int mib[6] = {CTL_KERN, KERN_PROC2, KERN_PROC_PID, pid, > + static_cast (size), 1}; > + return sysctl (mib, ARRAY_SIZE (mib), ki, &size, NULL, 0); That looks counter-intuitive. sysctl returns 0 on success, which translates to false as a bool. WHen a function returns a bool it is more typical that true means success. This happens lower too. > +} > + > +/* Return the RLIMIT value for the process identified by PID. */ > + > +static bool > +nbsd_pid_to_rlimit_cur(pid_t pid, rlim_t *rlim, int type) Space. > +{ > + gdb_assert (rlim != nullptr); > + > + size_t size = sizeof (*rlim); > + int mib[5] = {CTL_PROC, pid, PROC_PID_LIMIT, type, PROC_PID_LIMIT_TYPE_SOFT}; > + return sysctl (mib, ARRAY_SIZE (mib), rlim, &size, NULL, 0); > +} > + > +/* Return the emulated /proc/#/stat string for the process identified by PID. > + NetBSD keeps the /proc filesystem that is optional and not recommended for > + new software, thus this call emulates it with sysctl(3). */ > + > +static gdb::unique_xmalloc_ptr > +nbsd_pid_to_statstr (int pid) > +{ > + struct kinfo_proc2 ki; > + if (nbsd_pid_to_kinfo_proc2 (pid, &ki)) > + return nullptr; > + > + struct rusage cru = {}; > + /* We can check only the current process. */ > + if (getpid() == pid) Space. > @@ -433,6 +568,149 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what) > else > warning (_("unable to fetch virtual memory map")); > } > + if (do_stat) > + { > + /* First, try with Linux-compat /proc//stat. */ > + char filename[100]; > + xsnprintf (filename, sizeof filename, "/proc/%d/stat", pid); Use std::string filename = string_printf (...); > + gdb::unique_xmalloc_ptr statstr > + = target_fileio_read_stralloc (NULL, filename); > + > + /* Then fallback to emulating /proc//stat with sysctl(3). */ > + if (statstr == nullptr) > + statstr = nbsd_pid_to_statstr (pid); I find it a bit strange to take the route of formatting the contents of kinfo_proc2, only to parse it here. I think it would have been more natural to do: - If /proc/#/stat is available: string -> struct some_internal_struct -> print function - If /proc/#/stat is available: kinfo_proc2 -> struct some_internal_struct -> print function > + > + if (statstr) != nullptr > + { > + const char *p = statstr.get (); > + > + printf_filtered (_("Process: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + > + p = skip_spaces (p); > + if (*p == '(') > + { > + /* ps command also relies on no trailing fields > + ever contain ')'. */ > + const char *ep = strrchr (p, ')'); > + if (ep != NULL) > + { > + printf_filtered ("Exec file: %.*s\n", > + (int) (ep - p - 1), p + 1); > + p = ep + 1; > + } > + } > + > + p = skip_spaces (p); > + if (*p) > + printf_filtered (_("State: %c\n"), *p++); > + > + if (*p) > + printf_filtered (_("Parent process: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Process group: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Session id: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("TTY: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("TTY owner process group: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + > + if (*p) > + printf_filtered (_("Flags: %s\n"), > + hex_string (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Minor faults (no memory page): %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Minor faults, children: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Major faults (memory page faults): %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Major faults, children: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("utime: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("stime: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("utime, children: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("stime, children: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("jiffies remaining in current " > + "time slice: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("'nice' value: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("jiffies until next timeout: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("jiffies until next SIGALRM: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("start time (jiffies since " > + "system boot): %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Virtual memory size: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Resident set size: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("rlim: %s\n"), > + pulongest (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Start of text: %s\n"), > + hex_string (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("End of text: %s\n"), > + hex_string (strtoulst (p, &p, 10))); > + if (*p) > + printf_filtered (_("Start of stack: %s\n"), > + hex_string (strtoulst (p, &p, 10))); > +#if 0 /* Don't know how architecture-dependent the rest is... > + Anyway the signal bitmap info is available from "status". */ Either figure out how to print this correctly, or let's just leave it out. Simon